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

[Daniel P. Berrange]

On Thu, Jun 02, 2011 at 01:27:46PM -0600, Eric Blake wrote:
> On 06/02/2011 10:03 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > 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;
> }

We can actually remove the goto entirely, so I changed
it to this:

            if (qemuDomainObjBeginJob(vm) < 0)
                goto cleanup;

            if (virDomainObjIsActive(vm)) {
                qemuDomainObjEnterMonitor(vm);
                ret = qemuMonitorGetBlockExtent(priv->mon,
                                                disk->info.alias,
                                                &info->allocation);
		qemuDomainObjExitMonitor(vm);
            } else {
                ret = 0;
	    }

            if (qemuDomainObjEndJob(vm) == 0)
                vm = NULL;


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|