On Wed, Feb 19, 2025 at 22:27:22 +0530, Harikumar Rajkumar wrote:
> From: Chun Feng Wu <[email protected]>
>
> Update "attach_disk" to support new option: throttle-groups to
> form filter chain in QEMU for specific disk
>
> Signed-off-by: Chun Feng Wu <[email protected]>
>
> * apply suggested coding style changes.
>
> Signed-off-by: Harikumar Rajkumar <[email protected]>
> ---
> docs/manpages/virsh.rst | 3 ++-
> tools/virsh-completer-domain.c | 27 +++++++++++++++++++++++++++
> tools/virsh-completer-domain.h | 5 +++++
> tools/virsh-domain.c | 25 ++++++++++++++++++++++++-
> 4 files changed, 58 insertions(+), 2 deletions(-)
[...]
> index 3b0df15c13..b96180be9e 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -303,6 +303,33 @@ virshDomainThrottleGroupCompleter(vshControl *ctl,
> }
>
>
> +static char **
> +virshDomainThrottleGroupListCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + const char *argname)
> +{
> + const char *curval = NULL;
> + g_auto(GStrv) groups = virshDomainThrottleGroupCompleter(ctl, cmd, 0);
> +
> + if (vshCommandOptStringQuiet(ctl, cmd, argname, &curval) < 0)
> + return NULL;
> +
> + if (!groups)
> + return NULL;
> +
> + return virshCommaStringListComplete(curval, (const char **) groups);
> +}
> +
> +
> +char **
> +virshDomainThrottleGroupsCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int completeflags G_GNUC_UNUSED)
> +{
> + return virshDomainThrottleGroupListCompleter(ctl, cmd,
> "throttle-groups");
> +}
This wrapper is pointless and can be folded into the above function.
> +
> +
> char **
> virshDomainUndefineStorageDisksCompleter(vshControl *ctl,
> const vshCmd *cmd,
> diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
> index 680b3fc018..23b432d05c 100644
> --- a/tools/virsh-completer-domain.h
> +++ b/tools/virsh-completer-domain.h
> @@ -52,6 +52,11 @@ virshDomainThrottleGroupCompleter(vshControl *ctl,
> const vshCmd *cmd,
> unsigned int flags);
>
> +char **
> +virshDomainThrottleGroupsCompleter(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 b8f61340cc..e156199eb1 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -522,6 +522,11 @@ static const vshCmdOptDef opts_attach_disk[] = {
> .type = VSH_OT_STRING,
> .help = N_("host socket for source of disk device")
> },
> + {.name = "throttle-groups",
> + .type = VSH_OT_STRING,
> + .completer = virshDomainThrottleGroupsCompleter,
> + .help = N_("comma separated list of throttle groups to be applied")
> + },
> VIRSH_COMMON_OPT_DOMAIN_PERSISTENT,
> VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> VIRSH_COMMON_OPT_DOMAIN_LIVE,
> @@ -611,6 +616,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> const char *host_name = NULL;
> const char *host_transport = NULL;
> const char *host_socket = NULL;
> + const char *throttle_groups_str = NULL;
> + g_auto(GStrv) throttle_groups = NULL;
> int ret;
> unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> const char *stype = NULL;
> @@ -665,9 +672,14 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> vshCommandOptString(ctl, cmd, "source-protocol", &source_protocol) <
> 0 ||
> vshCommandOptString(ctl, cmd, "source-host-name", &host_name) < 0 ||
> vshCommandOptString(ctl, cmd, "source-host-transport",
> &host_transport) < 0 ||
> - vshCommandOptString(ctl, cmd, "source-host-socket", &host_socket) <
> 0)
> + vshCommandOptString(ctl, cmd, "source-host-socket", &host_socket) <
> 0 ||
> + vshCommandOptString(ctl, cmd, "throttle-groups",
> &throttle_groups_str) < 0)
> return false;
>
> + if (throttle_groups_str) {
> + throttle_groups = g_strsplit(throttle_groups_str, ",", 0);
> + }
> +
> if (stype &&
> (type = virshAttachDiskSourceTypeFromString(stype)) < 0) {
> vshError(ctl, _("Unknown source type: '%1$s'"), stype);
> @@ -714,6 +726,17 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>
> virXMLFormatElement(&diskChildBuf, "driver", &driverAttrBuf, NULL);
>
> + if (throttle_groups) {
> + g_auto(virBuffer) throttleChildBuf = VIR_BUFFER_INITIALIZER;
This ought to use VIR_BUFFER_INIT_CHILD(&diskCHildBuf) otherwise the
resulting XML will be malformated. (visible with --print-xml)
> + char **iter;
> + for (iter = throttle_groups; *iter != NULL; iter++) {
> + g_auto(virBuffer) throttleAttrBuf = VIR_BUFFER_INITIALIZER;
> + virBufferAsprintf(&throttleAttrBuf, " group='%s'", *iter);
> + virXMLFormatElement(&throttleChildBuf, "throttlefilter",
> &throttleAttrBuf, NULL);
> + }
> + virXMLFormatElement(&diskChildBuf, "throttlefilters", NULL,
> &throttleChildBuf);
> + }
This whole block should go to the same spot as where it's formatted in
the domain XML; behind 'target'.
> +
> switch ((enum virshAttachDiskSourceType) type) {
> case VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE:
> virBufferEscapeString(&sourceAttrBuf, " file='%s'", source);
> --
> 2.39.5 (Apple Git-154)
>
