On Wed, Feb 19, 2025 at 22:27:21 +0530, Harikumar Rajkumar wrote:
> From: Chun Feng Wu <[email protected]>
>
> Implement new throttle cmds
>
> * Add new virsh cmds: domthrottlegroupset, domthrottlegrouplist,
> domthrottlegroupinfo, domthrottlegroupdel
> * Add doc for new cmds at docs/manpages/virsh.rst
> * Add cmd helper "virshDomainThrottleGroupCompleter", which is used by
> domthrottlegroupset, domthrottlegroupinfo, domthrottlegroupdel
>
> Signed-off-by: Chun Feng Wu <[email protected]>
>
> * Update of code documentation comments.
> * Reimplement Get throttle group from XML.
>
> Signed-off-by: Harikumar Rajkumar <[email protected]>
> ---
> docs/manpages/virsh.rst | 134 +++++++++++
> tools/virsh-completer-domain.c | 55 +++++
> tools/virsh-completer-domain.h | 11 +
> tools/virsh-domain.c | 405 ++++++++++++++++++++++++++++++++-
> 4 files changed, 604 insertions(+), 1 deletion(-)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 0eb1d6ea93..8726870cf9 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -1128,6 +1128,140 @@ given, but *--current* is exclusive. For querying
> only one of *--live*,
> is different depending on hypervisor.
>
>
> +domthrottlegroupset
> +-------------------
> +
> +**Syntax:**
> +
> +::
> +
> + domthrottlegroupset domain group-name [[--config] [--live] | [--current]]
> + [[total-bytes-sec] | [read-bytes-sec] [write-bytes-sec]]
> + [[total-iops-sec] | [read-iops-sec] [write-iops-sec]]
> + [[total-bytes-sec-max] | [read-bytes-sec-max] [write-bytes-sec-max]]
> + [[total-iops-sec-max] | [read-iops-sec-max] [write-iops-sec-max]]
> + [[total-bytes-sec-max-length] |
> + [read-bytes-sec-max-length] [write-bytes-sec-max-length]]
> + [[total-iops-sec-max-length] |
> + [read-iops-sec-max-length] [write-iops-sec-max-length]]
> + [size-iops-sec]
> +
> +Add or update a throttle group against specific *domain*.
> +*group-name* specifies a unique throttle group name, which defines limit, and
> +will be referenced by drives.
> +
> +If no limit is specified, default them as all zeros, which will fail,
> +Otherwise, set limits with these flags:
> +*--total-bytes-sec* specifies total throughput limit as a scaled integer, the
> +default being bytes per second if no suffix is specified.
> +*--read-bytes-sec* specifies read throughput limit as a scaled integer, the
> +default being bytes per second if no suffix is specified.
> +*--write-bytes-sec* specifies write throughput limit as a scaled integer, the
> +default being bytes per second if no suffix is specified.
> +*--total-iops-sec* specifies total I/O operations limit per second.
> +*--read-iops-sec* specifies read I/O operations limit per second.
> +*--write-iops-sec* specifies write I/O operations limit per second.
> +*--total-bytes-sec-max* specifies maximum total throughput limit as a scaled
> +integer, the default being bytes per second if no suffix is specified
> +*--read-bytes-sec-max* specifies maximum read throughput limit as a scaled
> +integer, the default being bytes per second if no suffix is specified.
> +*--write-bytes-sec-max* specifies maximum write throughput limit as a scaled
> +integer, the default being bytes per second if no suffix is specified.
> +*--total-iops-sec-max* specifies maximum total I/O operations limit per
> second.
> +*--read-iops-sec-max* specifies maximum read I/O operations limit per second.
> +*--write-iops-sec-max* specifies maximum write I/O operations limit per
> second.
> +*--total-bytes-sec-max-length* specifies duration in seconds to allow maximum
> +total throughput limit.
> +*--read-bytes-sec-max-length* specifies duration in seconds to allow maximum
> +read throughput limit.
> +*--write-bytes-sec-max-length* specifies duration in seconds to allow maximum
> +write throughput limit.
> +*--total-iops-sec-max-length* specifies duration in seconds to allow maximum
> +total I/O operations limit.
> +*--read-iops-sec-max-length* specifies duration in seconds to allow maximum
> +read I/O operations limit.
> +*--write-iops-sec-max-length* specifies duration in seconds to allow maximum
> +write I/O operations limit.
> +*--size-iops-sec* specifies size I/O operations limit per second.
> +
> +Bytes and iops values are independent, but setting only one value (such
> +as --read-bytes-sec) resets the other two in that category to unlimited.
> +An explicit 0 also clears any limit. A non-zero value for a given total
> +cannot be mixed with non-zero values for read or write.
> +
> +It is up to the hypervisor to determine how to handle the length values.
> +For the QEMU hypervisor, if an I/O limit value or maximum value is set,
> +then the default value of 1 second will be displayed. Supplying a 0 will
> +reset the value back to the default.
> +
> +If *--live* is specified, affect a running guest.
> +If *--config* is specified, affect the next start of a persistent guest.
> +If *--current* is specified, it is equivalent to either *--live* or
> +*--config*, depending on the current state of the guest.
> +When setting the disk io parameters both *--live* and *--config*
> +are specified, both live configuration and config are updated while setting
> +the description, but *--current* is exclusive. If no flag is specified,
> behavior
> +is different depending on hypervisor.
> +
> +
> +domthrottlegroupdel
> +-------------------
> +
> +**Syntax:**
> +
> +::
> +
> + domthrottlegroupdel domain group-name [[--config] [--live] | [--current]]
> +
> +Delete a Throttlegroup from the domain using the specified *group-name*.
> +If an Throttlegroup is currently referenced by a disk resource, then the
> attempt
> +to remove the Throttlegroup will fail.
> +If the *group-name* does not exist an error will occur.
> +
> +If *--live* is specified, affect a running guest. If the guest is not
> +running an error is returned.
> +If *--config* is specified, affect the next start of a persistent guest.
> +If *--current* is specified, it is equivalent to either *--live* or
> +*--config*, depending on the current state of the guest.
> +
> +
> +domthrottlegroupinfo
> +--------------------
> +
> +**Syntax:**
> +
> +::
> +
> + domthrottlegroupinfo domain group-name [[--config] [--live] | [--current]]
> +
> +Display domain Throttlegroup information including I/O limits setting.
> +
> +If *--live* is specified, get the Throttlegroup data from the running guest.
> If
> +the guest is not running, an error is returned.
> +If *--config* is specified, get the Throttlegroup data from the next start of
> +a persistent guest.
> +If *--current* is specified or *--live* and *--config* are not specified,
> +then get the Throttlegroup data based on the current guest state, which can
> +either be live or offline.
> +If both *--live* and *--config* are specified, the *--config* option takes
> +precedence on getting the current description.
> +
> +
> +domthrottlegrouplist
> +--------------------
> +
> +**Syntax:**
> +
> +::
> +
> + domthrottlegrouplist domain [--inactive]
> +
> +Print a table showing names of all throttle groups
> +associated with *domain*. If *--inactive* is specified, query the
> +Throttlegroup data that will be used on the next boot, rather than those
> +currently in use by a running domain.
> +
> +
> blkiotune
> ---------
>
> diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> index 61362224a3..3b0df15c13 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -248,6 +248,61 @@ virshDomainMigrateDisksCompleter(vshControl *ctl,
> }
>
>
> +int
> +virshGetThrottleGroupNames(xmlXPathContext *ctxt,
> + xmlNodePtr **groups,
> + char ***groupNames)
> +{
> + int ngroups;
> + size_t i;
> +
> + ngroups = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt,
> groups);
> + if (ngroups < 0)
> + return -1;
> +
> + *groupNames = g_new0(char *, ngroups + 1);
> +
> + for (i = 0; i < ngroups; i++) {
> + ctxt->node = (*groups)[i];
> + if (!((*groupNames)[i] = virXPathString("string(./group_name)",
> ctxt))) {
> + g_strfreev(*groupNames);
> + *groupNames = NULL;
> + return -1;
> + }
> + }
> +
> + return ngroups;
> +}
> +
> +
> +char **
> +virshDomainThrottleGroupCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags)
> +{
> + virshControl *priv = ctl->privData;
> + g_autoptr(xmlDoc) xmldoc = NULL;
> + g_autoptr(xmlXPathContext) ctxt = NULL;
> + g_autofree xmlNodePtr *groups = NULL;
> + g_auto(GStrv) groupNames = NULL;
> + int ngroups;
> +
> + virCheckFlags(0, NULL);
> +
> + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> + return NULL;
> +
> + if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
> + return NULL;
> +
> + ngroups = virshGetThrottleGroupNames(ctxt, &groups, &groupNames);
> + if (ngroups < 0)
> + return NULL;
> +
> + return g_steal_pointer(&groupNames);
> +}
> +
> +
> char **
> virshDomainUndefineStorageDisksCompleter(vshControl *ctl,
> const vshCmd *cmd,
> diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
> index 27cf963912..680b3fc018 100644
> --- a/tools/virsh-completer-domain.h
> +++ b/tools/virsh-completer-domain.h
> @@ -21,6 +21,7 @@
> #pragma once
>
> #include "vsh.h"
> +#include <libxml/xpath.h>
>
> char **
> virshDomainNameCompleter(vshControl *ctl,
> @@ -41,6 +42,16 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
> const vshCmd *cmd,
> unsigned int flags);
>
> +int
> +virshGetThrottleGroupNames(xmlXPathContext *ctxt,
> + xmlNodePtr **groups,
> + char ***groupNames);
> +
> +char **
> +virshDomainThrottleGroupCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags);
> +
> char **
> virshDomainInterfaceStateCompleter(vshControl *ctl,
> const vshCmd *cmd,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 9f522da544..b8f61340cc 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1375,7 +1375,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = {
> VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> {.name = NULL}
> };
> -#undef VSH_OPTS_IOTUNE
> +
>
> static bool
> cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> @@ -1513,6 +1513,385 @@ 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;
> + g_auto(GStrv) groupNames = 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;
> +
> + table = vshTableNew(_("Name"), NULL);
> +
> + if (!table)
> + return false;
> +
> + ngroups = virshGetThrottleGroupNames(ctxt, &groups, &groupNames);
> + for (i = 0; i < ngroups; i++) {
> + if (vshTableRowAppend(table, groupNames[i], 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 = virshDomainThrottleGroupCompleter,
> + .help = N_("throttle group name")
> + },
> + VSH_OPTS_IOTUNE(),
> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> + {.name = NULL}
> +};
> +#undef VSH_OPTS_IOTUNE
> +
> +
> +static bool
> +cmdThrottleGroupSet(vshControl *ctl,
> + const vshCmd *cmd)
> +{
> + g_autoptr(virshDomain) dom = NULL;
> + 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, NULL)))
> + 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(¶ms, &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(¶ms, &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(¶ms, &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 set 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 = virshDomainThrottleGroupCompleter,
> + .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;
> + }
Inconsistent coding style
> +
> + if (virDomainDelThrottleGroup(dom, group_name, flags) < 0)
> + return false;
> + vshPrintExtra(ctl, "%s", _("Throttle group deleted successfully\n"));
> +
> + return true;
> +}
> +
> +
> +/*
> + * "throttlegroupinfo" command
This doesn't match the command name
> + */
> +static const vshCmdInfo info_throttlegroupinfo = {
> + .help = N_("Get a throttling group."),
> + .desc = N_("Get a throttling group."),
> +};
> +
> +
> +static const vshCmdOptDef opts_throttlegroupinfo[] = {
The struct name doesn't match the actual command
> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> + {.name = "group-name",
> + .type = VSH_OT_STRING,
> + .positional = true,
> + .required = true,
> + .completer = virshDomainThrottleGroupCompleter,
> + .help = N_("throttle group name")
> + },
> + VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> + VIRSH_COMMON_OPT_DOMAIN_LIVE,
> + VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> + {.name = NULL}
> +};
> +
> +
> +#define PARSE_THROTTLER_GROUP(val) \
> + if (virXPathULongLong("string(./" #val ")", \
> + ctxt, &throttleGroup->val) == -2) { \
> + virReportError(VIR_ERR_XML_ERROR, \
> + _("throttle group field '%1$s' must be an integer"),
> #val); \
> + goto cleanup; \
> + } \
> + if ((virTypedParamsAddULLong(¶ms, &nparams, &maxparams, \
> + #val, throttleGroup->val) < 0)) { \
> + vshError(ctl, "%s", _("Unable to get throttle group parameters")); \
> + goto cleanup; \
> + }
> +
> +static bool
> +cmdThrottleGroupInfo(vshControl *ctl,
> + const vshCmd *cmd)
> +{
> + g_autoptr(virshDomain) dom = NULL;
> + 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;
> +
> + g_autoptr(xmlDoc) xml = NULL;
> + g_autoptr(xmlXPathContext) ctxt = NULL;
> + g_autofree xmlNodePtr *node = NULL;
> + int n = 0;
> + int maxparams = 0;
> +
> + 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;
'flags' is effectively passed to virDomainGetXMLDesc, which doesn't take
any of the AFFECT_ flags. While VIR_DOMAIN_AFFECT_CONFIG accidentally
maps to VIR_DOMAIN_XML_INACTIVE; VIR_DOMAIN_AFFECT_LIVE maps to
VIR_DOMAIN_XML_SECURE.
--live doesn't make sense for this
> +
> + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> + goto cleanup;
> +
> + if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
> + goto cleanup;
> + }
Inconsistent coding style
> +
> + if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
> + goto cleanup;
> +
> + if ((n = virXPathNodeSet("/domain/throttlegroups/throttlegroup", ctxt,
> &node)) < 0)
> + goto cleanup;
> +
> + if (n == 0)
> + return 0;
> +
> + for (i = 0; i < n; i++) {
> + g_autoptr(virDomainThrottleGroupDef) throttleGroup =
> g_new0(virDomainThrottleGroupDef, 1);
> +
> + VIR_XPATH_NODE_AUTORESTORE(ctxt)
> + ctxt->node = node[i];
> +
> + if (STREQ(group_name, virXPathString("string(./group_name)", ctxt))
> == 0) {
virXPathString allocates a buffer for the string and returns it; this is
a memleak as it's not freed after use ...
> + continue;
> + }
> +
> + PARSE_THROTTLER_GROUP(total_bytes_sec);
> + PARSE_THROTTLER_GROUP(read_bytes_sec);
> + PARSE_THROTTLER_GROUP(write_bytes_sec);
> + PARSE_THROTTLER_GROUP(total_iops_sec);
> + PARSE_THROTTLER_GROUP(read_iops_sec);
> + PARSE_THROTTLER_GROUP(write_iops_sec);
> +
> + PARSE_THROTTLER_GROUP(total_bytes_sec_max);
> + PARSE_THROTTLER_GROUP(read_bytes_sec_max);
> + PARSE_THROTTLER_GROUP(write_bytes_sec_max);
> + PARSE_THROTTLER_GROUP(total_iops_sec_max);
> + PARSE_THROTTLER_GROUP(read_iops_sec_max);
> + PARSE_THROTTLER_GROUP(write_iops_sec_max);
> +
> + PARSE_THROTTLER_GROUP(size_iops_sec);
> +
> + PARSE_THROTTLER_GROUP(total_bytes_sec_max_length);
> + PARSE_THROTTLER_GROUP(read_bytes_sec_max_length);
> + PARSE_THROTTLER_GROUP(write_bytes_sec_max_length);
> + PARSE_THROTTLER_GROUP(total_iops_sec_max_length);
> + PARSE_THROTTLER_GROUP(read_iops_sec_max_length);
> + PARSE_THROTTLER_GROUP(write_iops_sec_max_length);
I don't quite understand the point of parsing this from XML and putting
it into typed parameters ...
> + }
> +
> + for (i = 0; i < nparams; i++) {
> + g_autofree char *str = vshGetTypedParamValue(ctl, ¶ms[i]);
> + vshPrint(ctl, "%-15s: %s\n", params[i].field, str);
... just to print it back from the parameter.
> + }
> +
> + ret = true;
> +
> + cleanup:
> + virTypedParamsFree(params, nparams);
> + return ret;
> +}
> +#undef PARSE_THROTTLEGROUP
> +
> /*
> * "blkiotune" command
> */
