Daniel P. Berrange wrote:
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
The existing behavior is essentially VIR_DOMAIN_DEVICE_MODIFY_LIVE. qemu fails the operation if domain is inactive. Attach works on inactive Xen domains, but detach does not. vbox has an impl for inactive domains, but I haven't tested it. I kept the existing behavior by only calling vir{Attach,Detach}DeviceFlags() only when the new virsh flag "persistent" is specified. Revised patch below. Thanks, Jim