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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
