Re: [PATCH] Fix regressions BlockStats/Info APIs in QEMU driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/02/2011 10:03 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> The change 18c2a592064d69499f70428e498f4a3cb5161cda caused
> some regressions in behaviour of virDomainBlockStats
> and virDomainBlockInfo in the QEMU driver.
> 
> The virDomainBlockInfo API stopped working for inactive
> guests if querying a block device.
> 
> The virDomainBlockStats API did not promptly report
> an error if the guest was not running in some cases.
> 
> * src/qemu/qemu_driver.c: Fix inactive guest handling
>   in BlockStats/Info APIs
> ---
>  src/qemu/qemu_driver.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> @@ -6017,10 +6024,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
>              if (qemuDomainObjBeginJob(vm) < 0)
>                  goto cleanup;
>  
> -            if (!virDomainObjIsActive(vm)) {
> -                qemuReportError(VIR_ERR_OPERATION_INVALID,
> -                                "%s", _("domain is not running"));
> -                goto endjob;
> +            if (virDomainObjIsActive(vm)) {
> +                ret = 0;
> +                goto cleanup;

Oops - you lost the ! in that conditional.  Also, 'goto cleanup' forgets
to end the job condition that we hold.  The real answer is that if the
domain is not active, we set ret to 0 and short-circuit the attempt to
query the guest.

Conditional ACK if you change this hunk to be:

if (!virDomainObjIsActive(vm)) {
    ret = 0;
    if (qemuDomainObjEndJob(vm) == 0)
        vm = NULL;
    goto cleanup;
}

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]