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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list