[libvirt] [PATCH 02/19] virsh: Add logical CPU IDs completion for nodecpustats command

Lin Ma posted 19 patches 5 years, 3 months ago
There is a newer version of this series
[libvirt] [PATCH 02/19] virsh: Add logical CPU IDs completion for nodecpustats command
Posted by Lin Ma 5 years, 3 months ago
Signed-off-by: Lin Ma <lma@suse.com>
---
 tools/virsh-completer-host.c | 35 +++++++++++++++++++++++++++++++++++
 tools/virsh-completer-host.h |  4 ++++
 tools/virsh-host.c           |  1 +
 3 files changed, 40 insertions(+)

diff --git a/tools/virsh-completer-host.c b/tools/virsh-completer-host.c
index 339390aa00..a74578d2e2 100644
--- a/tools/virsh-completer-host.c
+++ b/tools/virsh-completer-host.c
@@ -136,3 +136,38 @@ virshCellnoCompleter(vshControl *ctl,
 
     return g_steal_pointer(&tmp);
 }
+
+
+char **
+virshCpuCompleter(vshControl *ctl,
+                  const vshCmd *cmd G_GNUC_UNUSED,
+                  unsigned int flags)
+{
+    int i, cpuid = 0, cpunum, offset = 0;
+    unsigned int online;
+    g_autofree unsigned char *cpumap = NULL;
+    char **ret = NULL;
+    VIR_AUTOSTRINGLIST tmp = NULL;
+    virshControlPtr priv = ctl->privData;
+
+    virCheckFlags(0, NULL);
+
+    if ((cpunum = virNodeGetCPUMap(priv->conn, &cpumap, &online, 0)) < 0)
+        return NULL;
+
+    tmp = g_new0(char *, online + 1);
+
+    for (i = 0; i < cpunum; i++) {
+        if (VIR_CPU_USED(cpumap, cpuid) == 0) {
+            offset += 1;
+            cpuid += 1;
+            continue;
+        } else {
+            tmp[i - offset] = g_strdup_printf("%u", cpuid++);
+        }
+    }
+
+    ret = g_steal_pointer(&tmp);
+
+    return ret;
+}
diff --git a/tools/virsh-completer-host.h b/tools/virsh-completer-host.h
index 921beb7a2d..777783b908 100644
--- a/tools/virsh-completer-host.h
+++ b/tools/virsh-completer-host.h
@@ -29,3 +29,7 @@ char ** virshAllocpagesPagesizeCompleter(vshControl *ctl,
 char ** virshCellnoCompleter(vshControl *ctl,
                              const vshCmd *cmd,
                              unsigned int flags);
+
+char ** virshCpuCompleter(vshControl *ctl,
+                          const vshCmd *cmd,
+                          unsigned int flags);
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index cda2ef4ac1..4774f76ed8 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -751,6 +751,7 @@ static const vshCmdInfo info_nodecpustats[] = {
 static const vshCmdOptDef opts_node_cpustats[] = {
     {.name = "cpu",
      .type = VSH_OT_INT,
+     .completer = virshCpuCompleter,
      .help = N_("prints specified cpu statistics only.")
     },
     {.name = "percent",
-- 
2.26.0


Re: [libvirt] [PATCH 02/19] virsh: Add logical CPU IDs completion for nodecpustats command
Posted by Michal Privoznik 5 years, 3 months ago
On 11/2/20 9:26 AM, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>   tools/virsh-completer-host.c | 35 +++++++++++++++++++++++++++++++++++
>   tools/virsh-completer-host.h |  4 ++++
>   tools/virsh-host.c           |  1 +
>   3 files changed, 40 insertions(+)
> 
> diff --git a/tools/virsh-completer-host.c b/tools/virsh-completer-host.c
> index 339390aa00..a74578d2e2 100644
> --- a/tools/virsh-completer-host.c
> +++ b/tools/virsh-completer-host.c
> @@ -136,3 +136,38 @@ virshCellnoCompleter(vshControl *ctl,
>   
>       return g_steal_pointer(&tmp);
>   }
> +
> +
> +char **
> +virshCpuCompleter(vshControl *ctl,
> +                  const vshCmd *cmd G_GNUC_UNUSED,
> +                  unsigned int flags)
> +{
> +    int i, cpuid = 0, cpunum, offset = 0;
> +    unsigned int online;
> +    g_autofree unsigned char *cpumap = NULL;
> +    char **ret = NULL;
> +    VIR_AUTOSTRINGLIST tmp = NULL;
> +    virshControlPtr priv = ctl->privData;
> +
> +    virCheckFlags(0, NULL);
> +
> +    if ((cpunum = virNodeGetCPUMap(priv->conn, &cpumap, &online, 0)) < 0)
> +        return NULL;
> +
> +    tmp = g_new0(char *, online + 1);
> +
> +    for (i = 0; i < cpunum; i++) {
> +        if (VIR_CPU_USED(cpumap, cpuid) == 0) {
> +            offset += 1;
> +            cpuid += 1;
> +            continue;
> +        } else {
> +            tmp[i - offset] = g_strdup_printf("%u", cpuid++);
> +        }
> +    }

This loop is needlessly complicated.

> +
> +    ret = g_steal_pointer(&tmp);

@ret can be dropped.
> +
> +    return ret;
> +}

ACK with this squashed in:

diff --git i/tools/virsh-completer-host.c w/tools/virsh-completer-host.c
index a74578d2e2..685fa23fd4 100644
--- i/tools/virsh-completer-host.c
+++ w/tools/virsh-completer-host.c
@@ -143,12 +143,13 @@ virshCpuCompleter(vshControl *ctl,
                    const vshCmd *cmd G_GNUC_UNUSED,
                    unsigned int flags)
  {
-    int i, cpuid = 0, cpunum, offset = 0;
+    virshControlPtr priv = ctl->privData;
+    VIR_AUTOSTRINGLIST tmp = NULL;
+    size_t i;
+    int cpunum;
+    size_t offset = 0;
      unsigned int online;
      g_autofree unsigned char *cpumap = NULL;
-    char **ret = NULL;
-    VIR_AUTOSTRINGLIST tmp = NULL;
-    virshControlPtr priv = ctl->privData;

      virCheckFlags(0, NULL);

@@ -158,16 +159,11 @@ virshCpuCompleter(vshControl *ctl,
      tmp = g_new0(char *, online + 1);

      for (i = 0; i < cpunum; i++) {
-        if (VIR_CPU_USED(cpumap, cpuid) == 0) {
-            offset += 1;
-            cpuid += 1;
+        if (VIR_CPU_USED(cpumap, i) == 0)
              continue;
-        } else {
-            tmp[i - offset] = g_strdup_printf("%u", cpuid++);
-        }
+
+        tmp[offset++] = g_strdup_printf("%zu", i);
      }

-    ret = g_steal_pointer(&tmp);
-
-    return ret;
+    return g_steal_pointer(&tmp);
  }


Michal