[v1] tools: virsh: Add virshCellnoCompleter.

[libvirt] [PATCH] tools: virsh: Add virshCellnoCompleter.

Posted by Roland Schulz 6 years, 11 months ago

Signed-off-by: Roland Schulz <schullzroll@gmail.com>
---
 tools/virsh-completer.c | 59 +++++++++++++++++++++++++++++++++++++++++
 tools/virsh-completer.h |  3 +++
 tools/virsh-host.c      |  3 +++
 3 files changed, 65 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 5713a887e..ce152226f 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -756,3 +756,62 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
     virStringListFree(ret);
     return NULL;
 }
+
+
+char **
+virshCellnoCompleter(vshControl *ctl,
+                     const vshCmd *cmd ATTRIBUTE_UNUSED,
+                     unsigned int flags)
+{
+    xmlXPathContextPtr ctxt = NULL;
+    virshControlPtr priv = ctl->privData;
+    unsigned int ncells = 0;
+    xmlNodePtr *cells = NULL;
+    xmlDocPtr doc = NULL;
+    size_t i = 0;
+    char *cap_xml = NULL;
+    char *cellid = NULL;
+    char **ret = NULL;
+
+    virCheckFlags(0, NULL);
+
+    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+        goto error;
+
+    if (!(cap_xml = virConnectGetCapabilities(priv->conn)))
+        goto error;
+
+    if (!(doc = virXMLParseStringCtxt(cap_xml, _("capabilities"), &ctxt)))
+        goto error;
+
+    ncells = virXPathNodeSet("/capabilities/host/topology/cells/cell", ctxt, &cells);
+    if (ncells <= 0)
+        goto error;
+
+    if (VIR_ALLOC_N(ret, ncells + 1))
+        goto error;
+
+    for (i = 0; i < ncells; i++) {
+        VIR_FREE(cellid);
+        cellid = virXMLPropString(cells[i], "id");
+        if (VIR_STRDUP(ret[i], cellid) < 0)
+            goto error;
+    }
+
+ cleanup:
+    xmlXPathFreeContext(ctxt);
+    VIR_FREE(cells);
+    xmlFreeDoc(doc);
+    VIR_FREE(cap_xml);
+    VIR_FREE(cellid);
+
+    return ret;
+
+ error:
+    if (ret) {
+        for (i = 0; i < ncells; i++)
+            VIR_FREE(ret[i]);
+    }
+    VIR_FREE(ret);
+    goto cleanup;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 86632ee91..ee7eec68c 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -94,4 +94,7 @@ char ** virshNodedevEventNameCompleter(vshControl *ctl,
                                        const vshCmd *cmd,
                                        unsigned int flags);
 
+char ** virshCellnoCompleter(vshControl *ctl,
+                             const vshCmd *cmd,
+                             unsigned int flags);
 #endif
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 12c13f522..991c0f50a 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -149,6 +149,7 @@ static const vshCmdInfo info_freecell[] = {
 static const vshCmdOptDef opts_freecell[] = {
     {.name = "cellno",
      .type = VSH_OT_INT,
+     .completer = virshCellnoCompleter,
      .help = N_("NUMA cell number")
     },
     {.name = "all",
@@ -274,6 +275,7 @@ static const vshCmdInfo info_freepages[] = {
 static const vshCmdOptDef opts_freepages[] = {
     {.name = "cellno",
      .type = VSH_OT_INT,
+     .completer = virshCellnoCompleter,
      .help = N_("NUMA cell number")
     },
     {.name = "pagesize",
@@ -483,6 +485,7 @@ static const vshCmdOptDef opts_allocpages[] = {
     },
     {.name = "cellno",
      .type = VSH_OT_INT,
+     .completer = virshCellnoCompleter,
      .help = N_("NUMA cell number")
     },
     {.name = "add",
-- 
2.17.0

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

Re: [libvirt] [PATCH] tools: virsh: Add virshCellnoCompleter.

Posted by Michal Privoznik 6 years, 11 months ago

On 05/25/2018 09:12 AM, Roland Schulz wrote:
> Signed-off-by: Roland Schulz <schullzroll@gmail.com>
> ---
>  tools/virsh-completer.c | 59 +++++++++++++++++++++++++++++++++++++++++
>  tools/virsh-completer.h |  3 +++
>  tools/virsh-host.c      |  3 +++
>  3 files changed, 65 insertions(+)
> 
> diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> index 5713a887e..ce152226f 100644
> --- a/tools/virsh-completer.c
> +++ b/tools/virsh-completer.c
> @@ -756,3 +756,62 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
>      virStringListFree(ret);
>      return NULL;
>  }
> +
> +
> +char **
> +virshCellnoCompleter(vshControl *ctl,
> +                     const vshCmd *cmd ATTRIBUTE_UNUSED,
> +                     unsigned int flags)
> +{
> +    xmlXPathContextPtr ctxt = NULL;
> +    virshControlPtr priv = ctl->privData;
> +    unsigned int ncells = 0;
> +    xmlNodePtr *cells = NULL;
> +    xmlDocPtr doc = NULL;
> +    size_t i = 0;
> +    char *cap_xml = NULL;
> +    char *cellid = NULL;
> +    char **ret = NULL;
> +
> +    virCheckFlags(0, NULL);
> +
> +    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> +        goto error;
> +
> +    if (!(cap_xml = virConnectGetCapabilities(priv->conn)))
> +        goto error;
> +
> +    if (!(doc = virXMLParseStringCtxt(cap_xml, _("capabilities"), &ctxt)))
> +        goto error;
> +
> +    ncells = virXPathNodeSet("/capabilities/host/topology/cells/cell", ctxt, &cells);
> +    if (ncells <= 0)
> +        goto error;
> +
> +    if (VIR_ALLOC_N(ret, ncells + 1))
> +        goto error;
> +
> +    for (i = 0; i < ncells; i++) {
> +        VIR_FREE(cellid);
> +        cellid = virXMLPropString(cells[i], "id");
> +        if (VIR_STRDUP(ret[i], cellid) < 0)

The virXMLPropString() already returns a dynamically allocated string
(that's why you call VIR_FREE(cellid)). There's not much value added in
duplicating the string again. What you can do is assign the returned
value to ret[i] directly:

for (i = 0; i < ncells; i++) {
  if (!(ret[i] = virXMLPropString(cells[i], "id")))
    goto error;
}

This way @cellid variable is needless.

> +            goto error;
> +    }
> +
> + cleanup:
> +    xmlXPathFreeContext(ctxt);
> +    VIR_FREE(cells);
> +    xmlFreeDoc(doc);
> +    VIR_FREE(cap_xml);
> +    VIR_FREE(cellid);
> +
> +    return ret;
> +
> + error:
> +    if (ret) {
> +        for (i = 0; i < ncells; i++)
> +            VIR_FREE(ret[i]);
> +    }
> +    VIR_FREE(ret);
> +    goto cleanup;
> +}

I'm fixing that, ACKing and pushing.

Michal

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

Re: [libvirt] [PATCH] tools: virsh: Add virshCellnoCompleter.

Posted by Ján Tomko 6 years, 11 months ago

Please do not put periods at the end of commit summaries.

On Fri, May 25, 2018 at 09:12:37AM +0200, Roland Schulz wrote:
>Signed-off-by: Roland Schulz <schullzroll@gmail.com>
>---
> tools/virsh-completer.c | 59 +++++++++++++++++++++++++++++++++++++++++
> tools/virsh-completer.h |  3 +++
> tools/virsh-host.c      |  3 +++
> 3 files changed, 65 insertions(+)

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