[libvirt] [PATCH v2] New virsh feature: domif-setlink --domain --interface --state completer

Simon Kobyda posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180712140759.3995-1-skobyda@redhat.com
Test syntax-check passed
tools/virsh-completer.c | 73 ++++++++++++++++++++++++++++++++++++++++-
tools/virsh-completer.h |  4 +++
tools/virsh-domain.c    |  1 +
3 files changed, 77 insertions(+), 1 deletion(-)
[libvirt] [PATCH v2] New virsh feature: domif-setlink --domain --interface --state completer
Posted by Simon Kobyda 5 years, 9 months ago
After you go through command mentioned above, completer
finds what state the device is currently in, and suggests
an opposite state.

Signed-off-by: Simon Kobyda <skobyda@redhat.com>
---

Changes in V2:
    - Added "Signed-off-by"
    - Changed format of commit message to make it more
    readable

 tools/virsh-completer.c | 73 ++++++++++++++++++++++++++++++++++++++++-
 tools/virsh-completer.h |  4 +++
 tools/virsh-domain.c    |  1 +
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 2327e08340..e32fd82211 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -32,6 +32,7 @@
 #include "internal.h"
 #include "virutil.h"
 #include "viralloc.h"
+#include "virmacaddr.h"
 #include "virstring.h"
 #include "virxml.h"
 
@@ -750,7 +751,6 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
     return NULL;
 }
 
-
 char **
 virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
                             const vshCmd *cmd ATTRIBUTE_UNUSED,
@@ -776,6 +776,77 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
     return NULL;
 }
 
+char **
+virshDomainInterfaceStateCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
+                            const vshCmd *cmd ATTRIBUTE_UNUSED,
+                            unsigned int flags)
+{
+    virshControlPtr priv = ctl->privData;
+    const char *iface = NULL;
+    char **ret = NULL;
+    xmlDocPtr xml = NULL;
+    virMacAddr macaddr;
+    char *state = NULL;
+    char *xpath = NULL;
+    char macstr[18] = "";
+    xmlXPathContextPtr ctxt = NULL;
+    xmlNodePtr *interfaces = NULL;
+    int ninterfaces;
+
+    virCheckFlags(0, NULL);
+
+    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+        return NULL;
+
+    if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0)
+        goto cleanup;
+
+    if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
+        goto cleanup;
+
+    /* normalize the mac addr */
+    if (virMacAddrParse(iface, &macaddr) == 0)
+        virMacAddrFormat(&macaddr, macstr);
+
+    if (virAsprintf(&xpath, "/domain/devices/interface[(mac/@address = '%s') or "
+                            "                          (target/@dev = '%s')]",
+                           macstr, iface) < 0)
+        goto cleanup;
+
+    if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0)
+        goto cleanup;
+
+    if (ninterfaces != 1)
+        goto cleanup;
+
+    ctxt->node = interfaces[0];
+
+    if (VIR_ALLOC_N(ret, 2) < 0)
+        goto error;
+
+    if ((state = virXPathString("string(./link/@state)", ctxt)) &&
+        STREQ(state, "down")) {
+        if (VIR_STRDUP(ret[0], "up") < 0)
+            goto error;
+    } else {
+        if (VIR_STRDUP(ret[0], "down") < 0)
+            goto error;
+    }
+
+ cleanup:
+    VIR_FREE(state);
+    VIR_FREE(interfaces);
+    VIR_FREE(xpath);
+    xmlXPathFreeContext(ctxt);
+    xmlFreeDoc(xml);
+    return ret;
+
+ error:
+    virStringListFree(ret);
+    ret = NULL;
+    goto cleanup;
+}
+
 
 char **
 virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 1c14bb2a54..b4fd2a86c6 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -94,6 +94,10 @@ char ** virshPoolEventNameCompleter(vshControl *ctl,
                                     const vshCmd *cmd,
                                     unsigned int flags);
 
