From: Chun Feng Wu <wucf@linux.ibm.com>

* Add new cmds: throttlegroupset, throttlegrouplist, throttlegroupinfo, throttlegroupdel

Signed-off-by: Chun Feng Wu <wucf@linux.ibm.com>
---
 tools/virsh-domain.c | 428 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 428 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 50e80689a2..f84a65451a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1509,6 +1509,410 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
     goto cleanup;
 }
 
+
+/*
+ * "throttlegrouplist" command
+ */
+static const vshCmdInfo info_throttlegrouplist = {
+    .help = N_("list all domain throttlegroups."),
+    .desc = N_("Get the summary of throttle groups for a domain."),
+};
+
+
+static const vshCmdOptDef opts_throttlegrouplist[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "inactive",
+     .type = VSH_OT_BOOL,
+     .help = N_("get inactive rather than running configuration")
+    },
+    {.name = NULL}
+};
+
+
+static bool
+cmdThrottleGroupList(vshControl *ctl,
+                     const vshCmd *cmd)
+{
+    unsigned int flags = 0;
+    size_t i;
+    g_autoptr(xmlDoc) xml = NULL;
+    g_autoptr(xmlXPathContext) ctxt = NULL;
+    g_autofree xmlNodePtr *groups = NULL;
+    ssize_t ngroups;
+    g_autoptr(vshTable) table = NULL;
+
+    if (vshCommandOptBool(cmd, "inactive"))
+        flags |= VIR_DOMAIN_XML_INACTIVE;
+
+    if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
+        return false;
+
+    ngroups = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt, &groups);
+    if (ngroups < 0)
+        return false;
+
+    table = vshTableNew(_("Name"), NULL);
+
+    if (!table)
+        return false;
+
+    for (i = 0; i < ngroups; i++) {
+        g_autofree char *name = NULL;
+        ctxt->node = groups[i];
+        name = virXPathString("string(./group_name)", ctxt);
+        if (vshTableRowAppend(table, name, NULL) < 0)
+            return false;
+    }
+
+    vshTablePrintToStdout(table, ctl);
+
+    return true;
+}
+
+
+/*
+ * "throttlegroupset" command
+ */
+static const vshCmdInfo info_throttlegroupset = {
+    .help = N_("Add or update a throttling group."),
+    .desc = N_("Add or updte a throttling group."),
+};
+
+
+static const vshCmdOptDef opts_throttlegroupset[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "group-name",
+     .type = VSH_OT_STRING,
+     .positional = true,
+     .required = true,
+     .completer = virshCompleteEmpty,
+     .help = N_("throttle group name")
+    },
+    {.name = "total-bytes-sec",
+     .type = VSH_OT_INT,
+     .help = N_("total throughput limit, as scaled integer (default bytes)")
+    },
+    {.name = "read-bytes-sec",
+     .type = VSH_OT_INT,
+     .help = N_("read throughput limit, as scaled integer (default bytes)")
+    },
+    {.name = "write-bytes-sec",
+     .type = VSH_OT_INT,
+     .help =  N_("write throughput limit, as scaled integer (default bytes)")
+    },
+    {.name = "total-iops-sec",
+     .type = VSH_OT_INT,
+     .help = N_("total I/O operations limit per second")
+    },
+    {.name = "read-iops-sec",
+     .type = VSH_OT_INT,
+     .help = N_("read I/O operations limit per second")
+    },
+    {.name = "write-iops-sec",
+     .type = VSH_OT_INT,
+     .help = N_("write I/O operations limit per second")
+    },
+    {.name = "total-bytes-sec-max",
+     .type = VSH_OT_INT,
+     .help = N_("total max, as scaled integer (default bytes)")
+    },
+    {.name = "read-bytes-sec-max",
+     .type = VSH_OT_INT,
+     .help = N_("read max, as scaled integer (default bytes)")
+    },
+    {.name = "write-bytes-sec-max",
+     .type = VSH_OT_INT,
+     .help = N_("write max, as scaled integer (default bytes)")
+    },
+    {.name = "total-iops-sec-max",
+     .type = VSH_OT_INT,
+     .help = N_("total I/O operations max")
+    },
+    {.name = "read-iops-sec-max",
+     .type = VSH_OT_INT,
+     .help = N_("read I/O operations max")
+    },
+    {.name = "write-iops-sec-max",
+     .type = VSH_OT_INT,
+     .help = N_("write I/O operations max")
+    },
+    {.name = "size-iops-sec",
+     .type = VSH_OT_INT,
+     .help = N_("I/O size in bytes")
+    },
+    {.name = "total-bytes-sec-max-length",
+     .type = VSH_OT_INT,
+     .help = N_("duration in seconds to allow total max bytes")
+    },
+    {.name = "read-bytes-sec-max-length",
+     .type = VSH_OT_INT,
+     .help = N_("duration in seconds to allow read max bytes")
+    },
+    {.name = "write-bytes-sec-max-length",
+     .type = VSH_OT_INT,
+     .help = N_("duration in seconds to allow write max bytes")
+    },
+    {.name = "total-iops-sec-max-length",
+     .type = VSH_OT_INT,
+     .help = N_("duration in seconds to allow total I/O operations max")
+    },
+    {.name = "read-iops-sec-max-length",
+     .type = VSH_OT_INT,
+     .help = N_("duration in seconds to allow read I/O operations max")
+    },
+    {.name = "write-iops-sec-max-length",
+     .type = VSH_OT_INT,
+     .help = N_("duration in seconds to allow write I/O operations max")
+    },
+    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+    VIRSH_COMMON_OPT_DOMAIN_LIVE,
+    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+    {.name = NULL}
+};
+
+
+static bool
+cmdThrottleGroupSet(vshControl *ctl,
+                    const vshCmd *cmd)
+{
+    g_autoptr(virshDomain) dom = NULL;
+    const char *name;
+    const char *group_name = NULL;
+    unsigned long long value;
+    int nparams = 0;
+    int maxparams = 0;
+    virTypedParameterPtr params = NULL;
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+    int rv = 0;
+    bool current = vshCommandOptBool(cmd, "current");
+    bool config = vshCommandOptBool(cmd, "config");
+    bool live = vshCommandOptBool(cmd, "live");
+    bool ret = false;
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+    if (config)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
+        goto cleanup;
+
+
+#define VSH_SET_THROTTLE_GROUP_SCALED(PARAM, CONST) \
+    if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value, \
+                                     1, ULLONG_MAX)) < 0) { \
+        goto interror; \
+    } else if (rv > 0) { \
+        if (virTypedParamsAddULLong(&params, &nparams, &maxparams, \
+                                    VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
+                                    value) < 0) \
+            goto save_error; \
+    }
+
+    VSH_SET_THROTTLE_GROUP_SCALED(total-bytes-sec, TOTAL_BYTES_SEC);
+    VSH_SET_THROTTLE_GROUP_SCALED(read-bytes-sec, READ_BYTES_SEC);
+    VSH_SET_THROTTLE_GROUP_SCALED(write-bytes-sec, WRITE_BYTES_SEC);
+    VSH_SET_THROTTLE_GROUP_SCALED(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX);
+    VSH_SET_THROTTLE_GROUP_SCALED(read-bytes-sec-max, READ_BYTES_SEC_MAX);
+    VSH_SET_THROTTLE_GROUP_SCALED(write-bytes-sec-max, WRITE_BYTES_SEC_MAX);
+#undef VSH_SET_THROTTLE_GROUP_SCALED
+
+#define VSH_SET_THROTTLE_GROUP(PARAM, CONST) \
+    if ((rv = vshCommandOptULongLong(ctl, cmd, #PARAM, &value)) < 0) { \
+        goto interror; \
+    } else if (rv > 0) { \
+        if (virTypedParamsAddULLong(&params, &nparams, &maxparams, \
+                                    VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
+                                    value) < 0) \
+            goto save_error; \
+    }
+
+    VSH_SET_THROTTLE_GROUP(total-iops-sec, TOTAL_IOPS_SEC);
+    VSH_SET_THROTTLE_GROUP(read-iops-sec, READ_IOPS_SEC);
+    VSH_SET_THROTTLE_GROUP(write-iops-sec, WRITE_IOPS_SEC);
+    VSH_SET_THROTTLE_GROUP(total-iops-sec-max, TOTAL_IOPS_SEC_MAX);
+    VSH_SET_THROTTLE_GROUP(read-iops-sec-max, READ_IOPS_SEC_MAX);
+    VSH_SET_THROTTLE_GROUP(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
+    VSH_SET_THROTTLE_GROUP(size-iops-sec, SIZE_IOPS_SEC);
+
+    VSH_SET_THROTTLE_GROUP(total-bytes-sec-max-length, TOTAL_BYTES_SEC_MAX_LENGTH);
+    VSH_SET_THROTTLE_GROUP(read-bytes-sec-max-length, READ_BYTES_SEC_MAX_LENGTH);
+    VSH_SET_THROTTLE_GROUP(write-bytes-sec-max-length, WRITE_BYTES_SEC_MAX_LENGTH);
+    VSH_SET_THROTTLE_GROUP(total-iops-sec-max-length, TOTAL_IOPS_SEC_MAX_LENGTH);
+    VSH_SET_THROTTLE_GROUP(read-iops-sec-max-length, READ_IOPS_SEC_MAX_LENGTH);
+    VSH_SET_THROTTLE_GROUP(write-iops-sec-max-length, WRITE_IOPS_SEC_MAX_LENGTH);
+#undef VSH_SET_THROTTLE_GROUP
+
+    if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
+        goto cleanup;
+    }
+
+    if (group_name) {
+        if (virTypedParamsAddString(&params, &nparams, &maxparams,
+                                    VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME,
+                                    group_name) < 0)
+            goto save_error;
+    }
+
+    if (virDomainSetThrottleGroup(dom, group_name, params, nparams, flags) < 0)
+        goto error;
+    vshPrintExtra(ctl, "%s", _("Throttle group set successfully\n"));
+
+    ret = true;
+
+ cleanup:
+    virTypedParamsFree(params, nparams);
+    return ret;
+
+ save_error:
+    vshSaveLibvirtError();
+ error:
+    vshError(ctl, "%s", _("Unable to change throttle group"));
+    goto cleanup;
+
+ interror:
+    vshError(ctl, "%s", _("Unable to parse integer parameter"));
+    goto cleanup;
+}
+
+
+/*
+ * "throttlegroupdel" command
+ */
+static const vshCmdInfo info_throttlegroupdel = {
+    .help = N_("Delete a throttling group."),
+    .desc = N_("Delete a throttling group."),
+};
+
+
+static const vshCmdOptDef opts_throttlegroupdel[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "group-name",
+     .type = VSH_OT_STRING,
+     .positional = true,
+     .required = true,
+     .completer = virshCompleteEmpty,
+     .help = N_("throttle group name")
+    },
+    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+    VIRSH_COMMON_OPT_DOMAIN_LIVE,
+    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+    {.name = NULL}
+};
+
+
+static bool
+cmdThrottleGroupDel(vshControl *ctl,
+                    const vshCmd *cmd)
+{
+    g_autoptr(virshDomain) dom = NULL;
+    const char *group_name = NULL;
+    bool config = vshCommandOptBool(cmd, "config");
+    bool live = vshCommandOptBool(cmd, "live");
+    bool current = vshCommandOptBool(cmd, "current");
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+    if (config)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+
+    if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
+        return false;
+    }
+
+    if (virDomainDelThrottleGroup(dom, group_name, flags) < 0)
+        return false;
+    vshPrintExtra(ctl, "%s", _("Throttle group deleted successfully\n"));
+
+    return true;
+}
+
+
+/*
+ * "throttlegroupinfo" command
+ */
+static const vshCmdInfo info_throttlegroupinfo = {
+    .help = N_("Get a throttling group."),
+    .desc = N_("Get a throttling group."),
+};
+
+
+static const vshCmdOptDef opts_throttlegroupinfo[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "group-name",
+     .type = VSH_OT_STRING,
+     .positional = true,
+     .required = true,
+     .completer = virshCompleteEmpty,
+     .help = N_("throttle group name")
+    },
+    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+    VIRSH_COMMON_OPT_DOMAIN_LIVE,
+    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+    {.name = NULL}
+};
+
+
+static bool
+cmdThrottleGroupInfo(vshControl *ctl,
+                     const vshCmd *cmd)
+{
+    g_autoptr(virshDomain) dom = NULL;
+    const char *name;
+    const char *group_name = NULL;
+    int nparams = 0;
+    virTypedParameterPtr params = NULL;
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+    size_t i;
+    bool current = vshCommandOptBool(cmd, "current");
+    bool config = vshCommandOptBool(cmd, "config");
+    bool live = vshCommandOptBool(cmd, "live");
+    bool ret = false;
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+    if (config)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
+        goto cleanup;
+
+    if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
+        goto cleanup;
+    }
+
+    if (virDomainGetThrottleGroup(dom, group_name, &params, &nparams, flags) != 0) {
+        vshError(ctl, "%s",
+                    _("Unable to get throttle group parameters"));
+        goto cleanup;
+    }
+
+    for (i = 0; i < nparams; i++) {
+        g_autofree char *str = vshGetTypedParamValue(ctl, &params[i]);
+        vshPrint(ctl, "%-15s: %s\n", params[i].field, str);
+    }
+
+    ret = true;
+
+ cleanup:
+    virTypedParamsFree(params, nparams);
+    return ret;
+}
+
+
 /*
  * "blkiotune" command
  */
