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

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

 



On 07/01/2013 06:40 PM, Eric Blake wrote:
> On 07/01/2013 07:09 AM, 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"));
> 
> s/asynchronnous/asynchronous/
> 
> Should this restriction be enforced at the API level in src/libvirt.c
> instead?  Probably not, because it might be conceivable that some types
> of pivot operations might be long-running.
> 
> Next, do we need the restriction?  If the 'async' flag merely means
> return as fast as possible, and pivots (currently) always return
> immediately (no long-running operations), then we can declare that use
> or absence of the async flag makes no difference, and can silently
> ignore it instead of forcefully rejecting it.

Well, since the flag actually works, I agree that we shouldn't reject it.

> 
> I'm inclined to NACK this patch, if we can't come up with a better
> explanation of why it is needed.
> 

I'm dropping the patch then.

Jan

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]