Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods

On Wed, May 08, 2013 at 11:37:44AM +0100, Daniel P. Berrange wrote: > On Mon, May 06, 2013 at 09:40:43PM -0600, Jim Fehlig wrote: > >> Jim Fehlig wrote: >> >>> Daniel P. Berrange wrote: >>> >>> >>>> From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; >>>> >>>> Unconditionally invoke the xenHypervisorLookupDomainByID, >>>> xenHypervisorLookupDomainByUUID or xenDaemonLookupByName >>>> for looking up domains. Fallback to xenXMDomainLookupByUUID >>>> and xenXMDomainLookupByName for legacy XenD without inactive >>>> domain support >>>> >>>> >>>> >>> Do you think there are any Xen installations running such an old xend >>> toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the >>> XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code. >>> >>> >>> >>> >>>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; >>>> --- >>>> src/xen/xen_driver.c | 99 +++++++++++-------------------------------------- >>>> src/xen/xend_internal.c | 89 -------------------------------------------- >>>> src/xen/xend_internal.h | 14 ------- >>>> src/xen/xs_internal.c | 62 ------------------------------- >>>> src/xen/xs_internal.h | 2 - >>>> 5 files changed, 22 insertions(+), 244 deletions(-) >>>> >>>> >>>> >>> I spent some time testing this one and didn't notice any problems. >>> >>> >> Apparently "some" time was not enough time. With this patch, I noticed >> 'virsh undefine dom' failing because the tri-state virDomainIsActive() >> is returning -1. >> > Opps, I made a mistake only checking the hypervisor, which of course > will not know about the domain if it is shutoff :-) Adding the following > extra hunk fixes it > Sigh, helps if i paste the right patch @@ -660,15 +660,26 @@ xenUnifiedDomainLookupByName(virConnectPtr conn, static int xenUnifiedDomainIsActive(virDomainPtr dom) { + xenUnifiedPrivatePtr priv = dom->conn->privateData; virDomainPtr currdom; int ret = -1; /* ID field in dom may be outdated, so re-lookup */ currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); + /* Try XM for inactive domains. */ + if (!currdom) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + currdom = xenXMDomainLookupByUUID(dom->conn, dom->uuid); + else + currdom = xenDaemonLookupByUUID(dom->conn, dom->uuid); + } + if (currdom) { ret = currdom->id == -1 ? 0 : 1; virDomainFree(currdom); + } else if (virGetLastError() == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); } return ret;