Re: [libvirt] [PATCH 9/9] Modify virsh commands

On Thu, Jan 14, 2010 at 10:42:46AM -0700, Jim Fehlig wrote: > Change all virsh commands that invoke virDomain{Attach,Detach}Device() > to use virDomain{Attach,Detach}DeviceFlags() instead. > > Add a "--persistent" flag to these virsh commands, allowing user to > specify that the domain persisted config be modified as well. > > --- > tools/virsh.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 files changed, 49 insertions(+), 6 deletions(-) > > diff --git a/tools/virsh.c b/tools/virsh.c > index 1fae5e6..a082b89 100644 > --- a/tools/virsh.c > +++ b/tools/virsh.c > @@ -6285,6 +6285,7 @@ static const vshCmdInfo info_attach_device[] = { > static const vshCmdOptDef opts_attach_device[] = { > {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, > {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("XML file")}, > + {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist device attachment")}, > {NULL, 0, 0, NULL} > }; > > @@ -6296,6 +6297,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) > char *buffer; > int ret = TRUE; > int found; > + int flags = VIR_DOMAIN_DEVICE_MODIFY_CURRENT; > > if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) > return FALSE; > @@ -6309,13 +6311,18 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) > virDomainFree(dom); > return FALSE; > } > + if (vshCommandOptBool(cmd, "persistent")) > + flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; > > if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) { > virDomainFree(dom); > return FALSE; > } > > - ret = virDomainAttachDevice(dom, buffer); > + if (virDomainIsActive(dom)) > + flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE; > + > + ret = virDomainAttachDeviceFlags(dom, buffer, flags); > VIR_FREE(buffer); > > if (ret < 0) { > This has the same subtle compatability problem that the public API entry point suffers from. New virsh won't be able to modify guests from an existing libvirtd. I think that if flags == 0, then we should use the existing API method, and only use the new virDomainAttachDeviceFlags if flags != 0. I think we probably want to default to 0, and only set the VIR_DOMAIN_DEVICE_MODIFY_LIVE flag if a '--live' flag is given. Basically we need to try & ensure compatability with existing operation if at all possible