[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

commit 4e52e0d49d96958facab3e99b6b6f8519d2d9c76
Author: Jim Fehlig <jfehlig novell com>
Date:   Wed Jan 13 18:54:58 2010 -0700

    Modify virsh commands
    
    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.
    
    V2: Only invoke virDomain{Attach,Detach}DeviceFlags() if
    "--persistent" flag is specified.  Otherwise invoke
    virDomain{Attach,Detach}Device() to retain current behavior.

diff --git a/tools/virsh.c b/tools/virsh.c
index 1fae5e6..0763dcc 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;
+    unsigned int flags;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         return FALSE;
@@ -6315,7 +6317,14 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
         return FALSE;
     }
 
-    ret = virDomainAttachDevice(dom, buffer);
+    if (vshCommandOptBool(cmd, "persistent")) {
+        flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+        if (virDomainIsActive(dom) == 1)
+           flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+        ret = virDomainAttachDeviceFlags(dom, buffer, flags);
+    } else {
+        ret = virDomainAttachDevice(dom, buffer);
+    }
     VIR_FREE(buffer);
 
     if (ret < 0) {
@@ -6343,6 +6352,7 @@ static const vshCmdInfo info_detach_device[] = {
 static const vshCmdOptDef opts_detach_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 detachment")},
     {NULL, 0, 0, NULL}
 };
 
@@ -6354,6 +6364,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
     char *buffer;
     int ret = TRUE;
     int found;
+    unsigned int flags;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         return FALSE;
@@ -6373,7 +6384,14 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
         return FALSE;
     }
 
