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

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

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)
>> +                        int mode,
>> +                        bool async)
>   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 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]