[libvirt] [PATCHv2 2/4] virsh-domain: Add --live, --config, --current logic to cmdDetachInterface

[Peter Krempa]

Use the established approach to improve this function too.
---

Notes:
    Version 2:
    - state that legacy API has no well defined semantics
    - add note about possible semantics change
    - remove mutual exclusivity of --persistent --live
    - remove --force parameter added due to too much copy and paste

 tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++++++++-------------
 tools/virsh.pod      | 19 +++++++++++++++----
 2 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2a228f5..d7778dd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9352,13 +9352,21 @@ static const vshCmdOptDef opts_detach_interface[] = {
      .help = N_("MAC address")
     },
     {.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 = NULL}
 };

@@ -9372,12 +9380,26 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
     xmlNodePtr cur = NULL, matchNode = NULL;
     xmlBufferPtr xml_buf = NULL;
     const char *mac =NULL, *type = NULL;
-    char *doc;
+    char *doc = NULL;
     char buf[64];
     int i = 0, diff_mac;
     int ret;
     int functionReturn = 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");
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+    if (config || persistent)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;

     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
@@ -9388,13 +9410,14 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
         goto cleanup;

-    doc = virDomainGetXMLDesc(dom, 0);
-    if (!doc)
+    if (persistent &&
+        virDomainIsActive(dom) == 1)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (!(doc = virDomainGetXMLDesc(dom, 0)))
         goto cleanup;

-    xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt);
-    VIR_FREE(doc);
-    if (!xml) {
+    if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt))) {
         vshError(ctl, "%s", _("Failed to get interface information"));
         goto cleanup;
     }
@@ -9459,10 +9482,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
     }

-    if (vshCommandOptBool(cmd, "config")) {
-        flags = VIR_DOMAIN_AFFECT_CONFIG;
-        if (virDomainIsActive(dom) == 1)
-            flags |= VIR_DOMAIN_AFFECT_LIVE;
+    if (flags != 0) {
         ret = virDomainDetachDeviceFlags(dom,
                                          (char *)xmlBufferContent(xml_buf),
                                          flags);
@@ -9478,6 +9498,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
     }

  cleanup:
+    VIR_FREE(doc);
     virDomainFree(dom);
     xmlXPathFreeObject(obj);
     xmlXPathFreeContext(ctxt);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 04ea542..e67c3cc 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1877,16 +1877,27 @@ 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<detach-interface> I<domain> I<type> [I<--mac mac>] [I<--config>]
+=item B<detach-interface> I<domain> I<type> [I<--mac mac>]
+[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]

 Detach a network interface from a domain.
 I<type> can be either I<network> to indicate a physical network device or
 I<bridge> to indicate a bridge to a device. It is recommended to use the
 I<mac> option to distinguish between the interfaces if more than one are
 present on the domain.
-If I<--config> is specified, alter persistent configuration, effect observed
-on next boot, for compatibility purposes, I<--persistent> is alias of
-I<--config>.
+
+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. When no flag is specified legacy API is used whose behavior depends
+on the hypervisor driver.
+
+For compatibility purposes, I<--persistent> behaves like I<--config> for
+an offline domain, and like I<--live> I<--config> for a running domain.
+
+Note that older versions of virsh used I<--config> as an alias for
+I<--persistent>.

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