On Mon, May 06, 2013 at 02:32:13PM -0600, Jim Fehlig wrote:
Jim Fehlig wrote:
Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
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@redhat.com> --- 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?
The issue is that trying multiple drivers doesn't actually solve any inconsistencies - the first driver to succeed wins, even if it has stale data. You are right though that we did have some issues in the past - this is why with certain methods we twiddle it so that it would try XenStore before trying XenD, since XenStore was faster & more accurate. This cleanup as maintained this prioritization, so I don't think we're going to regress in this respect. 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 :|