@@ -13378,6 +13782,30 @@ const vshCmdDef domManagementCmds[] = {
      .info = &info_blkdeviotune,
      .flags = 0
     },
+    {.name = "throttlegroupset",
+     .handler = cmdThrottleGroupSet,
+     .opts = opts_throttlegroupset,
+     .info = &info_throttlegroupset,
+     .flags = 0
+    },
+    {.name = "throttlegroupdel",
+     .handler = cmdThrottleGroupDel,
+     .opts = opts_throttlegroupdel,
+     .info = &info_throttlegroupdel,
+     .flags = 0
+    },
+    {.name = "throttlegroupinfo",
+     .handler = cmdThrottleGroupInfo,
+     .opts = opts_throttlegroupinfo,
+     .info = &info_throttlegroupinfo,
+     .flags = 0
+    },
+    {.name = "throttlegrouplist",
+     .handler = cmdThrottleGroupList,
+     .opts = opts_throttlegrouplist,
+     .info = &info_throttlegrouplist,
+     .flags = 0
+    },
     {.name = "blkiotune",
      .handler = cmdBlkiotune,
      .opts = opts_blkiotune,
-- 
2.34.1

On Wed, Jun 12, 2024 at 03:02:23 -0700, wucf@linux.ibm.com wrote:
> From: Chun Feng Wu <wucf@linux.ibm.com>
> 
> * Add new cmds: throttlegroupset, throttlegrouplist, throttlegroupinfo, throttlegroupdel
> 
> Signed-off-by: Chun Feng Wu <wucf@linux.ibm.com>
> ---
>  tools/virsh-domain.c | 428 +++++++++++++++++++++++++++++++++++++++++++

This patch is missing addition to the manpage documenting the commands
in docs/manpages/virsh.rst.

>  1 file changed, 428 insertions(+)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 50e80689a2..f84a65451a 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1509,6 +1509,410 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
>      goto cleanup;
>  }
>  
> +
> +/*
> + * "throttlegrouplist" command
> + */
> +static const vshCmdInfo info_throttlegrouplist = {
> +    .help = N_("list all domain throttlegroups."),
> +    .desc = N_("Get the summary of throttle groups for a domain."),
> +};
> +
> +
> +static const vshCmdOptDef opts_throttlegrouplist[] = {
> +    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> +    {.name = "inactive",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("get inactive rather than running configuration")
> +    },
> +    {.name = NULL}
> +};
> +
> +
> +static bool
> +cmdThrottleGroupList(vshControl *ctl,
> +                     const vshCmd *cmd)
> +{
> +    unsigned int flags = 0;
> +    size_t i;
> +    g_autoptr(xmlDoc) xml = NULL;
> +    g_autoptr(xmlXPathContext) ctxt = NULL;
> +    g_autofree xmlNodePtr *groups = NULL;
> +    ssize_t ngroups;
> +    g_autoptr(vshTable) table = NULL;
> +
> +    if (vshCommandOptBool(cmd, "inactive"))
> +        flags |= VIR_DOMAIN_XML_INACTIVE;
> +
> +    if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
> +        return false;
> +
> +    ngroups = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt, &groups);
> +    if (ngroups < 0)

