On Mon, Nov 18, 2024 at 19:24:25 +0530, Harikumar R wrote:
> From: Chun Feng Wu <danielwuwy@xxxxxxx>
>
> Update "attach_disk" to support new option: throttle-groups to
> form filter chain in QEMU for specific disk
>
> Signed-off-by: Chun Feng Wu <danielwuwy@xxxxxxx>
> ---
> 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(-)
[...]
> @@ -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_autofree char **throttle_groups = NULL;
This is filled by g_strsplit, so this would leak the individual strings.
You need to use g_auto(GStrv) to declare this.
> int ret;
> unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> const char *stype = NULL;
> @@ -622,6 +629,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> g_auto(virBuffer) sourceAttrBuf = VIR_BUFFER_INITIALIZER;
> g_auto(virBuffer) sourceChildBuf = VIR_BUFFER_INIT_CHILD(&diskChildBuf);
> g_auto(virBuffer) hostAttrBuf = VIR_BUFFER_INITIALIZER;
> + g_auto(virBuffer) throttleChildBuf = VIR_BUFFER_INITIALIZER;
Declare this inside the block using it.
> g_autofree char *xml = NULL;
> struct stat st;
> bool current = vshCommandOptBool(cmd, "current");
> @@ -665,9 +673,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 +727,16 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>
> virXMLFormatElement(&diskChildBuf, "driver", &driverAttrBuf, NULL);
>
> + if (throttle_groups) {
> + 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);
> + }
> +
With the bug fixed:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
