On 01/25/2017 10:16 AM, Martin Kletzander wrote:
Due to our APIs not copying various pointers, we need to carry it around on the side and just supply it every time it is needed. Otherwise it will not work with both --live and --config options.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f45972e3c823..3a0245ec1cb8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17256,6 +17256,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virTypedParameterPtr eventParams = NULL; int eventNparams = 0; int eventMaxparams = 0; + char *group_name = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -17370,7 +17371,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
/* NB: Cannot use macro since this is a value.s not a value.ul */ if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME)) { - if (VIR_STRDUP(info.group_name, params->value.s) < 0) + if (VIR_STRDUP(group_name, params->value.s) < 0) goto endjob; set_fields |= QEMU_BLOCK_IOTUNE_SET_GROUP_NAME; if (virTypedParamsAddString(&eventParams, &eventNparams, @@ -17500,6 +17501,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
#undef CHECK_MAX
+ if (set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME && + VIR_STRDUP(info.group_name, group_name) < 0) + goto endjob; +
There is still one problem. qemuDomainSetBlockIoTuneDefaults steals disk->blkdeviotune->group_name pointer (into info->group_name). In ideal case, when there is no error this is not a problem since the disk->blkdeviotune is going to be replaced with info. However, should something go wrong in between those two lines the group_name is lost.
/* NB: Let's let QEMU decide how to handle issues with _length * via the JSON error code from the block_set_io_throttle call */
@@ -17513,7 +17518,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (ret < 0) goto endjob; disk->blkdeviotune = info; - info.group_name = NULL;
ret = virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps); if (ret < 0) @@ -17533,10 +17537,14 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, path); goto endjob; } + + if (set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME && + VIR_STRDUP(info.group_name, group_name) < 0) + goto endjob; + qemuDomainSetBlockIoTuneDefaults(&info, &conf_disk->blkdeviotune, set_fields); conf_disk->blkdeviotune = info; - info.group_name = NULL; ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef); if (ret < 0) goto endjob; @@ -17547,7 +17555,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEndJob(driver, vm);
cleanup: - VIR_FREE(info.group_name); + VIR_FREE(group_name); VIR_FREE(device); virDomainObjEndAPI(&vm); if (eventNparams)
Despite the problem I'm describing above, this is undoubtedly an improvement. ACK Michal