[libvirt] [PATCH 0/2] qemu: Don't lose group_name
Don't lose group_name: the movie. I mean, the series. Martin Kletzander (2): conf: Add virDomainDiskSetBlockIOTune qemu: Don't lose group_name src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 31 +++++++++++++++++++------------ 4 files changed, 51 insertions(+), 12 deletions(-) -- 2.11.0
That function sets disk->blkdeviotune sensibly. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 32 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 877a0bf5c148..c06b128ddcb4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25845,3 +25845,30 @@ virDomainDefVcpuOrderClear(virDomainDefPtr def) for (i = 0; i < def->maxvcpus; i++) def->vcpus[i]->order = 0; } + + +/** + * virDomainDiskSetBlockIOTune: + * @disk: The disk to set block I/O tuning on + * @info: The BlockIoTuneInfo to be set on the @disk + * + * Set the block I/O tune settings from @info on the @disk, but error out early + * in case of any error. That is to make sure nothing will fail half-way. + * + * Returns: 0 on success, -1 otherwise + */ +int +virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, + virDomainBlockIoTuneInfo *info) +{ + char *tmp_group = NULL; + + if (VIR_STRDUP(tmp_group, info->group_name) < 0) + return -1; + + VIR_FREE(disk->blkdeviotune.group_name); + disk->blkdeviotune = *info; + VIR_STEAL_PTR(disk->blkdeviotune.group_name, tmp_group); + + return 0; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4d830c51d4f7..507ace871174 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3219,4 +3219,8 @@ bool virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, const virDomainDeviceInfo *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, + virDomainBlockIoTuneInfo *info); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2866a3a06f0..8e994c7f062b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -314,6 +314,7 @@ virDomainDiskMirrorStateTypeToString; virDomainDiskPathByName; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSetBlockIOTune; virDomainDiskSetDriver; virDomainDiskSetFormat; virDomainDiskSetSource; -- 2.11.0
Now that we have a function for properly assigning the blockdeviotune info, let's use it instead of dropping the group name on every assignment. Otherwise it will not work with both --live and --config options. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc5e4487c861..37ccfdf6baa3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17173,7 +17173,7 @@ typedef enum { /* If the user didn't specify bytes limits, inherit previous values; * likewise if the user didn't specify iops limits. */ -static void +static int qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo, virDomainBlockIoTuneInfoPtr oldinfo, qemuBlockIoTuneSetFlags set_fields) @@ -17193,8 +17193,9 @@ qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo, if (!(set_fields & QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS)) newinfo->size_iops_sec = oldinfo->size_iops_sec; - if (!(set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME)) - VIR_STEAL_PTR(newinfo->group_name, oldinfo->group_name); + if (!(set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME) && + VIR_STRDUP(newinfo->group_name, oldinfo->group_name) < 0) + return -1; /* The length field is handled a bit differently. If not defined/set, * QEMU will default these to 0 or 1 depending on whether something in @@ -17226,6 +17227,7 @@ qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo, #undef SET_MAX_LENGTH + return 0; } @@ -17477,8 +17479,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (!(device = qemuAliasFromDisk(disk))) goto endjob; - qemuDomainSetBlockIoTuneDefaults(&info, &disk->blkdeviotune, - set_fields); + if (qemuDomainSetBlockIoTuneDefaults(&info, &disk->blkdeviotune, + set_fields) < 0) + goto endjob; #define CHECK_MAX(val, _bool) \ do { \ @@ -17529,8 +17532,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (ret < 0) goto endjob; ret = -1; - disk->blkdeviotune = info; - info.group_name = NULL; + + if (virDomainDiskSetBlockIOTune(disk, &info) < 0) + goto endjob; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) @@ -17550,14 +17554,17 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, path); goto endjob; } - qemuDomainSetBlockIoTuneDefaults(&info, &conf_disk->blkdeviotune, - set_fields); - conf_disk->blkdeviotune = info; - info.group_name = NULL; + + if (qemuDomainSetBlockIoTuneDefaults(&info, &conf_disk->blkdeviotune, + set_fields) < 0) + goto endjob; + + if (virDomainDiskSetBlockIOTune(conf_disk, &info) < 0) + goto endjob; + if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) goto endjob; - } ret = 0; -- 2.11.0
On 01/31/2017 04:15 PM, Martin Kletzander wrote:
Don't lose group_name: the movie. I mean, the series.
Martin Kletzander (2): conf: Add virDomainDiskSetBlockIOTune qemu: Don't lose group_name
src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 31 +++++++++++++++++++------------ 4 files changed, 51 insertions(+), 12 deletions(-)
ACK to both. Michal
participants (2)
-
Martin Kletzander -
Michal Privoznik