[libvirt] [PATCH v7 4/7] qemu: Add bps_max and friends qemu driver
John Ferlan
jferlan at redhat.com
Tue Nov 11 12:31:24 UTC 2014
On 11/10/2014 10:19 AM, Matthias Gatto wrote:
> Add support for bps_max and friends in the driver part.
> In the part checking if a qemu is running, check if the running binary
> support bps_max, if not print an error message, if yes add it to
> "info" variable
>
> This patch follow therse patchs: http://www.redhat.com/archives/libvir-list/2014-November/msg00271.html
> I've fix the syntax error and the nparams detection problem
>
> Signed-off-by: Matthias Gatto <matthias.gatto at outscale.com>
> ---
> include/libvirt/libvirt-domain.h | 56 +++++++++++
> src/qemu/qemu_driver.c | 197 ++++++++++++++++++++++++++++++++++++---
> src/qemu/qemu_monitor.c | 10 +-
> src/qemu/qemu_monitor.h | 6 +-
> src/qemu/qemu_monitor_json.c | 11 ++-
> src/qemu/qemu_monitor_json.h | 6 +-
> tests/qemumonitorjsontest.c | 4 +-
> 7 files changed, 264 insertions(+), 26 deletions(-)
>
Coverity is unhappy here too - DEADCODE...
<...snip...>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5eccacc..242b30e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
<...snip...>
> @@ -16753,13 +16870,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> {
> virQEMUDriverPtr driver = dom->conn->privateData;
> virDomainObjPtr vm = NULL;
> - qemuDomainObjPrivatePtr priv;
> + qemuDomainObjPrivatePtr priv = NULL;
> virDomainDefPtr persistentDef = NULL;
> virDomainBlockIoTuneInfo reply;
> char *device = NULL;
> int ret = -1;
> size_t i;
> virCapsPtr caps = NULL;
(1) Event assignment: Assigning: "supportMaxOptions" = "true".
Also see events:
> + bool supportMaxOptions = true;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> VIR_DOMAIN_AFFECT_CONFIG |
> @@ -16777,13 +16895,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> goto cleanup;
>
> - if ((*nparams) == 0) {
> - /* Current number of parameters supported by QEMU Block I/O Throttling */
> - *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
> - ret = 0;
> - goto cleanup;
> - }
> -
> if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> goto cleanup;
>
> @@ -16791,6 +16902,18 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> &persistentDef) < 0)
> goto endjob;
>
> + if ((*nparams) == 0) {
> + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> + priv = vm->privateData;
> + /* 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 the VM data to do so */
> + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);
> + }
> + *nparams = supportMaxOptions ? QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX : QEMU_NB_BLOCK_IO_TUNE_PARAM;
> + ret = 0;
> + goto endjob;
^^^^
The only way supportMaxOptions can change (be false) is in this block;
however, the "goto endjob;" here jumps around the later check regarding
where Coverity complains about DEADCODE
Less sure how to handle in this case, but my gut says the (!supportMaxOptions &&
check below is unnecessary.
THoughts?
John
> + }
> +
> device = qemuDiskPathToAlias(vm, disk, NULL);
> if (!device) {
> goto endjob;
> @@ -16799,7 +16922,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> priv = vm->privateData;
> qemuDomainObjEnterMonitor(driver, vm);
> - ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply);
> + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions);
> qemuDomainObjExitMonitor(driver, vm);
> if (ret < 0)
> goto endjob;
> @@ -16816,7 +16939,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> reply = persistentDef->disks[idx]->blkdeviotune;
> }
>
> - for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) {
> + for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX && i < *nparams; i++) {
> virTypedParameterPtr param = ¶ms[i];
>
> switch (i) {
> @@ -16862,14 +16985,64 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> 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;
> }
> }
>
17144
(2) Event const: At condition "supportMaxOptions", the value of "supportMaxOptions" must be equal to 1.
(3) Event dead_error_condition: The condition "!supportMaxOptions" cannot be true.
(4) Event dead_error_line: Execution cannot reach the expression "*nparams > 6" inside this statement: "if (!supportMaxOptions && *...".
Also see events: [assignment]
17145 if (!supportMaxOptions && *nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM)
> - if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM)
> + 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;
> ret = 0;
>
> endjob:
<...snip...>
More information about the libvir-list
mailing list