[libvirt] [PATCH RFC] virsh: Fix semantics of --config for "update-device" command

[Peter Krempa]

The man page states that with --config the next boot is affected. This
can be understood as if _only_ the next bood was affected. This isn't
true if the machine is running.

This patch adds the full --live, --config, --current infrastructure and
tweaks stuff to correctly support the obsolete --persistent flag.
---

Notes:
    - This patch will be greatly simplified with macros from:
      http://www.redhat.com/archives/libvir-list/2013-March/msg00268.html
    
    - There are multiple places like this in virsh that will need update too.
      (detach-device for example)

    - https://bugzilla.redhat.com/show_bug.cgi?id=921398

 tools/virsh-domain.c | 53 +++++++++++++++++++++++++++++++++++++++++-----------
 tools/virsh.pod      | 22 +++++++++++++++-------
 2 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e9da11f..33545f6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9304,13 +9304,21 @@ static const vshCmdOptDef opts_update_device[] = {
      .help = N_("XML file")
     },
     {.name = "persistent",
-     .type = VSH_OT_ALIAS,
-     .help = "config"
+     .type = VSH_OT_BOOL,
+     .help = N_("make live change persistent")
     },
     {.name = "config",
      .type = VSH_OT_BOOL,
      .help = N_("affect next boot")
     },
+    {.name = "live",
+     .type = VSH_OT_BOOL,
+     .help = N_("affect running domain")
+    },
+    {.name = "current",
+     .type = VSH_OT_BOOL,
+     .help = N_("affect current domain")
+    },
     {.name = "force",
      .type = VSH_OT_BOOL,
      .help = N_("force device update")
@@ -9325,7 +9333,34 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
     const char *from = NULL;
     char *buffer = NULL;
     bool ret = false;
-    unsigned int flags;
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+    bool current = vshCommandOptBool(cmd, "current");
+    bool config = vshCommandOptBool(cmd, "config");
+    bool live = vshCommandOptBool(cmd, "live");
+    bool persistent = vshCommandOptBool(cmd, "persistent");
+
+    if (persistent) {
+        if (current || config || live) {
+            vshError(ctl, "%s", _("--persistent is incompatible with "
+                                  "--current, --live or --config"));
+            return false;
+        }
+
+        flags = VIR_DOMAIN_AFFECT_CONFIG;
+    } else {
+        if (current) {
+            if (live || config) {
+                vshError(ctl, "%s", _("--current must be specified "
+                                      "exclusively"));
+                return false;
+            }
+        } else {
+            if (config)
+                flags |= VIR_DOMAIN_AFFECT_CONFIG;
+            if (live)
+                flags |= VIR_DOMAIN_AFFECT_LIVE;
+        }
+    }

     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
@@ -9333,19 +9368,15 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
         goto cleanup;

+    if (persistent &&
+        virDomainIsActive(dom) == 1)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
     if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
         vshReportError(ctl);
         goto cleanup;
     }

-    if (vshCommandOptBool(cmd, "config")) {
-        flags = VIR_DOMAIN_AFFECT_CONFIG;
-        if (virDomainIsActive(dom) == 1)
-           flags |= VIR_DOMAIN_AFFECT_LIVE;
-    } else {
-        flags = VIR_DOMAIN_AFFECT_LIVE;
-    }
-
     if (vshCommandOptBool(cmd, "force"))
         flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE;

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 7fb89e4..8bb0490 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1887,18 +1887,26 @@ If I<--config> is specified, alter persistent configuration, effect observed
 on next boot, for compatibility purposes, I<--persistent> is alias of
 I<--config>.

-=item B<update-device> I<domain> I<file> [I<--config>] [I<--force>]
+=item B<update-device> I<domain> I<file> [I<--force>]
+[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]

 Update the characteristics of a device associated with I<domain>,
-based on the device definition in an XML I<file>.  If the I<--config>
-option is used, the changes will take affect the next time libvirt
-starts the domain.  For compatibility purposes, I<--persistent> is
-alias of I<--config>.  The I<--force> option can be used to force
-device update, e.g., to eject a CD-ROM even if it is locked/mounted in
-the domain. See the documentation at
+based on the device definition in an XML I<file>.  The I<--force> option
+can be used to force device update, e.g., to eject a CD-ROM even if it is
+locked/mounted in the domain. See the documentation at
 L<http://libvirt.org/formatdomain.html#elementsDevices> to learn about
 libvirt XML format for a device.

+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current> is
+exclusive. Not specifying any flag is the same as specifying I<--current>.
+
+For compatibility purposes, I<--persistent> is alias of I<--config> and
+I<--live> if the domain is running. This flag isn't compatible with the
+other domain status flags.
+
 =item B<change-media> I<domain> I<path> [I<--eject>] [I<--insert>]
 [I<--update>] [I<source>] [I<--force>] [[I<--live>] [I<--config>] | [I<--current>]]

-- 
1.8.1.5