You need to report an error here, but on the other hand I don't think
that having 0 groups is an error.

> +        return false;
> +
> +    table = vshTableNew(_("Name"), NULL);
> +
> +    if (!table)
> +        return false;
> +
> +    for (i = 0; i < ngroups; i++) {
> +        g_autofree char *name = NULL;
> +        ctxt->node = groups[i];
> +        name = virXPathString("string(./group_name)", ctxt);

Here you don't handle NULL 

> +        if (vshTableRowAppend(table, name, NULL) < 0)

... which would make this fail use NULLSTR_EMPTY

> +            return false;
> +    }
> +
> +    vshTablePrintToStdout(table, ctl);
> +
> +    return true;
> +}
> +
> +
> +/*
> + * "throttlegroupset" command
> + */
> +static const vshCmdInfo info_throttlegroupset = {
> +    .help = N_("Add or update a throttling group."),
> +    .desc = N_("Add or updte a throttling group."),
> +};
> +
> +
> +static const vshCmdOptDef opts_throttlegroupset[] = {
> +    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> +    {.name = "group-name",
> +     .type = VSH_OT_STRING,
> +     .positional = true,
> +     .required = true,
> +     .completer = virshCompleteEmpty,

It will be possible to auto-complete this argument so virshCompleteEmpty
is not appropriate. That annotation must be used exclusively for fields
which don't make sense tot be complted.

> +     .help = N_("throttle group name")
> +    },
> +    {.name = "total-bytes-sec",
> +     .type = VSH_OT_INT,
> +     .help = N_("total throughput limit, as scaled integer (default bytes)")
> +    },
> +    {.name = "read-bytes-sec",
> +     .type = VSH_OT_INT,
> +     .help = N_("read throughput limit, as scaled integer (default bytes)")
> +    },
> +    {.name = "write-bytes-sec",
> +     .type = VSH_OT_INT,
> +     .help =  N_("write throughput limit, as scaled integer (default bytes)")
> +    },
> +    {.name = "total-iops-sec",
> +     .type = VSH_OT_INT,
> +     .help = N_("total I/O operations limit per second")
> +    },
> +    {.name = "read-iops-sec",
> +     .type = VSH_OT_INT,
> +     .help = N_("read I/O operations limit per second")
> +    },
> +    {.name = "write-iops-sec",
> +     .type = VSH_OT_INT,
> +     .help = N_("write I/O operations limit per second")
> +    },
> +    {.name = "total-bytes-sec-max",
> +     .type = VSH_OT_INT,
> +     .help = N_("total max, as scaled integer (default bytes)")
> +    },
> +    {.name = "read-bytes-sec-max",
> +     .type = VSH_OT_INT,
> +     .help = N_("read max, as scaled integer (default bytes)")
> +    },
> +    {.name = "write-bytes-sec-max",
> +     .type = VSH_OT_INT,
> +     .help = N_("write max, as scaled integer (default bytes)")
> +    },
> +    {.name = "total-iops-sec-max",
> +     .type = VSH_OT_INT,
> +     .help = N_("total I/O operations max")
> +    },
> +    {.name = "read-iops-sec-max",
> +     .type = VSH_OT_INT,
> +     .help = N_("read I/O operations max")
> +    },
> +    {.name = "write-iops-sec-max",
> +     .type = VSH_OT_INT,
> +     .help = N_("write I/O operations max")
> +    },
> +    {.name = "size-iops-sec",
> +     .type = VSH_OT_INT,
> +     .help = N_("I/O size in bytes")
> +    },
> +    {.name = "total-bytes-sec-max-length",
> +     .type = VSH_OT_INT,
> +     .help = N_("duration in seconds to allow total max bytes")
> +    },
> +    {.name = "read-bytes-sec-max-length",
> +     .type = VSH_OT_INT,
> +     .help = N_("duration in seconds to allow read max bytes")
> +    },
> +    {.name = "write-bytes-sec-max-length",
> +     .type = VSH_OT_INT,
> +     .help = N_("duration in seconds to allow write max bytes")
> +    },
> +    {.name = "total-iops-sec-max-length",
> +     .type = VSH_OT_INT,
> +     .help = N_("duration in seconds to allow total I/O operations max")
> +    },
> +    {.name = "read-iops-sec-max-length",
> +     .type = VSH_OT_INT,
> +     .help = N_("duration in seconds to allow read I/O operations max")
> +    },
> +    {.name = "write-iops-sec-max-length",
> +     .type = VSH_OT_INT,
> +     .help = N_("duration in seconds to allow write I/O operations max")
> +    },

I guess that once we have two of the commands using these it'd make
sense to have these as a macro to avoid having to update two places the
next time another tunable is added.

> +    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> +    VIRSH_COMMON_OPT_DOMAIN_LIVE,
> +    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> +    {.name = NULL}
> +};
> +
> +
> +static bool
> +cmdThrottleGroupSet(vshControl *ctl,
> +                    const vshCmd *cmd)
> +{
> +    g_autoptr(virshDomain) dom = NULL;
> +    const char *name;
> +    const char *group_name = NULL;
> +    unsigned long long value;
> +    int nparams = 0;
> +    int maxparams = 0;
> +    virTypedParameterPtr params = NULL;
> +    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> +    int rv = 0;
> +    bool current = vshCommandOptBool(cmd, "current");
> +    bool config = vshCommandOptBool(cmd, "config");
> +    bool live = vshCommandOptBool(cmd, "live");
> +    bool ret = false;
> +
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> +
> +    if (config)
> +        flags |= VIR_DOMAIN_AFFECT_CONFIG;
> +    if (live)
> +        flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
> +    if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
> +        goto cleanup;

This function doesn't use 'name' any more so there's no point in
fetching it.

> +
> +
> +#define VSH_SET_THROTTLE_GROUP_SCALED(PARAM, CONST) \
> +    if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value, \
> +                                     1, ULLONG_MAX)) < 0) { \
> +        goto interror; \
> +    } else if (rv > 0) { \
> +        if (virTypedParamsAddULLong(&params, &nparams, &maxparams, \
> +                                    VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
> +                                    value) < 0) \
> +            goto save_error; \
> +    }
> +
> +    VSH_SET_THROTTLE_GROUP_SCALED(total-bytes-sec, TOTAL_BYTES_SEC);
> +    VSH_SET_THROTTLE_GROUP_SCALED(read-bytes-sec, READ_BYTES_SEC);
> +    VSH_SET_THROTTLE_GROUP_SCALED(write-bytes-sec, WRITE_BYTES_SEC);
> +    VSH_SET_THROTTLE_GROUP_SCALED(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX);
> +    VSH_SET_THROTTLE_GROUP_SCALED(read-bytes-sec-max, READ_BYTES_SEC_MAX);
> +    VSH_SET_THROTTLE_GROUP_SCALED(write-bytes-sec-max, WRITE_BYTES_SEC_MAX);
> +#undef VSH_SET_THROTTLE_GROUP_SCALED
> +
> +#define VSH_SET_THROTTLE_GROUP(PARAM, CONST) \
> +    if ((rv = vshCommandOptULongLong(ctl, cmd, #PARAM, &value)) < 0) { \
> +        goto interror; \
> +    } else if (rv > 0) { \
> +        if (virTypedParamsAddULLong(&params, &nparams, &maxparams, \
> +                                    VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
> +                                    value) < 0) \
> +            goto save_error; \
> +    }
> +
> +    VSH_SET_THROTTLE_GROUP(total-iops-sec, TOTAL_IOPS_SEC);
> +    VSH_SET_THROTTLE_GROUP(read-iops-sec, READ_IOPS_SEC);
> +    VSH_SET_THROTTLE_GROUP(write-iops-sec, WRITE_IOPS_SEC);
> +    VSH_SET_THROTTLE_GROUP(total-iops-sec-max, TOTAL_IOPS_SEC_MAX);
> +    VSH_SET_THROTTLE_GROUP(read-iops-sec-max, READ_IOPS_SEC_MAX);
> +    VSH_SET_THROTTLE_GROUP(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
> +    VSH_SET_THROTTLE_GROUP(size-iops-sec, SIZE_IOPS_SEC);
> +
> +    VSH_SET_THROTTLE_GROUP(total-bytes-sec-max-length, TOTAL_BYTES_SEC_MAX_LENGTH);
> +    VSH_SET_THROTTLE_GROUP(read-bytes-sec-max-length, READ_BYTES_SEC_MAX_LENGTH);
> +    VSH_SET_THROTTLE_GROUP(write-bytes-sec-max-length, WRITE_BYTES_SEC_MAX_LENGTH);
> +    VSH_SET_THROTTLE_GROUP(total-iops-sec-max-length, TOTAL_IOPS_SEC_MAX_LENGTH);
> +    VSH_SET_THROTTLE_GROUP(read-iops-sec-max-length, READ_IOPS_SEC_MAX_LENGTH);
> +    VSH_SET_THROTTLE_GROUP(write-iops-sec-max-length, WRITE_IOPS_SEC_MAX_LENGTH);
> +#undef VSH_SET_THROTTLE_GROUP
> +
> +    if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
> +        goto cleanup;
> +    }
> +
> +    if (group_name) {

gropu name is guaranteed to exist ...

> +        if (virTypedParamsAddString(&params, &nparams, &maxparams,
> +                                    VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME,
> +                                    group_name) < 0)
> +            goto save_error;
> +    }
> +
> +    if (virDomainSetThrottleGroup(dom, group_name, params, nparams, flags) < 0)
> +        goto error;

otherwis this'd fail anyways.

> +    vshPrintExtra(ctl, "%s", _("Throttle group set successfully\n"));
> +
> +    ret = true;
> +
> + cleanup:
> +    virTypedParamsFree(params, nparams);
> +    return ret;
> +
> + save_error:
> +    vshSaveLibvirtError();
> + error:
> +    vshError(ctl, "%s", _("Unable to change throttle group"));
> +    goto cleanup;
> +
> + interror:
> +    vshError(ctl, "%s", _("Unable to parse integer parameter"));
> +    goto cleanup;
> +}
> +
> +
> +/*
> + * "throttlegroupdel" command
> + */
> +static const vshCmdInfo info_throttlegroupdel = {
> +    .help = N_("Delete a throttling group."),
> +    .desc = N_("Delete a throttling group."),
> +};
> +
> +
> +static const vshCmdOptDef opts_throttlegroupdel[] = {
> +    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> +    {.name = "group-name",
> +     .type = VSH_OT_STRING,
> +     .positional = true,
> +     .required = true,
> +     .completer = virshCompleteEmpty,

Same as above regarding the annotation.

> +     .help = N_("throttle group name")
> +    },
> +    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> +    VIRSH_COMMON_OPT_DOMAIN_LIVE,
> +    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> +    {.name = NULL}
> +};
> +
> +
> +static bool
> +cmdThrottleGroupDel(vshControl *ctl,
> +                    const vshCmd *cmd)
> +{
> +    g_autoptr(virshDomain) dom = NULL;
> +    const char *group_name = NULL;
> +    bool config = vshCommandOptBool(cmd, "config");
> +    bool live = vshCommandOptBool(cmd, "live");
> +    bool current = vshCommandOptBool(cmd, "current");
> +    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> +
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> +
> +    if (config)
> +        flags |= VIR_DOMAIN_AFFECT_CONFIG;
> +    if (live)
> +        flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
> +    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> +        return false;
> +
> +    if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
> +        return false;
> +    }
> +
> +    if (virDomainDelThrottleGroup(dom, group_name, flags) < 0)
> +        return false;
> +    vshPrintExtra(ctl, "%s", _("Throttle group deleted successfully\n"));
> +
> +    return true;
> +}
> +
> +
> +/*
> + * "throttlegroupinfo" command
> + */
> +static const vshCmdInfo info_throttlegroupinfo = {
> +    .help = N_("Get a throttling group."),
> +    .desc = N_("Get a throttling group."),
> +};
> +
> +
> +static const vshCmdOptDef opts_throttlegroupinfo[] = {
> +    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> +    {.name = "group-name",
> +     .type = VSH_OT_STRING,
> +     .positional = true,
> +     .required = true,
> +     .completer = virshCompleteEmptya

Same as above.
,
> +     .help = N_("throttle group name")
> +    },
> +    VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> +    VIRSH_COMMON_OPT_DOMAIN_LIVE,
> +    VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> +    {.name = NULL}
> +};
> +
> +
> +static bool
> +cmdThrottleGroupInfo(vshControl *ctl,
> +                     const vshCmd *cmd)
> +{
> +    g_autoptr(virshDomain) dom = NULL;
> +    const char *name;
> +    const char *group_name = NULL;
> +    int nparams = 0;
> +    virTypedParameterPtr params = NULL;
> +    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> +    size_t i;
> +    bool current = vshCommandOptBool(cmd, "current");
> +    bool config = vshCommandOptBool(cmd, "config");
> +    bool live = vshCommandOptBool(cmd, "live");
> +    bool ret = false;
> +
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> +
> +    if (config)
> +        flags |= VIR_DOMAIN_AFFECT_CONFIG;
> +    if (live)
> +        flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
> +    if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
> +        goto cleanup;

Name is not used.

> +
> +    if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
> +        goto cleanup;
> +    }
> +
> +    if (virDomainGetThrottleGroup(dom, group_name, &params, &nparams, flags) != 0) {
> +        vshError(ctl, "%s",
> +                    _("Unable to get throttle group parameters"));

Misaligned.

> +        goto cleanup;
> +    }
> +
> +    for (i = 0; i < nparams; i++) {
> +        g_autofree char *str = vshGetTypedParamValue(ctl, &params[i]);
> +        vshPrint(ctl, "%-15s: %s\n", params[i].field, str);
> +    }
> +
> +    ret = true;
> +
> + cleanup:
> +    virTypedParamsFree(params, nparams);
> +    return ret;
> +}
> +
> +
>  /*
>   * "blkiotune" command
>   */
> @@ -13378,6 +13782,30 @@ const vshCmdDef domManagementCmds[] = {
>       .info = &info_blkdeviotune,
>       .flags = 0
>      },
> +    {.name = "throttlegroupset",
> +     .handler = cmdThrottleGroupSet,
> +     .opts = opts_throttlegroupset,
> +     .info = &info_throttlegroupset,
> +     .flags = 0
> +    },
> +    {.name = "throttlegroupdel",
> +     .handler = cmdThrottleGroupDel,
> +     .opts = opts_throttlegroupdel,
> +     .info = &info_throttlegroupdel,
> +     .flags = 0
> +    },
> +    {.name = "throttlegroupinfo",
> +     .handler = cmdThrottleGroupInfo,
> +     .opts = opts_throttlegroupinfo,
> +     .info = &info_throttlegroupinfo,
> +     .flags = 0
> +    },
> +    {.name = "throttlegrouplist",
> +     .handler = cmdThrottleGroupList,
> +     .opts = opts_throttlegrouplist,
> +     .info = &info_throttlegrouplist,
> +     .flags = 0
> +    },

I suggest using the 'dom' prefix as all of the objects are per-domain.

>      {.name = "blkiotune",
>       .handler = cmdBlkiotune,
>       .opts = opts_blkiotune,
> -- 
> 2.34.1
>