Re: [libvirt] [PATCH 08/40] Simplify the Xen count/list domains driver methods

Jim Fehlig wrote: > Daniel P. Berrange wrote: > >> From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; >> >> The XenStore driver is mandatory, so it can be used unconditonally >> for the xenUnifiedConnectListDomains & xenUnifiedConnectNumOfDomains >> drivers. Delete the unused XenD and Hypervisor driver code for >> listing / counting domains >> >> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; >> --- >> src/xen/xen_driver.c | 46 +-------------------- >> src/xen/xen_hypervisor.c | 101 ----------------------------------------------- >> src/xen/xen_hypervisor.h | 4 -- >> src/xen/xend_internal.c | 81 ------------------------------------- >> src/xen/xend_internal.h | 2 - >> src/xen/xs_internal.c | 8 ---- >> 6 files changed, 2 insertions(+), 240 deletions(-) >> >> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c >> index b6cf6ca..25fb7bb 100644 >> --- a/src/xen/xen_driver.c >> +++ b/src/xen/xen_driver.c >> @@ -583,55 +583,13 @@ xenUnifiedConnectGetCapabilities(virConnectPtr conn) >> static int >> xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids) >> { >> - xenUnifiedPrivatePtr priv = conn->privateData; >> - int ret; >> - >> - /* Try xenstore. */ >> - if (priv->opened[XEN_UNIFIED_XS_OFFSET]) { >> - ret = xenStoreListDomains(conn, ids, maxids); >> - if (ret >= 0) return ret; >> - } >> - >> - /* Try HV. */ >> - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { >> - ret = xenHypervisorListDomains(conn, ids, maxids); >> - if (ret >= 0) return ret; >> - } >> - >> - /* Try xend. */ >> - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { >> - ret = xenDaemonListDomains(conn, ids, maxids); >> - if (ret >= 0) return ret; >> - } >> - >> - return -1; >> + return xenStoreListDomains(conn, ids, maxids); >> >> > > Probably not likely, but what if xenStoreListDomains() failed, e.g. > xshandle somehow became stale? The existing code would try the other > drivers if xenstore one failed. > I started to make a similar comment for patch 10, but then re-read your cover letter and realize this is intentional. While I agree with the direct invocation in 10, and probably others I've yet to review, this may be a case where we actually want to try something other than xenstore. I seem to recall an issue long ago where trying multiple drivers helped when there were state inconsistencies in the xen tools. But then again, maybe it is best to simply fail instead of propagating those inconsistencies?