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@xxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxx>
> >> ---
> >> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list