Re: [PATCH RFC v2 05/12] qemu: hotplug: Support hot attach block disk along with throttle filters

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; When attaching disk along with specified throttle groups, those groups will be chained up by parent node name, this change includes service side codes: * Each filter references one throttle group by group name * Each filter has a nodename, and those filters are chained up in sequence * Filter nodename index is persistented in virDomainObj->privateData(qemuDomainObjPrivate) * During hotplug, filter is created through QMP request("blockdev-add" with "driver":"throttle") to QEMU * Finally, "device_add"(attach) QMP request is adjusted to set "drive" to be top filter nodename in chain. Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; ---a

src/qemu/qemu_block.c | 131 ++++++++++++++++++ src/qemu/qemu_block.h | 53 +++++++ src/qemu/qemu_command.c | 64 +++++++++ src/qemu/qemu_command.h | 6 + src/qemu/qemu_domain.c | 71 ++++++++++ src/qemu/qemu_domain.h | 19 ++- src/qemu/qemu_hotplug.c | 24 ++++ .../qemustatusxml2xmldata/backup-pull-in.xml | 1 + .../blockjob-blockdev-in.xml | 1 + .../blockjob-mirror-in.xml | 1 + .../migration-in-params-in.xml | 1 + .../migration-out-nbd-bitmaps-in.xml | 1 + .../migration-out-nbd-out.xml | 1 + .../migration-out-nbd-tls-out.xml | 1 + .../migration-out-params-in.xml | 1 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 1 + .../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 + 18 files changed, 375 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 738b72d7ea..c53de2465e 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2739,6 +2739,137 @@ qemuBlockStorageSourceCreateDetectSize(GHashTable *blockNamedNodeData, } +void +qemuBlockThrottleFilterSetNodename(virDomainThrottleFilterDef *filter, + char *nodename) +{ + g_free(filter->nodename); + filter->nodename = nodename; +} + + +const char * +qemuBlockThrottleFilterGetNodename(virDomainThrottleFilterDef *filter) +{ + return filter->nodename; +} + + +/** + * qemuBlockThrottleFilterGetProps: + * @filter: throttle filter + * @parentNodeName: parent nodename of @filter + * + * Build "arguments" part of "blockdev-add" QMP cmd. + * e.g. {"execute":"blockdev-add", "arguments":{"driver":"throttle", + * "node-name":"libvirt-2-filter", "throttle-group":"limits0", + * "file": "libvirt-1-format"}}

+ */ +virJSONValue * +qemuBlockThrottleFilterGetProps(virDomainThrottleFilterDef *filter, + const char *parentNodeName) +{ + g_autoptr(virJSONValue) props = NULL; + + if (virJSONValueObjectAdd(&props, + "s:driver", "throttle", + "s:node-name", qemuBlockThrottleFilterGetNodename(filter), + "s:throttle-group", filter->group_name, + "s:file", parentNodeName, + NULL) < 0) + return 0; + + return g_steal_pointer(&props); +} + + +void +qemuBlockThrottleFilterAttachDataFree(qemuBlockThrottleFilterAttachData *data) +{ + if (!data) + return; + + virJSONValueFree(data->filterProps); + g_free(data); +} + + +qemuBlockThrottleFilterAttachData * +qemuBlockThrottleFilterAttachPrepareBlockdev(virDomainThrottleFilterDef *filter, + const char *parentNodeName) +{ + g_autoptr(qemuBlockThrottleFilterAttachData) data = NULL; + + data = g_new0(qemuBlockThrottleFilterAttachData, 1); + + if (!(data->filterProps = qemuBlockThrottleFilterGetProps(filter, parentNodeName))) + return NULL; + + data->filterNodeName = qemuBlockThrottleFilterGetNodename(filter); + data->filterAttached = true;

+ + return g_steal_pointer(&data); +} + + +void +qemuBlockThrottleFilterAttachRollback(qemuMonitor *mon, + qemuBlockThrottleFilterAttachData *data) +{ + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); + + if (data->filterAttached) + ignore_value(qemuMonitorBlockdevDel(mon, data->filterNodeName));

+ + virErrorRestore(&orig_err); +} + + +void +qemuBlockThrottleFilterChainDataFree(qemuBlockThrottleFilterChainData *data)

