[libvirt] PATCH: Fix remote driver create/destroy methods to update ID field
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. Regards, Daniel diff -r 9ff786a7908d src/remote_internal.c --- a/src/remote_internal.c Fri Apr 17 12:05:11 2009 +0100 +++ b/src/remote_internal.c Fri Apr 17 12:06:21 2009 +0100 @@ -1939,6 +1939,7 @@ remoteDomainDestroy (virDomainPtr domain goto done; rv = 0; + domain->id = -1; done: remoteDriverUnlock(priv); @@ -2735,6 +2736,8 @@ remoteDomainCreate (virDomainPtr domain) { int rv = -1; remote_domain_create_args args; + remote_domain_lookup_by_uuid_args args2; + remote_domain_lookup_by_uuid_ret ret2; struct private_data *priv = domain->conn->privateData; remoteDriverLock(priv); @@ -2746,6 +2749,16 @@ remoteDomainCreate (virDomainPtr domain) (xdrproc_t) xdr_void, (char *) NULL) == -1) goto done; + 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); + rv = 0; done: -- |: 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 :|
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 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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 :|
participants (2)
-
Daniel P. Berrange -
Daniel Veillard