[libvirt] [PATCH 00/11] qemu: Erradicate use of virDomainLiveConfigHelperMethod
Don't use that terrible function in the qemu driver and a few relevant refactors. Peter Krempa (11): qemu: monitor: Remove 'supportMaxOptions' argument from qemuMonitorGetBlockIoThrottle qemu: Replace virDomainLiveConfigHelperMethod in qemuDomainGetBlockIoTune qemu: Refactor typed params assignment in qemuDomainGetBlockIoTune qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainSetMemoryParameters qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainGetBlkioParameters qemu: Refactor qemuDomainGetBlkioParameters qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainGetSchedulerParametersFlags conf: Change virDomainCputune member 'shares' to unsigned long long qemu: Refactor qemuDomainGetSchedulerParametersFlags qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainSetBlockIoTune qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainSetSchedulerParametersFlags src/conf/domain_conf.c | 6 +- src/conf/domain_conf.h | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/qemu_driver.c | 896 ++++++++++++------------------------------- src/qemu/qemu_monitor.c | 5 +- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_monitor_json.c | 24 +- src/qemu/qemu_monitor_json.h | 3 +- src/vmx/vmx.c | 6 +- tests/qemumonitorjsontest.c | 2 +- 10 files changed, 261 insertions(+), 688 deletions(-) -- 2.8.2
The caller is already aware that the params are missing and the extractor is ignoring the missing ones so the parameter isn't necessary. --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 5 ++--- src/qemu/qemu_monitor.h | 3 +-- src/qemu/qemu_monitor_json.c | 24 ++++++++++-------------- src/qemu/qemu_monitor_json.h | 3 +-- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ec2742c..af42c8d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17852,7 +17852,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(device = qemuAliasFromDisk(disk))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions); + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (ret < 0) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a0060cc..597307f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3126,15 +3126,14 @@ qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - bool supportMaxOptions) + virDomainBlockIoTuneInfoPtr reply) { VIR_DEBUG("device=%p, reply=%p", device, reply); QEMU_CHECK_MONITOR(mon); if (mon->json) - return qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, supportMaxOptions); + return qemuMonitorJSONGetBlockIoThrottle(mon, device, reply); else return qemuMonitorTextGetBlockIoThrottle(mon, device, reply); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a1cbc94..dd3587f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -804,8 +804,7 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - bool supportMaxOptions); + virDomainBlockIoTuneInfoPtr reply); int qemuMonitorSystemWakeup(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0508fe6..64827c3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4499,8 +4499,7 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, static int qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, const char *device, - virDomainBlockIoTuneInfoPtr reply, - bool supportMaxOptions) + virDomainBlockIoTuneInfoPtr reply) { virJSONValuePtr io_throttle; int ret = -1; @@ -4549,15 +4548,13 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, GET_THROTTLE_STATS("iops", total_iops_sec); GET_THROTTLE_STATS("iops_rd", read_iops_sec); GET_THROTTLE_STATS("iops_wr", write_iops_sec); - if (supportMaxOptions) { - GET_THROTTLE_STATS_OPTIONAL("bps_max", total_bytes_sec_max); - GET_THROTTLE_STATS_OPTIONAL("bps_rd_max", read_bytes_sec_max); - GET_THROTTLE_STATS_OPTIONAL("bps_wr_max", write_bytes_sec_max); - GET_THROTTLE_STATS_OPTIONAL("iops_max", total_iops_sec_max); - GET_THROTTLE_STATS_OPTIONAL("iops_rd_max", read_iops_sec_max); - GET_THROTTLE_STATS_OPTIONAL("iops_wr_max", write_iops_sec_max); - GET_THROTTLE_STATS_OPTIONAL("iops_size", size_iops_sec); - } + GET_THROTTLE_STATS_OPTIONAL("bps_max", total_bytes_sec_max); + GET_THROTTLE_STATS_OPTIONAL("bps_rd_max", read_bytes_sec_max); + GET_THROTTLE_STATS_OPTIONAL("bps_wr_max", write_bytes_sec_max); + GET_THROTTLE_STATS_OPTIONAL("iops_max", total_iops_sec_max); + GET_THROTTLE_STATS_OPTIONAL("iops_rd_max", read_iops_sec_max); + GET_THROTTLE_STATS_OPTIONAL("iops_wr_max", write_iops_sec_max); + GET_THROTTLE_STATS_OPTIONAL("iops_size", size_iops_sec); break; } @@ -4643,8 +4640,7 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - bool supportMaxOptions) + virDomainBlockIoTuneInfoPtr reply) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -4670,7 +4666,7 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, goto cleanup; } - ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply, supportMaxOptions); + ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply); cleanup: virJSONValueFree(cmd); virJSONValueFree(result); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 95bba69..76758db 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -328,8 +328,7 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply, - bool supportMaxOptions); + virDomainBlockIoTuneInfoPtr reply); int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 87b1a8f..819f7ce 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1882,7 +1882,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) goto cleanup; if (qemuMonitorJSONGetBlockIoThrottle(qemuMonitorTestGetMonitor(test), - "drive-virtio-disk0", &info, true) < 0) + "drive-virtio-disk0", &info) < 0) goto cleanup; if (memcmp(&info, &expectedInfo, sizeof(info)) != 0) { -- 2.8.2
Use virDomainObjGetDefs since the API guarantees that both live and config are never set together. --- src/qemu/qemu_driver.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index af42c8d..2e6b166 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17790,12 +17790,12 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; qemuDomainObjPrivatePtr priv = NULL; + virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo reply; char *device = NULL; int ret = -1; size_t i; - virCapsPtr caps = NULL; bool supportMaxOptions = true; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -17808,27 +17808,21 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1; - if (virDomainGetBlockIoTuneEnsureACL(dom->conn, vm->def) < 0) - goto cleanup; + priv = vm->privateData; - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + if (virDomainGetBlockIoTuneEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &persistentDef) < 0) + /* the API check guarantees that only one of the definitions will be set */ + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; - sa_assert((flags & VIR_DOMAIN_AFFECT_LIVE) || - (flags & VIR_DOMAIN_AFFECT_CONFIG)); - if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (def) { /* If the VM is running, we can check if the current VM can use - * optional parameters or not. We didn't made this check sooner - * because we need vm->privateData which need - * virDomainLiveConfigHelperMethod to do so. */ - priv = vm->privateData; + * optional parameters or not. */ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("block I/O throttling not supported with this " @@ -17845,8 +17839,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!(disk = qemuDomainDiskByName(vm->def, path))) + if (def) { + if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; if (!(device = qemuAliasFromDisk(disk))) @@ -17859,7 +17853,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (persistentDef) { if (!(disk = virDomainDiskByName(persistentDef, path, true))) { virReportError(VIR_ERR_INVALID_ARG, _("disk '%s' was not found in the domain config"), @@ -17981,7 +17975,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(device); virDomainObjEndAPI(&vm); - virObjectUnref(caps); return ret; } -- 2.8.2
Introduce a macro to assign the parameters to avoid the for loop and shuffle around various checks for a simpler and saner function. --- src/qemu/qemu_driver.c | 145 +++++++++++++------------------------------------ 1 file changed, 37 insertions(+), 108 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2e6b166..4ec04ee 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17795,8 +17795,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, virDomainBlockIoTuneInfo reply; char *device = NULL; int ret = -1; - size_t i; - bool supportMaxOptions = true; + int maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -17829,16 +17828,21 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, "QEMU binary")); goto endjob; } - supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX); + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) + maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; } - if ((*nparams) == 0) { - *nparams = supportMaxOptions ? - QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX : QEMU_NB_BLOCK_IO_TUNE_PARAM; + if (*nparams == 0) { + *nparams = maxparams; ret = 0; goto endjob; + } else if (*nparams < maxparams) { + maxparams = *nparams; } + *nparams = 0; + if (def) { if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; @@ -17863,110 +17867,35 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, reply = disk->blkdeviotune; } - for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX && i < *nparams; i++) { - virTypedParameterPtr param = ¶ms[i]; +#define BLOCK_IOTUNE_ASSIGN(name, var) \ + if (*nparams < maxparams && \ + virTypedParameterAssign(¶ms[(*nparams)++], \ + VIR_DOMAIN_BLOCK_IOTUNE_ ## name, \ + VIR_TYPED_PARAM_ULLONG, \ + reply.var) < 0) \ + goto endjob - switch (i) { - case 0: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.total_bytes_sec) < 0) - goto endjob; - break; - case 1: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.read_bytes_sec) < 0) - goto endjob; - break; - case 2: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.write_bytes_sec) < 0) - goto endjob; - break; - case 3: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.total_iops_sec) < 0) - goto endjob; - break; - case 4: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.read_iops_sec) < 0) - goto endjob; - break; - case 5: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.write_iops_sec) < 0) - goto endjob; - break; - case 6: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - reply.total_bytes_sec_max) < 0) - goto endjob; - break; - case 7: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - reply.read_bytes_sec_max) < 0) - goto endjob; - break; - case 8: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - reply.write_bytes_sec_max) < 0) - goto endjob; - break; - case 9: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - reply.total_iops_sec_max) < 0) - goto endjob; - break; - case 10: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - reply.read_iops_sec_max) < 0) - goto endjob; - break; - case 11: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - reply.write_iops_sec_max) < 0) - goto endjob; - break; - case 12: - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - reply.size_iops_sec) < 0) - goto endjob; - /* coverity[dead_error_begin] */ - default: - break; - } - } - if (!supportMaxOptions && *nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM) - *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; - else if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX) - *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX; + BLOCK_IOTUNE_ASSIGN(TOTAL_BYTES_SEC, total_bytes_sec); + BLOCK_IOTUNE_ASSIGN(READ_BYTES_SEC, read_bytes_sec); + BLOCK_IOTUNE_ASSIGN(WRITE_BYTES_SEC, write_bytes_sec); + + BLOCK_IOTUNE_ASSIGN(TOTAL_IOPS_SEC, total_iops_sec); + BLOCK_IOTUNE_ASSIGN(READ_IOPS_SEC, read_iops_sec); + BLOCK_IOTUNE_ASSIGN(WRITE_IOPS_SEC, write_iops_sec); + + BLOCK_IOTUNE_ASSIGN(TOTAL_BYTES_SEC_MAX, total_bytes_sec_max); + BLOCK_IOTUNE_ASSIGN(READ_BYTES_SEC_MAX, read_bytes_sec_max); + BLOCK_IOTUNE_ASSIGN(WRITE_BYTES_SEC_MAX, write_bytes_sec_max); + + BLOCK_IOTUNE_ASSIGN(TOTAL_IOPS_SEC_MAX, total_iops_sec_max); + BLOCK_IOTUNE_ASSIGN(READ_IOPS_SEC_MAX, read_iops_sec_max); + BLOCK_IOTUNE_ASSIGN(WRITE_IOPS_SEC_MAX, write_iops_sec_max); + + BLOCK_IOTUNE_ASSIGN(SIZE_IOPS_SEC, size_iops_sec); + +#undef BLOCK_IOTUNE_ASSIGN + ret = 0; endjob: -- 2.8.2
--- src/qemu/qemu_driver.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4ec04ee..db8b9d6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9497,6 +9497,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; + virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainObjPtr vm = NULL; unsigned long long swap_hard_limit; @@ -9508,7 +9509,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, virQEMUDriverConfigPtr cfg = NULL; int rc; int ret = -1; - virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9540,22 +9540,17 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &persistentDef) < 0) + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup memory controller is not mounted")); - goto endjob; - } + if (def && + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cgroup memory controller is not mounted")); + goto endjob; } #define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ @@ -9592,13 +9587,13 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, #define QEMU_SET_MEM_PARAMETER(FUNC, VALUE) \ if (set_ ## VALUE) { \ - if (flags & VIR_DOMAIN_AFFECT_LIVE) { \ + if (def) { \ if ((rc = FUNC(priv->cgroup, VALUE)) < 0) \ goto endjob; \ - vm->def->mem.VALUE = VALUE; \ + def->mem.VALUE = VALUE; \ } \ \ - if (flags & VIR_DOMAIN_AFFECT_CONFIG) \ + if (persistentDef) \ persistentDef->mem.VALUE = VALUE; \ } @@ -9606,7 +9601,7 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, QEMU_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit); /* set hard limit before swap hard limit if decreasing it */ - if (vm->def->mem.hard_limit > hard_limit) { + if (def && def->mem.hard_limit > hard_limit) { QEMU_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit); /* inhibit changing the limit a second time */ set_hard_limit = false; @@ -9619,11 +9614,11 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, #undef QEMU_SET_MEM_PARAMETER - if (flags & VIR_DOMAIN_AFFECT_LIVE && + if (def && virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) goto endjob; - if (flags & VIR_DOMAIN_AFFECT_CONFIG && + if (persistentDef && virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) goto endjob; @@ -9634,7 +9629,6 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(caps); virObjectUnref(cfg); return ret; } -- 2.8.2
--- src/qemu/qemu_driver.c | 65 ++++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index db8b9d6..e8e1418 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9089,10 +9089,10 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; size_t i, j; virDomainObjPtr vm = NULL; + virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; - virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9118,9 +9118,6 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - if ((*nparams) == 0) { /* Current number of blkio parameters supported by cgroups */ *nparams = QEMU_NB_BLKIO_PARAM; @@ -9128,19 +9125,16 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &persistentDef) < 0) + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (def) { if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); goto cleanup; } - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = ¶ms[i]; val = 0; @@ -9155,20 +9149,20 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, break; case 1: /* blkiotune.device_weight */ - if (vm->def->blkio.ndevices > 0) { + if (def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool comma = false; - for (j = 0; j < vm->def->blkio.ndevices; j++) { - if (!vm->def->blkio.devices[j].weight) + for (j = 0; j < def->blkio.ndevices; j++) { + if (!def->blkio.devices[j].weight) continue; if (comma) virBufferAddChar(&buf, ','); else comma = true; virBufferAsprintf(&buf, "%s,%u", - vm->def->blkio.devices[j].path, - vm->def->blkio.devices[j].weight); + def->blkio.devices[j].path, + def->blkio.devices[j].weight); } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -9182,20 +9176,20 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, break; case 2: /* blkiotune.device_read_iops */ - if (vm->def->blkio.ndevices > 0) { + if (def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool comma = false; - for (j = 0; j < vm->def->blkio.ndevices; j++) { - if (!vm->def->blkio.devices[j].riops) + for (j = 0; j < def->blkio.ndevices; j++) { + if (!def->blkio.devices[j].riops) continue; if (comma) virBufferAddChar(&buf, ','); else comma = true; virBufferAsprintf(&buf, "%s,%u", - vm->def->blkio.devices[j].path, - vm->def->blkio.devices[j].riops); + def->blkio.devices[j].path, + def->blkio.devices[j].riops); } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -9209,20 +9203,20 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, break; case 3: /* blkiotune.device_write_iops */ - if (vm->def->blkio.ndevices > 0) { + if (def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool comma = false; - for (j = 0; j < vm->def->blkio.ndevices; j++) { - if (!vm->def->blkio.devices[j].wiops) + for (j = 0; j < def->blkio.ndevices; j++) { + if (!def->blkio.devices[j].wiops) continue; if (comma) virBufferAddChar(&buf, ','); else comma = true; virBufferAsprintf(&buf, "%s,%u", - vm->def->blkio.devices[j].path, - vm->def->blkio.devices[j].wiops); + def->blkio.devices[j].path, + def->blkio.devices[j].wiops); } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -9236,20 +9230,20 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, break; case 4: /* blkiotune.device_read_bps */ - if (vm->def->blkio.ndevices > 0) { + if (def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool comma = false; - for (j = 0; j < vm->def->blkio.ndevices; j++) { - if (!vm->def->blkio.devices[j].rbps) + for (j = 0; j < def->blkio.ndevices; j++) { + if (!def->blkio.devices[j].rbps) continue; if (comma) virBufferAddChar(&buf, ','); else comma = true; virBufferAsprintf(&buf, "%s,%llu", - vm->def->blkio.devices[j].path, - vm->def->blkio.devices[j].rbps); + def->blkio.devices[j].path, + def->blkio.devices[j].rbps); } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -9263,20 +9257,20 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, break; case 5: /* blkiotune.device_write_bps */ - if (vm->def->blkio.ndevices > 0) { + if (def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool comma = false; - for (j = 0; j < vm->def->blkio.ndevices; j++) { - if (!vm->def->blkio.devices[j].wbps) + for (j = 0; j < def->blkio.ndevices; j++) { + if (!def->blkio.devices[j].wbps) continue; if (comma) virBufferAddChar(&buf, ','); else comma = true; virBufferAsprintf(&buf, "%s,%llu", - vm->def->blkio.devices[j].path, - vm->def->blkio.devices[j].wbps); + def->blkio.devices[j].path, + def->blkio.devices[j].wbps); } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -9295,7 +9289,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, /* should not hit here */ } } - } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + } else if (persistentDef) { for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = ¶ms[i]; val = 0; @@ -9486,7 +9480,6 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(caps); return ret; } -- 2.8.2
Get rid of lots of duplicated code. --- src/qemu/qemu_driver.c | 411 +++++++++---------------------------------------- 1 file changed, 75 insertions(+), 336 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e8e1418..a9cfde2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9080,6 +9080,57 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, return ret; } + +static int +qemuDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def, + virTypedParameterPtr params, + int *nparams, + int maxparams) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *data = NULL; + size_t i; + +#define QEMU_BLKIO_ASSIGN(param, format, name) \ + if (*nparams < maxparams) { \ + for (i = 0; i < def->blkio.ndevices; i++) { \ + if (!def->blkio.devices[i].param) \ + continue; \ + virBufferAsprintf(&buf, "%s," format ",", \ + def->blkio.devices[i].path, \ + def->blkio.devices[i].param); \ + } \ + virBufferTrim(&buf, ",", -1); \ + if (virBufferCheckError(&buf) < 0) \ + goto error; \ + data = virBufferContentAndReset(&buf); \ + if (virTypedParameterAssign(&(params[(*nparams)++]), name, \ + VIR_TYPED_PARAM_STRING, data) < 0) \ + goto error; \ + VIR_FREE(data); \ + } + + /* blkiotune.device_weight */ + QEMU_BLKIO_ASSIGN(weight, "%u", VIR_DOMAIN_BLKIO_DEVICE_WEIGHT); + /* blkiotune.device_read_iops */ + QEMU_BLKIO_ASSIGN(riops, "%u", VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS); + /* blkiotune.device_write_iops */ + QEMU_BLKIO_ASSIGN(wiops, "%u", VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS); + /* blkiotune.device_read_bps */ + QEMU_BLKIO_ASSIGN(rbps, "%llu", VIR_DOMAIN_BLKIO_DEVICE_READ_BPS); + /* blkiotune.device_write_bps */ + QEMU_BLKIO_ASSIGN(wbps, "%llu", VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS); + +#undef QEMU_BLKIO_ASSIGN + + return 0; + + error: + VIR_FREE(data); + virBufferFreeAndReset(&buf); + return -1; +} + static int qemuDomainGetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, @@ -9087,10 +9138,10 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - size_t i, j; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; + int maxparams = QEMU_NB_BLKIO_PARAM; unsigned int val; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -9123,8 +9174,12 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, *nparams = QEMU_NB_BLKIO_PARAM; ret = 0; goto cleanup; + } else if (*nparams < maxparams) { + maxparams = *nparams; } + *nparams = 0; + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto cleanup; @@ -9135,347 +9190,31 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } - for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { - virTypedParameterPtr param = ¶ms[i]; - val = 0; - - switch (i) { - case 0: /* fill blkio weight here */ - if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0) - goto cleanup; - if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, - VIR_TYPED_PARAM_UINT, val) < 0) - goto cleanup; - break; - - case 1: /* blkiotune.device_weight */ - if (def->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < def->blkio.ndevices; j++) { - if (!def->blkio.devices[j].weight) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%u", - def->blkio.devices[j].path, - def->blkio.devices[j].weight); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, - VIR_TYPED_PARAM_STRING, - param->value.s) < 0) - goto cleanup; - break; - - case 2: /* blkiotune.device_read_iops */ - if (def->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < def->blkio.ndevices; j++) { - if (!def->blkio.devices[j].riops) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%u", - def->blkio.devices[j].path, - def->blkio.devices[j].riops); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS, - VIR_TYPED_PARAM_STRING, - param->value.s) < 0) - goto cleanup; - break; - - case 3: /* blkiotune.device_write_iops */ - if (def->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < def->blkio.ndevices; j++) { - if (!def->blkio.devices[j].wiops) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%u", - def->blkio.devices[j].path, - def->blkio.devices[j].wiops); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS, - VIR_TYPED_PARAM_STRING, - param->value.s) < 0) - goto cleanup; - break; - - case 4: /* blkiotune.device_read_bps */ - if (def->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < def->blkio.ndevices; j++) { - if (!def->blkio.devices[j].rbps) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%llu", - def->blkio.devices[j].path, - def->blkio.devices[j].rbps); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, - VIR_TYPED_PARAM_STRING, - param->value.s) < 0) - goto cleanup; - break; + /* fill blkio weight here */ + if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0) + goto cleanup; + if (virTypedParameterAssign(&(params[(*nparams)++]), + VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, val) < 0) + goto cleanup; - case 5: /* blkiotune.device_write_bps */ - if (def->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < def->blkio.ndevices; j++) { - if (!def->blkio.devices[j].wbps) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%llu", - def->blkio.devices[j].path, - def->blkio.devices[j].wbps); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (virTypedParameterAssign(param, - VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS, - VIR_TYPED_PARAM_STRING, - param->value.s) < 0) - goto cleanup; - break; + if (qemuDomainGetBlkioParametersAssignFromDef(def, params, nparams, + maxparams) < 0) + goto cleanup; - /* coverity[dead_error_begin] */ - default: - break; - /* should not hit here */ - } - } } else if (persistentDef) { - for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { - virTypedParameterPtr param = ¶ms[i]; - val = 0; - param->value.ui = 0; - param->type = VIR_TYPED_PARAM_UINT; - - switch (i) { - case 0: /* fill blkio weight here */ - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_WEIGHT); - goto cleanup; - } - param->value.ui = persistentDef->blkio.weight; - break; - - case 1: /* blkiotune.device_weight */ - if (persistentDef->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < persistentDef->blkio.ndevices; j++) { - if (!persistentDef->blkio.devices[j].weight) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%u", - persistentDef->blkio.devices[j].path, - persistentDef->blkio.devices[j].weight); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) - goto cleanup; - param->type = VIR_TYPED_PARAM_STRING; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_WEIGHT); - goto cleanup; - } - break; - - case 2: /* blkiotune.device_read_iops */ - if (persistentDef->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < persistentDef->blkio.ndevices; j++) { - if (!persistentDef->blkio.devices[j].riops) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%u", - persistentDef->blkio.devices[j].path, - persistentDef->blkio.devices[j].riops); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) - goto cleanup; - param->type = VIR_TYPED_PARAM_STRING; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS); - goto cleanup; - } - break; - case 3: /* blkiotune.device_write_iops */ - if (persistentDef->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < persistentDef->blkio.ndevices; j++) { - if (!persistentDef->blkio.devices[j].wiops) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%u", - persistentDef->blkio.devices[j].path, - persistentDef->blkio.devices[j].wiops); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) - goto cleanup; - param->type = VIR_TYPED_PARAM_STRING; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS); - goto cleanup; - } - break; - case 4: /* blkiotune.device_read_bps */ - if (persistentDef->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < persistentDef->blkio.ndevices; j++) { - if (!persistentDef->blkio.devices[j].rbps) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%llu", - persistentDef->blkio.devices[j].path, - persistentDef->blkio.devices[j].rbps); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) - goto cleanup; - param->type = VIR_TYPED_PARAM_STRING; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_READ_BPS); - goto cleanup; - } - break; - - case 5: /* blkiotune.device_write_bps */ - if (persistentDef->blkio.ndevices > 0) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - bool comma = false; - - for (j = 0; j < persistentDef->blkio.ndevices; j++) { - if (!persistentDef->blkio.devices[j].wbps) - continue; - if (comma) - virBufferAddChar(&buf, ','); - else - comma = true; - virBufferAsprintf(&buf, "%s,%llu", - persistentDef->blkio.devices[j].path, - persistentDef->blkio.devices[j].wbps); - } - if (virBufferCheckError(&buf) < 0) - goto cleanup; - param->value.s = virBufferContentAndReset(&buf); - } - if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) - goto cleanup; - param->type = VIR_TYPED_PARAM_STRING; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS); - goto cleanup; - } - break; - + /* fill blkio weight here */ + if (virTypedParameterAssign(&(params[(*nparams)++]), + VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, + persistentDef->blkio.weight) < 0) + goto cleanup; - /* coverity[dead_error_begin] */ - default: - break; - /* should not hit here */ - } - } + if (qemuDomainGetBlkioParametersAssignFromDef(persistentDef, params, + nparams, maxparams) < 0) + goto cleanup; } - if (QEMU_NB_BLKIO_PARAM < *nparams) - *nparams = QEMU_NB_BLKIO_PARAM; ret = 0; cleanup: -- 2.8.2
On Wed, May 25, 2016 at 03:04:04PM +0200, Peter Krempa wrote:
Get rid of lots of duplicated code. --- src/qemu/qemu_driver.c | 411 +++++++++---------------------------------------- 1 file changed, 75 insertions(+), 336 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e8e1418..a9cfde2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9080,6 +9080,57 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, return ret; }
+
Two empty lines here...
+static int +qemuDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def, + virTypedParameterPtr params, + int *nparams, + int maxparams)
...
+ error: + VIR_FREE(data); + virBufferFreeAndReset(&buf); + return -1; +} +
... but only one here.
static int qemuDomainGetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params,
...
- if (virTypedParameterAssign(param, - VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, - VIR_TYPED_PARAM_STRING, - param->value.s) < 0) - goto cleanup; - break; + /* fill blkio weight here */
The indentation is off.
+ if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0) + goto cleanup; + if (virTypedParameterAssign(&(params[(*nparams)++]), + VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, val) < 0) + goto cleanup;
...
- virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS); - goto cleanup; - } - break; - + /* fill blkio weight here */
Here too. Jan
--- src/qemu/qemu_driver.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a9cfde2..0f79dcd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10336,7 +10336,7 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, bool cpu_bw_status = false; int saved_nparams = 0; virDomainDefPtr persistentDef; - virCapsPtr caps = NULL; + virDomainDefPtr def; qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -10363,14 +10363,10 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, if (*nparams > 1) cpu_bw_status = virCgroupSupportsCpuBW(priv->cgroup); - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &persistentDef) < 0) + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (persistentDef) { shares = persistentDef->cputune.shares; if (*nparams > 1) { period = persistentDef->cputune.period; @@ -10476,7 +10472,6 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(caps); return ret; } -- 2.8.2
cgroup functions set and get the longer type so use it everywhere --- src/conf/domain_conf.c | 6 +++--- src/conf/domain_conf.h | 2 +- src/lxc/lxc_native.c | 2 +- src/vmx/vmx.c | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fb05bf7..7108c7e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15528,8 +15528,8 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; /* Extract cpu tunables. */ - if ((n = virXPathULong("string(./cputune/shares[1])", ctxt, - &def->cputune.shares)) < -1) { + if ((n = virXPathULongLong("string(./cputune/shares[1])", ctxt, + &def->cputune.shares)) < -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("can't parse cputune shares value")); goto error; @@ -22158,7 +22158,7 @@ virDomainCputuneDefFormat(virBufferPtr buf, virBufferAdjustIndent(&childrenBuf, virBufferGetIndent(buf, false) + 2); if (def->cputune.sharesSpecified) - virBufferAsprintf(&childrenBuf, "<shares>%lu</shares>\n", + virBufferAsprintf(&childrenBuf, "<shares>%llu</shares>\n", def->cputune.shares); if (def->cputune.period) virBufferAsprintf(&childrenBuf, "<period>%llu</period>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 82e581b..4e0550c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2011,7 +2011,7 @@ typedef struct _virDomainCputune virDomainCputune; typedef virDomainCputune *virDomainCputunePtr; struct _virDomainCputune { - unsigned long shares; + unsigned long long shares; bool sharesSpecified; unsigned long long period; long long quota; diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 0bea32e..5f9e50e 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -800,7 +800,7 @@ lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties) if ((value = virConfGetValue(properties, "lxc.cgroup.cpu.shares")) && value->str) { - if (virStrToLong_ul(value->str, NULL, 10, &def->cputune.shares) < 0) + if (virStrToLong_ull(value->str, NULL, 10, &def->cputune.shares) < 0) goto error; def->cputune.sharesSpecified = true; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index f729e5d..68a199f 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1547,8 +1547,8 @@ virVMXParseConfig(virVMXContext *ctx, def->cputune.shares = vcpus * 1000; } else if (STRCASEEQ(sched_cpu_shares, "high")) { def->cputune.shares = vcpus * 2000; - } else if (virStrToLong_ul(sched_cpu_shares, NULL, 10, - &def->cputune.shares) < 0) { + } else if (virStrToLong_ull(sched_cpu_shares, NULL, 10, + &def->cputune.shares) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting VMX entry 'sched.cpu.shares' to be an " "unsigned integer or 'low', 'normal' or 'high' but " @@ -3251,7 +3251,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe } else if (def->cputune.shares == vcpus * 2000) { virBufferAddLit(&buffer, "sched.cpu.shares = \"high\"\n"); } else { - virBufferAsprintf(&buffer, "sched.cpu.shares = \"%lu\"\n", + virBufferAsprintf(&buffer, "sched.cpu.shares = \"%llu\"\n", def->cputune.shares); } } -- 2.8.2
Use virDomainCputune struct to store the data rather than exploding the fields and use macros to fill the typed params. --- src/qemu/qemu_driver.c | 143 +++++++++++++++---------------------------------- 1 file changed, 42 insertions(+), 101 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0f79dcd..71d25a4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10324,28 +10324,20 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - unsigned long long shares; - unsigned long long period; - long long quota; - unsigned long long global_period; - long long global_quota; - unsigned long long emulator_period; - long long emulator_quota; + virDomainCputune data; int ret = -1; - int rc; - bool cpu_bw_status = false; - int saved_nparams = 0; + bool cpu_bw_status = true; virDomainDefPtr persistentDef; virDomainDefPtr def; qemuDomainObjPrivatePtr priv; + int maxparams = *nparams; + + *nparams = 0; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | VIR_TYPED_PARAM_STRING_OKAY, -1); - /* We don't return strings, and thus trivially support this flag. */ - flags &= ~VIR_TYPED_PARAM_STRING_OKAY; - if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -10360,113 +10352,62 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - if (*nparams > 1) - cpu_bw_status = virCgroupSupportsCpuBW(priv->cgroup); - if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto cleanup; if (persistentDef) { - shares = persistentDef->cputune.shares; - if (*nparams > 1) { - period = persistentDef->cputune.period; - quota = persistentDef->cputune.quota; - global_period = persistentDef->cputune.global_period; - global_quota = persistentDef->cputune.global_quota; - emulator_period = persistentDef->cputune.emulator_period; - emulator_quota = persistentDef->cputune.emulator_quota; - cpu_bw_status = true; /* Allow copy of data to params[] */ - } - goto out; - } - - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup CPU controller is not mounted")); - goto cleanup; - } - - if (virCgroupGetCpuShares(priv->cgroup, &shares) < 0) - goto cleanup; - - if (*nparams > 1 && cpu_bw_status) { - rc = qemuGetVcpusBWLive(vm, &period, "a); - if (rc != 0) - goto cleanup; - } - - if (*nparams > 3 && cpu_bw_status) { - rc = qemuGetEmulatorBandwidthLive(priv->cgroup, &emulator_period, - &emulator_quota); - if (rc != 0) + data = persistentDef->cputune; + } else if (def) { + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup CPU controller is not mounted")); goto cleanup; - } + } - if (*nparams > 5 && cpu_bw_status) { - rc = qemuGetGlobalBWLive(priv->cgroup, &global_period, &global_quota); - if (rc != 0) + if (virCgroupGetCpuShares(priv->cgroup, &data.shares) < 0) goto cleanup; - } - - out: - if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_CPU_SHARES, - VIR_TYPED_PARAM_ULLONG, shares) < 0) - goto cleanup; - saved_nparams++; - if (cpu_bw_status) { - if (*nparams > saved_nparams) { - if (virTypedParameterAssign(¶ms[1], - VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, - VIR_TYPED_PARAM_ULLONG, period) < 0) + if (virCgroupSupportsCpuBW(priv->cgroup)) { + if (maxparams > 1 && + qemuGetVcpusBWLive(vm, &data.period, &data.quota) < 0) goto cleanup; - saved_nparams++; - } - if (*nparams > saved_nparams) { - if (virTypedParameterAssign(¶ms[2], - VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, - VIR_TYPED_PARAM_LLONG, quota) < 0) + if (maxparams > 3 && + qemuGetEmulatorBandwidthLive(priv->cgroup, &data.emulator_period, + &data.emulator_quota) < 0) goto cleanup; - saved_nparams++; - } - if (*nparams > saved_nparams) { - if (virTypedParameterAssign(¶ms[3], - VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, - VIR_TYPED_PARAM_ULLONG, - emulator_period) < 0) + if (maxparams > 5 && + qemuGetGlobalBWLive(priv->cgroup, &data.global_period, + &data.global_quota) < 0) goto cleanup; - saved_nparams++; + } else { + cpu_bw_status = false; } + } - if (*nparams > saved_nparams) { - if (virTypedParameterAssign(¶ms[4], - VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, - VIR_TYPED_PARAM_LLONG, - emulator_quota) < 0) - goto cleanup; - saved_nparams++; - } +#define QEMU_SCHED_ASSIGN(param, name, type) \ + if (*nparams < maxparams && \ + virTypedParameterAssign(&(params[(*nparams)++]), \ + VIR_DOMAIN_SCHEDULER_ ## name, \ + VIR_TYPED_PARAM_ ## type, \ + data.param) < 0) \ + goto cleanup - if (*nparams > saved_nparams) { - if (virTypedParameterAssign(¶ms[5], - VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD, - VIR_TYPED_PARAM_ULLONG, global_period) < 0) - goto cleanup; - saved_nparams++; - } + QEMU_SCHED_ASSIGN(shares, CPU_SHARES, ULLONG); - if (*nparams > saved_nparams) { - if (virTypedParameterAssign(¶ms[6], - VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA, - VIR_TYPED_PARAM_LLONG, global_quota) < 0) - goto cleanup; - saved_nparams++; - } + if (cpu_bw_status) { + QEMU_SCHED_ASSIGN(period, VCPU_PERIOD, ULLONG); + QEMU_SCHED_ASSIGN(quota, VCPU_QUOTA, LLONG); + + QEMU_SCHED_ASSIGN(emulator_period, EMULATOR_PERIOD, ULLONG); + QEMU_SCHED_ASSIGN(emulator_quota, EMULATOR_QUOTA, LLONG); + + QEMU_SCHED_ASSIGN(global_period, GLOBAL_PERIOD, ULLONG); + QEMU_SCHED_ASSIGN(global_quota, GLOBAL_QUOTA, LLONG); } - *nparams = saved_nparams; +#undef QEMU_SCHED_ASSIGN ret = 0; -- 2.8.2
--- src/qemu/qemu_driver.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 71d25a4..0fbce1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17090,6 +17090,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; qemuDomainObjPrivatePtr priv; + virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo info; virDomainBlockIoTuneInfo *oldinfo; @@ -17105,7 +17106,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, bool set_size_iops = false; bool supportMaxOptions = true; virQEMUDriverConfigPtr cfg = NULL; - virCapsPtr caps = NULL; virObjectEventPtr event = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; @@ -17158,11 +17158,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, priv = vm->privateData; - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto endjob; - - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &persistentDef) < 0) + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams, @@ -17330,7 +17326,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (persistentDef) { if (!(conf_disk = virDomainDiskByName(persistentDef, path, true))) { virReportError(VIR_ERR_INVALID_ARG, _("missing persistent configuration for disk '%s'"), @@ -17339,7 +17335,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (def) { supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX); if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { @@ -17357,7 +17353,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } - if (!(disk = qemuDomainDiskByName(vm->def, path))) + if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; if (!(device = qemuAliasFromDisk(disk))) @@ -17409,7 +17405,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (persistentDef) { oldinfo = &conf_disk->blkdeviotune; if (!set_bytes) { info.total_bytes_sec = oldinfo->total_bytes_sec; @@ -17436,7 +17432,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); - virObjectUnref(caps); virObjectUnref(cfg); return ret; } -- 2.8.2
This refactor also makes a distinction between the pointer to the original definition and copied one to prevent mixups. --- src/qemu/qemu_driver.c | 93 +++++++++++++++++++++++++------------------------- 1 file changed, 47 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0fbce1b..a828a13 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9961,7 +9961,9 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; size_t i; virDomainObjPtr vm = NULL; - virDomainDefPtr vmdef = NULL; + virDomainDefPtr def = NULL; + virDomainDefPtr persistentDef = NULL; + virDomainDefPtr persistentDefCopy = NULL; unsigned long long value_ul; long long value_l; int ret = -1; @@ -10015,22 +10017,21 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, - &vmdef) < 0) + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (persistentDef) { /* Make a copy for updated domain. */ - if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt))) + if (!(persistentDefCopy = virDomainObjCopyPersistentDef(vm, caps, + driver->xmlopt))) goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup CPU controller is not mounted")); - goto endjob; - } + if (def && + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cgroup CPU controller is not mounted")); + goto endjob; } for (i = 0; i < nparams; i++) { @@ -10039,7 +10040,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, value_l = param->value.l; if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { - if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (def) { unsigned long long val; if (virCgroupSetCpuShares(priv->cgroup, value_ul) < 0) goto endjob; @@ -10047,8 +10048,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (virCgroupGetCpuShares(priv->cgroup, &val) < 0) goto endjob; - vm->def->cputune.shares = val; - vm->def->cputune.sharesSpecified = true; + def->cputune.shares = val; + def->cputune.sharesSpecified = true; if (virTypedParamsAddULLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10057,9 +10058,9 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - vmdef->cputune.shares = value_ul; - vmdef->cputune.sharesSpecified = true; + if (persistentDef) { + persistentDefCopy->cputune.shares = value_ul; + persistentDefCopy->cputune.sharesSpecified = true; } @@ -10067,11 +10068,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); - if (flags & VIR_DOMAIN_AFFECT_LIVE && value_ul) { + if (def && value_ul) { if ((rc = qemuSetVcpusBWLive(vm, priv->cgroup, value_ul, 0))) goto endjob; - vm->def->cputune.period = value_ul; + def->cputune.period = value_ul; if (virTypedParamsAddULLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10080,18 +10081,18 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - vmdef->cputune.period = params[i].value.ul; + if (persistentDef) + persistentDefCopy->cputune.period = params[i].value.ul; } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); - if (flags & VIR_DOMAIN_AFFECT_LIVE && value_l) { + if (def && value_l) { if ((rc = qemuSetVcpusBWLive(vm, priv->cgroup, 0, value_l))) goto endjob; - vm->def->cputune.quota = value_l; + def->cputune.quota = value_l; if (virTypedParamsAddLLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10100,18 +10101,18 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - vmdef->cputune.quota = value_l; + if (persistentDef) + persistentDefCopy->cputune.quota = value_l; } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD)) { SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD, QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); - if (flags & VIR_DOMAIN_AFFECT_LIVE && value_ul) { + if (def && value_ul) { if ((rc = qemuSetGlobalBWLive(priv->cgroup, value_ul, 0))) goto endjob; - vm->def->cputune.global_period = value_ul; + def->cputune.global_period = value_ul; if (virTypedParamsAddULLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10120,18 +10121,18 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - vmdef->cputune.period = params[i].value.ul; + if (persistentDef) + persistentDefCopy->cputune.period = params[i].value.ul; } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA, QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); - if (flags & VIR_DOMAIN_AFFECT_LIVE && value_l) { + if (def && value_l) { if ((rc = qemuSetGlobalBWLive(priv->cgroup, 0, value_l))) goto endjob; - vm->def->cputune.global_quota = value_l; + def->cputune.global_quota = value_l; if (virTypedParamsAddLLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10140,19 +10141,19 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - vmdef->cputune.global_quota = value_l; + if (persistentDef) + persistentDefCopy->cputune.global_quota = value_l; } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); - if (flags & VIR_DOMAIN_AFFECT_LIVE && value_ul) { + if (def && value_ul) { if ((rc = qemuSetEmulatorBandwidthLive(priv->cgroup, value_ul, 0))) goto endjob; - vm->def->cputune.emulator_period = value_ul; + def->cputune.emulator_period = value_ul; if (virTypedParamsAddULLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10161,19 +10162,19 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - vmdef->cputune.emulator_period = value_ul; + if (persistentDef) + persistentDefCopy->cputune.emulator_period = value_ul; } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); - if (flags & VIR_DOMAIN_AFFECT_LIVE && value_l) { + if (def && value_l) { if ((rc = qemuSetEmulatorBandwidthLive(priv->cgroup, 0, value_l))) goto endjob; - vm->def->cputune.emulator_quota = value_l; + def->cputune.emulator_quota = value_l; if (virTypedParamsAddLLong(&eventParams, &eventNparams, &eventMaxNparams, @@ -10182,8 +10183,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, goto endjob; } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - vmdef->cputune.emulator_quota = value_l; + if (persistentDef) + persistentDefCopy->cputune.emulator_quota = value_l; } } @@ -10196,13 +10197,13 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, qemuDomainEventQueue(driver, event); } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - rc = virDomainSaveConfig(cfg->configDir, driver->caps, vmdef); + if (persistentDef) { + rc = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDefCopy); if (rc < 0) goto endjob; - virDomainObjAssignDef(vm, vmdef, false, NULL); - vmdef = NULL; + virDomainObjAssignDef(vm, persistentDefCopy, false, NULL); + persistentDefCopy = NULL; } ret = 0; @@ -10211,7 +10212,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - virDomainDefFree(vmdef); + virDomainDefFree(persistentDefCopy); virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); -- 2.8.2
On Wed, May 25, 2016 at 03:03:58PM +0200, Peter Krempa wrote:
Don't use that terrible function in the qemu driver and a few relevant refactors.
Peter Krempa (11): qemu: monitor: Remove 'supportMaxOptions' argument from qemuMonitorGetBlockIoThrottle qemu: Replace virDomainLiveConfigHelperMethod in qemuDomainGetBlockIoTune qemu: Refactor typed params assignment in qemuDomainGetBlockIoTune qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainSetMemoryParameters qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainGetBlkioParameters qemu: Refactor qemuDomainGetBlkioParameters qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainGetSchedulerParametersFlags conf: Change virDomainCputune member 'shares' to unsigned long long qemu: Refactor qemuDomainGetSchedulerParametersFlags qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainSetBlockIoTune qemu: Remove virDomainLiveConfigHelperMethod from qemuDomainSetSchedulerParametersFlags
src/conf/domain_conf.c | 6 +- src/conf/domain_conf.h | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/qemu_driver.c | 896 ++++++++++++------------------------------- src/qemu/qemu_monitor.c | 5 +- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_monitor_json.c | 24 +- src/qemu/qemu_monitor_json.h | 3 +- src/vmx/vmx.c | 6 +- tests/qemumonitorjsontest.c | 2 +- 10 files changed, 261 insertions(+), 688 deletions(-)
ACK series Jan
participants (2)
-
Ján Tomko -
Peter Krempa