On 01/12/2015 11:30 PM, John Ferlan wrote:
<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.
Just a few lines above *nparams gets assigned to tmp. So if no block stats were requested, there's nothing to return back to the user and no need to fail.
goto endjob; }
ret = qemuMonitorGetBlockStatsInfo(priv->mon, - disk->info.alias, + diskAlias,
...used here regardless of *nparams... Can we get here with diskAlias == NULL?
No, if tmp == 0 we jump to endjob.
&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)
I've fixed that and wrote a commit message. Jan
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list