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