[libvirt] [PATCH 4/5] qemu: Refactor qemuDomainGetBlockInfo

[Peter Krempa]

On Thu, Jun 25, 2015 at 13:26:35 -0400, John Ferlan wrote:
> 
> 
> On 06/23/2015 01:15 PM, Peter Krempa wrote:
> > Change the code so that it queries the monitor when the VM is alive.
> > ---
> >  src/qemu/qemu_driver.c | 90 +++++++++++++++++++++++++++++---------------------
> >  1 file changed, 53 insertions(+), 37 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 004da7e..3da941e 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -11800,10 +11800,12 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> >      virQEMUDriverPtr driver = dom->conn->privateData;
> >      virDomainObjPtr vm;
> >      int ret = -1;
> > -    virDomainDiskDefPtr disk = NULL;
> > -    virStorageSourcePtr src;
> > -    bool activeFail = false;
> > +    virDomainDiskDefPtr disk;
> >      virQEMUDriverConfigPtr cfg = NULL;
> > +    int rc;
> > +    virHashTablePtr stats = NULL;
> > +    qemuBlockStats *entry;
> > +    char *alias;
> > 
> >      virCheckFlags(0, -1);
> > 
> > @@ -11815,11 +11817,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> >      if (virDomainGetBlockInfoEnsureACL(dom->conn, vm->def) < 0)
> >          goto cleanup;
> > 
> > -    /* Technically, we only need a job if we are going to query the
> > -     * monitor, which is only for active domains that are using
> > -     * non-raw block devices.  But it is easier to share code if we
> > -     * always grab a job; furthermore, grabbing the job ensures that
> > -     * hot-plug won't change disk behind our backs.  */
> 
> Was the comment wrong? I tend to like comments like this, since it gives
> me an understanding why something was done a particular way...

qemuStorageLimitsRefresh modifies few fields of the disk->src structure.

They are only integer modifications of capacity and they are guarded by
the vm lock but that still should be covered by a job semantically.
There are only very few exceptions to that rule.

Additionally, grabbing the job helps to prevent the situation where we'd
check that the VM is alive (where we do need a job to query the
monitor), then we'd enter the job, but the vm would die at that precise
moment and the rollback path from that situation would be convoluted.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150625/040de100/attachment-0001.sig>