On 01/25/2013 05:22 AM, Peter Krempa wrote: > The flag causes the XML of the disk that would be attached to be printed > instead. > --- > tools/virsh-domain.c | 46 ++++++++++++++++++++++++++++------------------ > tools/virsh.pod | 5 ++++- > 2 files changed, 32 insertions(+), 19 deletions(-) Nice usability improvement. ACK. > - VIR_FREE(xml); > + if (!vshCommandOptBool(cmd, "print-xml")) { > + if (vshCommandOptBool(cmd, "config")) { > + flags = VIR_DOMAIN_AFFECT_CONFIG; > + if (virDomainIsActive(dom) == 1) > + flags |= VIR_DOMAIN_AFFECT_LIVE; > + ret = virDomainAttachDeviceFlags(dom, xml, flags); > + } else { > + ret = virDomainAttachDevice(dom, xml); > + } > > - if (ret != 0) { > - vshError(ctl, "%s", _("Failed to attach disk")); > + if (ret != 0) { > + vshError(ctl, "%s", _("Failed to attach disk")); > + goto cleanup; > + } else { > + vshPrint(ctl, "%s", _("Disk attached successfully\n")); > + } > } else { > - vshPrint(ctl, "%s", _("Disk attached successfully\n")); > - functionReturn = true; > + vshPrint(ctl, "%s", xml); > } Rather than reindenting everything, it might have been shorter to do: if (vshCommandOptBool(cmd, "print-xml")) { vshPrint(ctl, "%s", xml); functionReturn = true; goto cleanup; } the leave existing code for AttachDevice alone. But that's cosmetic, I'm okay with your patch as-is. -- 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