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@xxxxxxxxxx> > >> > >> 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@xxxxxxxxxx> > >> --- > >> 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 @@ -664,11 +664,21 @@ xenUnifiedDomainIsActive(virDomainPtr dom) int ret = -1; /* ID field in dom may be outdated, so re-lookup */ - currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); + currdom = xenHypervisorLookupDomainByUUID(conn, uuid); + + /* Try XM for inactive domains. */ + if (!currdom) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + currdom = xenXMDomainLookupByUUID(conn, uuid); + else + currdom = xenDaemonLookupByUUID(conn, uuid); + } if (currdom) { ret = currdom->id == -1 ? 0 : 1; virDomainFree(currdom); + } else if (virGetLastError() == NULL) { + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); } return ret; Regards, 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list