[libvirt] [PATCH v3 04/18] blockjob: split out block info monitor handling

[Eric Blake]

On 09/04/2014 09:39 AM, Peter Krempa wrote:
> On 08/31/14 06:02, Eric Blake wrote:
>> Another layer of overly-multiplexed code that deserves to be
>> split into obviously separate paths for query vs. modify.
>> This continues the cleanup started in the previous patch.
>>
>> In the process, make some tweaks to simplify the logic when
>> parsing the JSON reply.  There should be no user-visible
>> semantic changes.
>>

>> +int qemuMonitorBlockJobInfo(qemuMonitorPtr mon,
>> +                            const char *device,
>> +                            virDomainBlockJobInfoPtr info)
>> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> 
> Implementation of this function doesn't check for presence of the @info
> structure and dereferences it always. You should mark argument 3 nonnull
> too.

Good catch.


>> -    for (i = 0; i < nr_results; i++) {
>> +    for (i = 0, ret = 0; i < nr_results && ret == 0; i++) {
>>          virJSONValuePtr entry = virJSONValueArrayGet(data, i);
>>          if (!entry) {
>>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                             _("missing array element"));
>> -            return -1;
>> +            ret = -1;
>> +            goto cleanup;
>>          }
>> -        if (qemuMonitorJSONGetBlockJobInfoOne(entry, device, info) == 0)
>> -            return 1;
>> +        ret = qemuMonitorJSONGetBlockJobInfoOne(entry, device, info);
> 
> This doesn't break the loop, thus if the device info isn't last in the
> structure you'd overwrite the return code. I presume the clients don't
> check that far but you've documented that semantics.

Actually, the loop DOES break, just on the next iteration.  I added '&&
ret == 0' to the loop continuation condition.  So the semantics are:

InfoOne fails and returns -1, the next iteration is aborted and overall
ret is -1

InfoOne returns 0 because the requested device didn't match, so the loop
continues to look at the next array member. If all array members are
exhausted, ret remains 0 and overall return is 0 for job not found.

InfoOne succeeds and returns 1, the next iteration is aborted and the
overall ret is 1 for info populated.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140904/eee686e2/attachment-0001.sig>