+{ + size_t i; + + if (!data) + return; + + for (i = 0; i < data->nfilterdata; i++) + qemuBlockThrottleFilterAttachDataFree(data->filterdata[i]); + + g_free(data->filterdata); + g_free(data); +} + + +int +qemuBlockThrottleFilterChainAttach(qemuMonitor *mon, + qemuBlockThrottleFilterChainData *data) +{ + size_t i; + + for (i = 0; i < data->nfilterdata; i++) { + if (qemuMonitorBlockdevAdd(mon, &data->filterdata[i]->filterProps) < 0) + return -1; + data->filterdata[i]->filterAttached = true; + } + + return 0; +} + + +void +qemuBlockThrottleFilterChainDetach(qemuMonitor *mon, + qemuBlockThrottleFilterChainData *data) +{ + size_t i; + + for (i = data->nfilterdata; i > 0; i--) + qemuBlockThrottleFilterAttachRollback(mon, data->filterdata[i-1]); +} + + int qemuBlockRemoveImageMetadata(virQEMUDriver *driver, virDomainObj *vm,

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9d4563861f..2d8036c3ae 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1577,6 +1577,13 @@ qemuDiskConfigBlkdeviotuneEnabled(const virDomainDiskDef *disk) } +bool +qemuDiskConfigThrottleFilterChainEnabled(const virDomainDiskDef *disk) +{ + return disk->nthrottlefilters > 0; +} + + /** * qemuDiskBusIsSD: * @bus: disk bus @@ -1936,6 +1943,11 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, } else { if (qemuDomainDiskGetBackendAlias(disk, &drive) < 0) return NULL;

+ + /* make sure device drive points to top throttle filter */ + if (qemuDiskConfigThrottleFilterChainEnabled(disk)) { + drive = g_strdup(disk->throttlefilters[disk->nthrottlefilters-1]->nodename);

+ } } /* bootindex for floppies is configured via the fdc controller */ @@ -11026,6 +11038,58 @@ qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainD } +/** + * qemuBuildThrottleFilterChainAttachPrepareBlockdevOne: + * @data: filter chain data, which consists of array of filters and size of such array + * @throttlefilter: new filter to be added into filter array + * @parentNodeName: parent nodename for this new throttlefilter, which is used to build "blockdev-add" QMP request + * + * Build filter node chain to provide more flexibility for block disk I/O limits + */ +static int +qemuBuildThrottleFilterChainAttachPrepareBlockdevOne(qemuBlockThrottleFilterChainData *data, + virDomainThrottleFilterDef *throttlefilter, + const char *parentNodeName) +{ + g_autoptr(qemuBlockThrottleFilterAttachData) elem = NULL; + + if (!(elem = qemuBlockThrottleFilterAttachPrepareBlockdev(throttlefilter, parentNodeName))) + return -1; + + VIR_APPEND_ELEMENT(data->filterdata, data->nfilterdata, elem); + return 0; +} + + +/** + * qemuBuildThrottleFilterChainAttachPrepareBlockdev: + * @disk: domain disk + * + * Build filter node chain to provide more flexibility for block disk I/O limits + */ +qemuBlockThrottleFilterChainData * +qemuBuildThrottleFilterChainAttachPrepareBlockdev(virDomainDiskDef *disk) +{ + g_autoptr(qemuBlockThrottleFilterChainData) data = NULL; + size_t i; + const char * parentNodeName = NULL; + g_autofree char *tmp_nodename = NULL; + + data = g_new0(qemuBlockThrottleFilterChainData, 1); + /* get starting parentNodename, e.g. libvirt-1-format */ + parentNodeName = qemuBlockStorageSourceGetEffectiveNodename(disk->src); + /* build filterdata, which contains all filters info and sequence info through parentNodeName */ + for (i = 0; i < disk->nthrottlefilters; i++) { + tmp_nodename = g_strdup(parentNodeName); + if (qemuBuildThrottleFilterChainAttachPrepareBlockdevOne(data, disk->throttlefilters[i], tmp_nodename) < 0) + return NULL; + parentNodeName = disk->throttlefilters[i]->nodename; + } + + return g_steal_pointer(&data); +} + + /** * qemuBuildStorageSourceChainAttachPrepareBlockdev: * @top: storage source chain diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 341ec43f9a..e2dee47906 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -116,6 +116,12 @@ qemuBlockStorageSourceChainData * qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSource *top, virStorageSource *backingStore); +qemuBlockThrottleFilterChainData * +qemuBuildThrottleFilterChainAttachPrepareBlockdev(virDomainDiskDef *disk); + +bool +qemuDiskConfigThrottleFilterChainEnabled(const virDomainDiskDef *disk); + virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, virDomainDiskDef *disk, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b676f59c3a..0d80f15f05 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -684,6 +684,19 @@ qemuDomainStorageIDNew(qemuDomainObjPrivate *priv) } +/** + * qemuDomainThrottleFilterIDNew: + * @priv: qemu VM private data object. + * + * Generate a new unique id for throttle filter object. Useful for node name generation. + */ +static unsigned int +qemuDomainThrottleFilterIDNew(qemuDomainObjPrivate *priv) +{ + return ++priv->filternodenameindex; +} + + /** * qemuDomainStorageIDReset: * @priv: qemu VM private data object. @@ -698,6 +711,20 @@ qemuDomainStorageIDReset(qemuDomainObjPrivate *priv) } +/** + * qemuDomainThrottleFilterIDReset: + * @priv: qemu VM private data object. + * + * Resets the data for the node name generator. The node names need to be unique + * for a single instance, so can be reset on VM shutdown. + */ +static void +qemuDomainThrottleFilterIDReset(qemuDomainObjPrivate *priv) +{ + priv->filternodenameindex = 0; +}

