[libvirt] [PATCH 2/2] blockjob: make PIVOT and ASYNC flags mutually exclusive

[Eric Blake]

On 07/01/2013 09:28 AM, Peter Krempa wrote:
> (CC'd Eric)
> 
> On 07/01/13 15:09, Ján Tomko wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=977678
>> ---
>>   src/qemu/qemu_driver.c | 7 +++++++
>>   tools/virsh-domain.c   | 9 ++++++---
>>   2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 6a83fda..aa7affe 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -14153,6 +14153,13 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
>> const char *path, unsigned int flags)
>>       virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC |
>>                     VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, -1);
>>
>> +    if ((flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC) &&
>> +        (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
>> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
>> +                       _("asynchronnous pivot not supported"));
>> +        return -1;
>> +    }
>> +
> 
> I agree with this hunk.

I'm still not convinced, but agree that only this hunk is necessary if
we want the patch.

>> +    VSH_EXCLUSIVE_OPTIONS_VAR(async, pivot);
> 
> .. but I don't think we should forbid this combination in virsh. I think
> could happen that we might need this combination. I think that the
> combination of _ABORT and _PIVOT is less usefull.
> 
> Eric, what do you think?
> 
> (Or if we do forbid it, we need to document it, and ban it at library
> level instead of qemu driver)

I prefer letting virsh pass ALL combinations down to lower level
software, when possible.  It lets us test that the lower-level software
is accepting or rejecting combinations, instead of silently
short-circuiting combinations that may later prove useful.  I agree with
your desire to NACK this hunk.

-- 
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: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130701/e190948e/attachment-0001.sig>