[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 12/18] blockcopy: expose new API in virsh



On 09/05/2014 03:41 AM, Peter Krempa wrote:
> On 08/31/14 06:02, Eric Blake wrote:
>> Expose the new power of virDomainBlockCopy through virsh (well,
>> all but the finer-grained bandwidth, as that is its own can of
>> worms for a later patch).  Continue to use the older API where
>> possible, for maximum compatibility.
>>

>>
>> +    if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
>> +        return false;

Oops, in all my refactoring, I missed that this statement...[1]

>> +    if (vshCommandOptString(cmd, "dest", &dest) < 0)
>> +        return false;
>> +    if (vshCommandOptString(cmd, "xml", &xml) < 0)
>> +        return false;
>> +    if (vshCommandOptString(cmd, "format", &format) < 0)
>> +        return false;
>> +
>> +    VSH_EXCLUSIVE_OPTIONS_VAR(dest, xml);
> 
> These were designed to work on booleans, but pointers are fortunately
> sharing the semantics 

Yep, convenient :)

> 
>> +    VSH_EXCLUSIVE_OPTIONS_VAR(format, xml);
>> +    VSH_EXCLUSIVE_OPTIONS_VAR(blockdev, xml);
>>
>>      if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
>>          return false;

[1]...was already present.

>> +
>> +        if (!xmlstr) {
>> +            virBuffer buf = VIR_BUFFER_INITIALIZER;
>> +            virBufferEscapeString(&buf, "<disk type='%s'>\n",
>> +                                  blockdev ? "block" : "file");
> 
> This one doesn't really need escaping
> 
>> +            virBufferAdjustIndent(&buf, 2);
>> +            virBufferEscapeString(&buf, "<source %s",
>> +                                  blockdev ? "dev" : "file");
> 
> and this either

Okay, I switched them to virBufferAsprintf.

> 
> ACK as is, all comments are pure cosmetic.

Now pushed with those fixes.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]