[libvirt] [PATCH] Add success output to virsh attach/detach commands
The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback. Thanks, Cole commit eda28c5de3d367d001ca89ffc969fcc3c0185abb Author: Cole Robinson <crobinso@dhcp-100-19-219.bos.redhat.com> Date: Thu Aug 21 17:59:13 2008 -0400 Report success message for virsh attach-* and detach-* diff --git a/src/virsh.c b/src/virsh.c index 67d9658..c22cdc4 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -4490,6 +4490,8 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) vshError(ctl, FALSE, _("Failed to attach device from %s"), from); virDomainFree(dom); return FALSE; + } else { + vshPrint(ctl, _("Device attached successfully\n")); } virDomainFree(dom); @@ -4547,6 +4549,8 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) vshError(ctl, FALSE, _("Failed to detach device from %s"), from); virDomainFree(dom); return FALSE; + } else { + vshPrint(ctl, _("Device detached successfully\n")); } virDomainFree(dom); @@ -4655,8 +4659,11 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (!buf) goto cleanup; strcat(buf, " </interface>\n"); - if (virDomainAttachDevice(dom, buf)) + if (virDomainAttachDevice(dom, buf)) { goto cleanup; + } else { + vshPrint(ctl, _("Interface attached successfully\n")); + } ret = TRUE; @@ -4772,8 +4779,10 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf)); if (ret != 0) ret = FALSE; - else + else { + vshPrint(ctl, _("Interface detached successfully\n")); ret = TRUE; + } cleanup: if (dom) @@ -4939,6 +4948,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (virDomainAttachDevice(dom, buf)) goto cleanup; + else + vshPrint(ctl, _("Disk attached successfully\n")); ret = TRUE; @@ -5046,8 +5057,10 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf)); if (ret != 0) ret = FALSE; - else + else { + vshPrint(ctl, _("Disk detached successfully\n")); ret = TRUE; + } cleanup: xmlXPathFreeObject(obj);
On Thu, Aug 21, 2008 at 11:19:29PM -0400, Cole Robinson wrote:
The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback.
Hmmm ... The patch is fine, but I don't really like commands which are verbose on anything other than errors. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Fri, Aug 22, 2008 at 09:26:13AM +0100, Richard W.M. Jones wrote:
On Thu, Aug 21, 2008 at 11:19:29PM -0400, Cole Robinson wrote:
The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback.
Hmmm ... The patch is fine, but I don't really like commands which are verbose on anything other than errors.
Nor do I, but all our other commands print a confirmation message too, so for sake of consistency I think Cole's patch is reasonable. We do have a --quiet arg to make it STFU Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Fri, Aug 22, 2008 at 09:26:13AM +0100, Richard W.M. Jones wrote:
On Thu, Aug 21, 2008 at 11:19:29PM -0400, Cole Robinson wrote:
The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback.
Hmmm ... The patch is fine, but I don't really like commands which are verbose on anything other than errors.
Nor do I, but all our other commands print a confirmation message too, so for sake of consistency I think Cole's patch is reasonable. We do have a --quiet arg to make it STFU
That was my reasoning as well. When commands like 'start' and 'define' report success but attach-device doesn't, I was left wondering if it even worked. - Cole
On Thu, Aug 21, 2008 at 11:19:29PM -0400, Cole Robinson wrote:
The virsh attach-* and detach-* commands don't say anything if the operation succeeds, which doesn't seem very polite. The attached adds some simple feedback.
Okay, based on the discussion and own opinion, this makes sense, so aplied and commited. BTW welcome to the commiter club, you can now help me pushing all those patches where you got ACKs but nobody actually did the last step :-) So thanks again ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones