On Wed, Feb 19, 2025 at 22:27:15 +0530, Harikumar Rajkumar wrote:
> From: Chun Feng Wu <danielwuwy@xxxxxxx>
>
> For hot attaching/detaching
> * Leverage qemuBlockThrottleFiltersData to prepare attaching/detaching
> throttle filter data for qemuMonitorBlockdevAdd and qemuMonitorBlockdevDel
> * For hot attaching, within qemuDomainAttachDiskGeneric,prepare throttle
> filters json data, and create corresponding blockdev for QMP request
> ("blockdev-add" with "driver":"throttle")
> * Each filter has a nodename, and those filters are chained up,
> create them in sequence, and delete them reversely
> * Delete filters by "qemuBlockThrottleFiltersDetach"("blockdev-del")
> when detaching device
>
> For throttle group commandline
> * Add qemuBuildThrottleGroupCommandLine in qemuBuildCommandLine to add
> "object" of throttle-group
> * Verify throttle group definition when lauching vm
> * Check QEMU_CAPS_OBJECT_JSON before "qemuBuildObjectCommandlineFromJSON",
> which is to build "-object" option
>
> For throttle filter commandline
> * Add qemuBuildDiskThrottleFiltersCommandLine in qemuBuildDiskCommandLine
> to add "blockdev"
>
> Signed-off-by: Chun Feng Wu <danielwuwy@xxxxxxx>
>
> * Apply suggested coding style changes.
> * Update of code documentation comments.
>
> Signed-off-by: Harikumar Rajkumar <harirajkumar230@xxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 99 +++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_hotplug.c | 21 +++++++++
> 2 files changed, 120 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 750edd4faa..6d9efbe887 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2045,6 +2045,99 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommand *cmd,
> }
>
>
> +static inline bool
> +qemuDiskConfigThrottleGroupEnabled(const virDomainThrottleGroupDef *group)
> +{
> + return !!group->group_name &&
> + virDomainBlockIoTuneInfoHasAny(group);
> +}
> +
> +
> +/**
> + * qemuBuildThrottleGroupCommandLine:
> + * @cmd: the command to modify
> + * @def: domain definition
> + * @qemuCaps: qemu capabilities object
> + *
> + * build throttle group object in json format
> + */
> +static int
> +qemuBuildThrottleGroupCommandLine(virCommand *cmd,
> + const virDomainDef *def,
> + virQEMUCaps *qemuCaps)
> +{
> + size_t i;
> +
> + for (i = 0; i < def->nthrottlegroups; i++) {
> + g_autoptr(virJSONValue) props = NULL;
> + g_autoptr(virJSONValue) limits = virJSONValueNewObject();
> + virDomainThrottleGroupDef *group = def->throttlegroups[i];
> + /* prefix group name with "throttle-" in QOM */
> + g_autofree char *prefixed_group_name = g_strdup_printf("throttle-%s", group->group_name);
> +
> + if (!qemuDiskConfigThrottleGroupEnabled(group)) {
> + continue;
> + }
> +
> + if (qemuMonitorThrottleGroupLimits(limits, group) < 0)
> + return -1;
> +
> + if (qemuMonitorCreateObjectProps(&props, "throttle-group", prefixed_group_name,
> + "a:limits", &limits,
> + NULL) < 0)
> + return -1;
> +
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_JSON)) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("This QEMU doesn't support throttle group creation"));
> + return -1;
This would be considered too late for this check; Luckily all of it can
be deledet now.
> + }
> +
> + if (qemuBuildObjectCommandlineFromJSON(cmd, props, qemuCaps) < 0)
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> +qemuBuildBlockThrottleFilterCommandline(virCommand *cmd,
> + qemuBlockThrottleFilterAttachData *data)
> +{
> + if (data->filterProps) {
> + g_autofree char *tmp = NULL;
> + if (!(tmp = virJSONValueToString(data->filterProps, false)))
> + return -1;
> +
> + virCommandAddArgList(cmd, "-blockdev", tmp, NULL);
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> +qemuBuildDiskThrottleFiltersCommandLine(virCommand *cmd,
> + virDomainDiskDef *disk)
> +{
> + g_autoptr(qemuBlockThrottleFiltersData) data = NULL;
> + size_t i;
> +
> + data = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk);
> + if (!data)
> + return -1;
> +
> + for (i = 0; i < data->nfilterdata; i++) {
> + if (qemuBuildBlockThrottleFilterCommandline(cmd,
> + data->filterdata[i]) < 0)
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> static int
> qemuBuildDiskSourceCommandLine(virCommand *cmd,
> virDomainDiskDef *disk,
> @@ -2102,6 +2195,9 @@ qemuBuildDiskCommandLine(virCommand *cmd,
> if (qemuBuildDiskSourceCommandLine(cmd, disk, qemuCaps) < 0)
> return -1;
>
> + if (qemuBuildDiskThrottleFiltersCommandLine(cmd, disk) < 0)
> + return -1;
> +
> /* SD cards are currently instantiated via -drive if=sd, so the -device
> * part must be skipped */
> if (qemuDiskBusIsSD(disk->bus))
> @@ -10479,6 +10575,9 @@ qemuBuildCommandLine(virDomainObj *vm,
> if (qemuBuildIOThreadCommandLine(cmd, def, qemuCaps) < 0)
> return NULL;
>
> + if (qemuBuildThrottleGroupCommandLine(cmd, def, qemuCaps) < 0)
> + return NULL;
> +
> if (virDomainNumaGetNodeCount(def->numa) &&
> qemuBuildNumaCommandLine(cfg, def, cmd, priv) < 0)
> return NULL;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 28ca321c5c..75437164ca 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -708,6 +708,7 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
> virDomainAsyncJob asyncJob)
> {
> g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
> + g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL;
> qemuDomainObjPrivate *priv = vm->privateData;
> g_autoptr(virJSONValue) devprops = NULL;
> bool extensionDeviceAttached = false;
> @@ -746,6 +747,15 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
> if (rc < 0)
> goto rollback;
>
> + if ((filterData = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk))) {
> + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
> + return -1;
> + rc = qemuBlockThrottleFiltersAttach(priv->mon, filterData);
> + qemuDomainObjExitMonitor(vm);
> + if (rc < 0)
> + goto rollback;
> + }
> +
> if (disk->transient) {
> g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL;
> g_autoptr(GHashTable) blockNamedNodeData = NULL;
> @@ -817,6 +827,8 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
> if (extensionDeviceAttached)
> ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info));
>
> + qemuBlockThrottleFiltersDetach(priv->mon, filterData);
> +
> qemuBlockStorageSourceChainDetach(priv->mon, data);
>
> qemuDomainObjExitMonitor(vm);
> @@ -4686,6 +4698,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
> {
> qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> g_autoptr(qemuBlockStorageSourceChainData) diskBackend = NULL;
> + g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL;
> size_t i;
> qemuDomainObjPrivate *priv = vm->privateData;
> int ret = -1;
> @@ -4726,6 +4739,14 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
>
> qemuDomainObjEnterMonitor(vm);
>
> + if ((filterData = qemuBuildThrottleFiltersDetachPrepareBlockdev(disk))) {
I'll have to also check the layering with copy on read which is detached
below
> +
> + qemuBlockThrottleFiltersDetach(priv->mon, filterData);
> + }
> + qemuDomainObjExitMonitor(vm);
> +
> + qemuDomainObjEnterMonitor(vm);
No need to exit the monitor to just enter it back.
> +
> if (diskBackend)
> qemuBlockStorageSourceChainDetach(priv->mon, diskBackend);
>
> --
> 2.39.5 (Apple Git-154)
>
