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

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.