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