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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list