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

Re: [libvirt] PATCH: Fix remote driver create/destroy methods to update ID field



On Fri, Apr 17, 2009 at 01:30:43PM +0200, Daniel Veillard wrote:
> On Fri, Apr 17, 2009 at 12:12:14PM +0100, Daniel P. Berrange wrote:
> > If you have an existing virDomainPtr object, and start it using the
> > virDomainCreate(virDomainPtr dom) method, then internal cached 'id'
> > field in the virDomainPtr object is never updated. Even more annoyingly
> > the remote protocol for the 'create' method doesn't even bother to
> > return the ID of the newly started guest.  Thre is a similar problem
> > with the virDomainDestroy method, not resetting the cached 'id' back
> > to -1.
> > 
> > We can't guarentee that the 'id' field is up2date wrt to changes made
> > by another libvirt client, but we can at least make sure its accurate
> > wrt to changes this client is making. For the destroy method the fix
> > is trivial. For the create method, after a successful creation, we do
> > a lookup based on UUID to fetch the real live ID, since the create
> > method didn't return it for us.
> > 
> > This fixes a significant number of problems identified by the TCK on
> > the QEMU driver usage.
> > @@ -2746,6 +2749,16 @@ remoteDomainCreate (virDomainPtr domain)
> >                (xdrproc_t) xdr_void, (char *) NULL) == -1)
> >          goto done;
> 
>   maybe a small comment here about why we are doing a remote lookup
> might be a good idea, something as simple as
> 
>        /* we need to update the id of the local docmain */
> > +    memcpy (args2.uuid, domain->uuid, VIR_UUID_BUFLEN);
> > +    memset (&ret2, 0, sizeof ret2);
> > +    if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_LOOKUP_BY_UUID,
> > +              (xdrproc_t) xdr_remote_domain_lookup_by_uuid_args, (char *) &args2,
> > +              (xdrproc_t) xdr_remote_domain_lookup_by_uuid_ret, (char *) &ret2) == -1)
> > +        goto done;
> > +
> > +    domain->id = ret2.dom.id;
> > +    xdr_free ((xdrproc_t) &xdr_remote_domain_lookup_by_uuid_ret, (char *) &ret2);
> > +
> 
>   I was just wondering, shouldn't something similar be done on remote
> virDomainRestore() but since we don't use a virDomainPtr as the input
> the domain info has to be refetched from scratch and then should be
> immune to the problem.
> 
>   ACK

I committed this patch, including a comment as you suggested

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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