Re: [libvirt] [PATCHv2 07/14] Fix vmdef usage while in monitor in BlockStat* APIs

<No commit message>? Seems we're fixing two issues here #1. The ExitMonitor issue #2. Somehow the disk->info.alias becomes NULL and thus we're making a local copy to avoid that. On 01/07/2015 10:42 AM, Ján Tomko wrote: > --- > src/qemu/qemu_driver.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) >

> @@ -10231,6 +10238,8 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, > disk->dst); > goto endjob; > } > + if (VIR_STRDUP(diskAlias, disk->info.alias) < 0) > + goto endjob; so diskAlias is only set if *nparams != 0, but... > } > > priv = vm->privateData; > @@ -10241,12 +10250,12 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, > ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams); > > if (tmp == 0 || ret < 0) { > - qemuDomainObjExitMonitor(driver, vm); > + ignore_value(qemuDomainObjExitMonitor(driver, vm)); Can tmp == 0 and ret == 0? Thus when go to endjob and eventually exit we don't a failure? Does it matter? I suppose for consistency it does.

> goto endjob; > } > > ret = qemuMonitorGetBlockStatsInfo(priv->mon, > - disk->info.alias, > + diskAlias, ...used here regardless of *nparams... Can we get here with diskAlias == NULL?

> &rd_req, > &rd_bytes, > &rd_total_times, > @@ -10257,7 +10266,8 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, > &flush_total_times, > &errs); > > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + ret = -1; > > if (ret < 0) > goto endjob; > @@ -10342,6 +10352,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, > qemuDomainObjEndJob(driver, vm); > > cleanup: > + VIR_FREE(diskAlias); > qemuDomObjEndAPI(&vm); > return ret; > } > @@ -16853,7 +16864,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, > qemuDomainObjEnterMonitor(driver, vm); > ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, > &info, supportMaxOptions); > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto endjob; Potentially leaving ret = 0 from the IoThrottle call... Should just set ret = -1 (liek done in qemuDomainBlockStatsFlags above)

ACK with a commit message and the issues resolved. John > if (ret < 0) > goto endjob; > vm->def->disks[idx]->blkdeviotune = info; > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list