+int +qemuDomainPrepareThrottleFilterBlockdev(virDomainThrottleFilterDef *filter, + qemuDomainObjPrivate *priv) +{ + g_autofree char *nodenameprefix = NULL; + + filter->id = qemuDomainThrottleFilterIDNew(priv); + + nodenameprefix = g_strdup_printf("libvirt-%u", filter->id); + + return qemuDomainPrepareThrottleFilterBlockdevNodename(filter, nodenameprefix); +} + + int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, virStorageSource *src, @@ -11371,6 +11436,7 @@ qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk, { qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); virStorageSource *n; + size_t i; if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && !diskPriv->nodeCopyOnRead) @@ -11381,6 +11447,11 @@ qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk, return -1; } + for (i = 0; i < disk->nthrottlefilters; i++) { + if (qemuDomainPrepareThrottleFilterBlockdev(disk->throttlefilters[i], priv) < 0) + return -1; + } + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 49e4da435b..a6158b39a9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -204,6 +204,9 @@ struct _qemuDomainObjPrivate { /* counter for generating node names for qemu disks */ unsigned long long nodenameindex; + /* counter for generating node names for throttle filters */ + unsigned long long filternodenameindex;

@@ -880,10 +895,6 @@ int qemuDomainSetPrivatePaths(virQEMUDriver *driver, virDomainDiskDef *qemuDomainDiskByName(virDomainDef *def, const char *name); -virDomainThrottleGroupDef * -qemuDomainThrottleGroupByName(virDomainDef *def, - const char *name);

- char *qemuDomainGetMasterKeyFilePath(const char *libDir); int qemuDomainMasterKeyReadFile(qemuDomainObjPrivate *priv); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 62dc879ed4..37d766b707 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -656,6 +656,7 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, virDomainAsyncJob asyncJob) { g_autoptr(qemuBlockStorageSourceChainData) data = NULL; + g_autoptr(qemuBlockThrottleFilterChainData) filterData = NULL; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virJSONValue) devprops = NULL; bool extensionDeviceAttached = false; @@ -694,6 +695,19 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if (rc < 0) goto rollback; + /* Setup throttling filter chain + * add additional "blockdev-add"(throttle filter) between "blockdev-add" (qemuBlockStorageSourceChainAttach) and "device_add" (qemuDomainAttachExtensionDevice) + */ + if ((filterData = qemuBuildThrottleFilterChainAttachPrepareBlockdev(disk))) { + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) + return -1; + /* QMP requests("blockdev-add" with "driver":"throttle") to QEMU */ + rc = qemuBlockThrottleFilterChainAttach(priv->mon, filterData); + qemuDomainObjExitMonitor(vm); + if (rc < 0) + goto rollback; + } + if (disk->transient) { g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL; g_autoptr(GHashTable) blockNamedNodeData = NULL;

@@ -765,6 +779,8 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if (extensionDeviceAttached) ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info)); + qemuBlockThrottleFilterChainDetach(priv->mon, filterData); + qemuBlockStorageSourceChainDetach(priv->mon, data); qemuDomainObjExitMonitor(vm); @@ -4504,6 +4520,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, { qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); g_autoptr(qemuBlockStorageSourceChainData) diskBackend = NULL; + g_autoptr(qemuBlockThrottleFilterChainData) filterData = NULL; size_t i; qemuDomainObjPrivate *priv = vm->privateData; int ret = -1; @@ -4542,6 +4559,13 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, } } + qemuDomainObjEnterMonitor(vm); + /* QMP request("blockdev-del") to QEMU to delete throttle filter*/ + if ((filterData = qemuBuildThrottleFilterChainAttachPrepareBlockdev(disk))) {

+ qemuBlockThrottleFilterChainDetach(priv->mon, filterData); + } + qemuDomainObjExitMonitor(vm); + qemuDomainObjEnterMonitor(vm); if (diskBackend)