+char ** virshDomainInterfaceStateCompleter(vshControl *ctl,
+                                           const vshCmd *cmd,
+                                           unsigned int flags);
+
 char ** virshNodedevEventNameCompleter(vshControl *ctl,
                                        const vshCmd *cmd,
                                        unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3959b5475b..8adec1d9b1 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3064,6 +3064,7 @@ static const vshCmdOptDef opts_domif_setlink[] = {
     {.name = "state",
      .type = VSH_OT_DATA,
      .flags = VSH_OFLAG_REQ,
+     .completer = virshDomainInterfaceStateCompleter,
      .help = N_("new state of the device")
     },
     {.name = "persistent",
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] New virsh feature: domif-setlink --domain --interface --state completer
Posted by Michal Privoznik 5 years, 9 months ago
On 07/12/2018 04:07 PM, Simon Kobyda wrote:
> After you go through command mentioned above, completer
> finds what state the device is currently in, and suggests
> an opposite state.
> 
> Signed-off-by: Simon Kobyda <skobyda@redhat.com>
> ---
> 
> Changes in V2:
>     - Added "Signed-off-by"
>     - Changed format of commit message to make it more
>     readable
> 
>  tools/virsh-completer.c | 73 ++++++++++++++++++++++++++++++++++++++++-
>  tools/virsh-completer.h |  4 +++
>  tools/virsh-domain.c    |  1 +
>  3 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> index 2327e08340..e32fd82211 100644
> --- a/tools/virsh-completer.c
> +++ b/tools/virsh-completer.c
> @@ -32,6 +32,7 @@
>  #include "internal.h"
>  #include "virutil.h"
>  #include "viralloc.h"
> +#include "virmacaddr.h"
>  #include "virstring.h"
>  #include "virxml.h"
>  
> @@ -750,7 +751,6 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
>      return NULL;
>  }
>  
> -


This is a spurious change and has nothing to do with the feature you're
proposing.

>  char **
>  virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
>                              const vshCmd *cmd ATTRIBUTE_UNUSED,
> @@ -776,6 +776,77 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
>      return NULL;
>  }
>  
> +char **
> +virshDomainInterfaceStateCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
> +                            const vshCmd *cmd ATTRIBUTE_UNUSED,
> +                            unsigned int flags)

Misaligned function arguments. Also, I'm not quite sure why you marked
@ctl as unused when it's being used at the very next line. Same goes for
@cmd.

> +{
> +    virshControlPtr priv = ctl->privData;
> +    const char *iface = NULL;
> +    char **ret = NULL;
> +    xmlDocPtr xml = NULL;
> +    virMacAddr macaddr;
> +    char *state = NULL;
> +    char *xpath = NULL;
> +    char macstr[18] = "";

What's wrong with VIR_MAC_STRING_BUFLEN?

> +    xmlXPathContextPtr ctxt = NULL;
> +    xmlNodePtr *interfaces = NULL;
> +    int ninterfaces;
> +
> +    virCheckFlags(0, NULL);
> +
> +    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> +        return NULL;
> +
> +    if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0)
> +        goto cleanup;
> +
> +    if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
> +        goto cleanup;
> +
> +    /* normalize the mac addr */
> +    if (virMacAddrParse(iface, &macaddr) == 0)
> +        virMacAddrFormat(&macaddr, macstr);
> +
> +    if (virAsprintf(&xpath, "/domain/devices/interface[(mac/@address = '%s') or "
> +                            "                          (target/@dev = '%s')]",
> +                           macstr, iface) < 0)
> +        goto cleanup;
> +
> +    if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0)
> +        goto cleanup;
> +
> +    if (ninterfaces != 1)
> +        goto cleanup;
> +
> +    ctxt->node = interfaces[0];
> +
> +    if (VIR_ALLOC_N(ret, 2) < 0)
> +        goto error;
> +
> +    if ((state = virXPathString("string(./link/@state)", ctxt)) &&
> +        STREQ(state, "down")) {
> +        if (VIR_STRDUP(ret[0], "up") < 0)
> +            goto error;
> +    } else {
> +        if (VIR_STRDUP(ret[0], "down") < 0)
> +            goto error;
> +    }
> +
> + cleanup:
> +    VIR_FREE(state);
> +    VIR_FREE(interfaces);
> +    VIR_FREE(xpath);
> +    xmlXPathFreeContext(ctxt);
> +    xmlFreeDoc(xml);
> +    return ret;
> +
> + error:
> +    virStringListFree(ret);
> +    ret = NULL;
> +    goto cleanup;
> +}
> +
>  


Fixed all the small nits I've found, ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list