On Mon, Nov 18, 2024 at 19:24:24 +0530, Harikumar R wrote:
> From: Chun Feng Wu <danielwuwy@xxxxxxx>
>
> 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 <danielwuwy@xxxxxxx>
> ---
> docs/manpages/virsh.rst | 132 +++++++++++++
> tools/virsh-completer-domain.c | 55 ++++++
> tools/virsh-completer-domain.h | 11 ++
> tools/virsh-domain.c | 349 ++++++++++++++++++++++++++++++++-
> 4 files changed, 546 insertions(+), 1 deletion(-)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 6776ea53d0..3b78f77384 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -1122,6 +1122,138 @@ 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.
> +*domain* (see also ``domblklist`` for listing these names).
Referencing domblklist doesn't seem to be relevant here.
> +
> +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* flags may be
This also seems to be referencing something that's not happening here.
> +given, 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 such as via the
> +``attach-disk`` command, then the attempt to remove the Throttlegroup will fail.
Referencing 'attach-disk' makes no sense. Stick to mentioning that it's
used by a disk.
> +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.
Missing note that --live and --config together makes no sense.
> +
> +
> +domthrottlegrouplist
> +--------------------
> +
> +**Syntax:**
> +
> +::
> +
> + domthrottlegrouplist domain [--inactive]
> +
> +Print a table showing names of all throttle groups
> +associated with *domain*. If *--inactive* is specified, query the
> +block devices that will be used on the next boot, rather than those
This is querying throttle groups; not block devices.
> +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 6eea5f176b..b7004979c6 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,329 @@ 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;
> + }
> +
> + 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 = 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
> +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;
> +
> + 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;
> +
> + if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) {
> + goto cleanup;
> + }
> +
> + if (virDomainGetThrottleGroup(dom, group_name, ¶ms, &nparams, flags) != 0) {
This will most likely need to be reimplemented by querying the data from
the XML as the API itself IMO doesn't make too much sense to exist as it
simply queries what we've set.
> + vshError(ctl, "%s",
> + _("Unable to get throttle group parameters"));
> + goto cleanup;
> + }
> +
> + for (i = 0; i < nparams; i++) {
> + g_autofree char *str = vshGetTypedParamValue(ctl, ¶ms[i]);
> + vshPrint(ctl, "%-15s: %s\n", params[i].field, str);
> + }
> +
> + ret = true;
> +
> + cleanup:
> + virTypedParamsFree(params, nparams);
> + return ret;
> +}