-    ret = virDomainDetachDevice(dom, buffer);
+    if (vshCommandOptBool(cmd, "persistent")) {
+        flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+        if (virDomainIsActive(dom) == 1)
+           flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+        ret = virDomainDetachDeviceFlags(dom, buffer, flags);
+    } else {
+        ret = virDomainDetachDevice(dom, buffer);
+    }
     VIR_FREE(buffer);
 
     if (ret < 0) {
@@ -6405,6 +6423,7 @@ static const vshCmdOptDef opts_attach_interface[] = {
     {"target", VSH_OT_DATA, 0, gettext_noop("target network name")},
     {"mac",    VSH_OT_DATA, 0, gettext_noop("MAC address")},
     {"script", VSH_OT_DATA, 0, gettext_noop("script used to bridge network interface")},
+    {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist interface attachment")},
     {NULL, 0, 0, NULL}
 };
 
@@ -6415,6 +6434,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
     char *mac, *target, *script, *type, *source;
     int typ, ret = FALSE;
     char *buf = NULL, *tmp = NULL;
+    unsigned int flags;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         goto cleanup;
@@ -6489,13 +6509,22 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
     if (!buf) goto cleanup;
     strcat(buf, "    </interface>\n");
 
-    if (virDomainAttachDevice(dom, buf)) {
-        goto cleanup;
+    if (vshCommandOptBool(cmd, "persistent")) {
+        flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+        if (virDomainIsActive(dom) == 1)
+            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+        ret = virDomainAttachDeviceFlags(dom, buf, flags);
     } else {
-        vshPrint(ctl, "%s", _("Interface attached successfully\n"));
+        ret = virDomainAttachDevice(dom, buf);
     }
 
-    ret = TRUE;
+    if (ret != 0) {
+        vshError(ctl, _("Failed to attach interface"));
+        ret = FALSE;
+    } else {
+        vshPrint(ctl, "%s", _("Interface attached successfully\n"));
+        ret = TRUE;
+    }
 
  cleanup:
     if (dom)
@@ -6518,6 +6547,7 @@ static const vshCmdOptDef opts_detach_interface[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")},
     {"type",   VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("network interface type")},
     {"mac",    VSH_OT_STRING, 0, gettext_noop("MAC address")},
+    {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist interface detachment")},
     {NULL, 0, 0, NULL}
 };
 
@@ -6534,6 +6564,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
     char *doc, *mac =NULL, *type;
     char buf[64];
     int i = 0, diff_mac, ret = FALSE;
+    unsigned int flags;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         goto cleanup;
@@ -6605,10 +6636,21 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
     }
 
-    ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf));
-    if (ret != 0)
+    if (vshCommandOptBool(cmd, "persistent")) {
+        flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+        if (virDomainIsActive(dom) == 1)
+            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+        ret = virDomainDetachDeviceFlags(dom,
+                                         (char *)xmlBufferContent(xml_buf),
+                                         flags);
+    } else {
+        ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf));
+    }
+
+    if (ret != 0) {
+        vshError(ctl, _("Failed to detach interface"));
         ret = FALSE;
-    else {
+    } else {
         vshPrint(ctl, "%s", _("Interface detached successfully\n"));
         ret = TRUE;
     }
@@ -6642,6 +6684,7 @@ static const vshCmdOptDef opts_attach_disk[] = {
     {"subdriver", VSH_OT_STRING, 0, gettext_noop("subdriver of disk device")},
     {"type",    VSH_OT_STRING, 0, gettext_noop("target device type")},
     {"mode",    VSH_OT_STRING, 0, gettext_noop("mode of device reading and writing")},
+    {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist disk attachment")},
     {NULL, 0, 0, NULL}
 };
 
@@ -6652,6 +6695,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
     char *source, *target, *driver, *subdriver, *type, *mode;
     int isFile = 0, ret = FALSE;
     char *buf = NULL, *tmp = NULL;
+    unsigned int flags;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         goto cleanup;
@@ -6767,12 +6811,22 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
     if (!buf) goto cleanup;
     strcat(buf, "    </disk>\n");
 
-    if (virDomainAttachDevice(dom, buf))
-        goto cleanup;
-    else
-        vshPrint(ctl, "%s", _("Disk attached successfully\n"));
+    if (vshCommandOptBool(cmd, "persistent")) {
+        flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+        if (virDomainIsActive(dom) == 1)
+            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+        ret = virDomainAttachDeviceFlags(dom, buf, flags);
+    } else {
+        ret = virDomainAttachDevice(dom, buf);
+    }
 
-    ret = TRUE;
+    if (ret != 0) {
+        vshError(ctl, _("Failed to attach disk"));
+        ret = FALSE;
+    } else {
+        vshPrint(ctl, "%s", _("Disk attached successfully\n"));
+        ret = TRUE;
+    }
 
  cleanup:
     if (dom)
@@ -6794,6 +6848,7 @@ static const vshCmdInfo info_detach_disk[] = {
 static const vshCmdOptDef opts_detach_disk[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")},
     {"target", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("target of disk device")},
+    {"persistent", VSH_OT_BOOL, 0, gettext_noop("persist disk detachment")},
     {NULL, 0, 0, NULL}
 };
 
@@ -6809,6 +6864,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
     virDomainPtr dom = NULL;
     char *doc, *target;
     int i = 0, diff_tgt, ret = FALSE;
+    unsigned int flags;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         goto cleanup;
@@ -6874,10 +6930,21 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
     }
 
-    ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf));
-    if (ret != 0)
+    if (vshCommandOptBool(cmd, "persistent")) {
+        flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+        if (virDomainIsActive(dom) == 1)
+            flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+        ret = virDomainDetachDeviceFlags(dom,
+                                         (char *)xmlBufferContent(xml_buf),
+                                         flags);
+    } else {
+        ret = virDomainDetachDevice(dom, (char *)xmlBufferContent(xml_buf));
+    }
+
+    if (ret != 0) {
+        vshError(ctl, _("Failed to detach disk"));
         ret = FALSE;
-    else {
+    } else {
         vshPrint(ctl, "%s", _("Disk detached successfully\n"));
         ret = TRUE;
     }

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]