Re: [libvirt] [PATCH 2/2] qemu: Fix DEATHCODE error in qemuDomainGetBlockIoTune.

Need to fix the commit message to something like "Resolve Coverity DEADCODE" (something I just realized for 1/2 which could read Resolve Coverity COPY_PASTE_ERROR" Something I can take care of when pushing. On 11/12/2014 08:04 AM, Matthias Gatto wrote: > reported here: http://www.redhat.com/archives/libvir-list/2014-November/msg00327.html > > I could have just remove bool supportMaxOptions variable, but > if I had do this, we could not check anymore if the nparams variable is > superior to QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX. > > Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; > --- > src/qemu/qemu_driver.c | 1 + > 1 file changed, 1 insertion(+) > While the following does work... > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 56e8430..61c4af6 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -17024,6 +17024,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, > > if (flags & VIR_DOMAIN_AFFECT_LIVE) { > priv = vm->privateData; > + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX); > qemuDomainObjEnterMonitor(driver, vm); > ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions); > qemuDomainObjExitMonitor(driver, vm); > Would perhaps the following be "cleaner": diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 56e8430..5124e56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17003,14 +17003,15 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, &persistentDef) < 0) goto endjob; + /* 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. */ + priv = vm->privateData; + if (flags & VIR_DOMAIN_AFFECT_LIVE) + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_DRIVE_IOTUNE_MAX); + 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; @@ -17023,7 +17024,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions); qemuDomainObjExitMonitor(driver, vm); The difference being moving the setting of 'priv' to be made regardless of flags and supportMaxOptions only once prior to using in either place. The second sentence of your comment is a bit confusing too - I assume the "VM data" you are referencing is the vm obtained much earlier. Or are you trying to indicate that perhaps one of the interceding calls is necessary? In any case, does the above look reasonable? Tks - John