[libvirt] [PATCH 1/5] blockjob: add qemu capabilities related to block pull jobs

[Eric Blake]

On 04/11/2012 07:09 PM, Daniel Veillard wrote:
> On Wed, Apr 11, 2012 at 05:40:34PM -0600, Eric Blake wrote:
>> RHEL 6.2 was released with an early version of block jobs, which only
>> worked on the qed file format, where the commands were spelled with
>> underscore (contrary to QMP style), and where 'block_job_cancel' was
>> synchronous and did not trigger an event.
>>

>> +++ b/src/qemu/qemu_monitor.h
>> @@ -538,8 +538,9 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
>>                          const char *back,
>>                          unsigned long bandwidth,
>>                          virDomainBlockJobInfoPtr info,
>> -                        int mode)
>> -    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
>> +                        int mode,
>> +                        bool async)
>> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> 
>   Hum, gcc wasn't complaining on an "ATTRIBUTE_NONNULL(5)" for something
> which wasn't a pointer ?

Context.  There are lines omitted between the @@ marker and the changed
lines; param 5 is virDomainBlockJobInfoPtr.  In fact, I introduced a bug
in commit 10ec36e2 for adding ATTRIBUTE_NONNULL(5) in the first place,
since we already have existing callers that pass NULL.


>> +        else if (STREQ(name, "block_job_cancel"))
>> +            qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC);
>> +        else if (STREQ(name, "block-job-cancel"))
>> +            qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC);
>>      }
> 
>   Hum ... seems to me that we always set bits QEMU_CAPS_BLOCKJOB_SYNC
> and QEMU_CAPS_BLOCKJOB_ASYNC together, so do you envision cases where
> one was set and not the other ? If not why not merge them for the sake
> of one less bit to manage ?

On the contrary, you will set at most one of the two, and never both.
That is, there is no qemu image that supports both 'block_job_cancel'
and 'block-job-cancel' simultaneously, so we need the two bits to tell
the two styles apart.

> 
>   Minor point about the extra bit, to me that's fine, ACK

Thanks for the review.  I'll see what you said about the rest of the
series before pushing.

-- 
Eric Blake   eblake at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120411/8c15f9fe/attachment-0001.sig>