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 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.

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

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]