[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[PATCH] xen: prefer XS for list Domains (was Re: [libvirt] libvirt + xen 3.2.1 oddities)



On Tue, Nov 25, 2008 at 11:39:57AM +0000, Daniel P. Berrange wrote:
> On Fri, Nov 21, 2008 at 11:13:04PM +0100, Guido G?nther wrote:
> > Hi,
> > I just ran across these oddities when using a bit more libvirt+xen:
> > 
> > 1.) virsh setmaxmem:
> > 
> > On a running domain:
> >         # virsh setmaxmem domain 256000
> > completes but virsh dumpxml as well as the config.sxp still shows the
> > old amount of memory. Looks as the set_maxmem hypercall simply gets
> > ignored. xm mem-max works as expected. Smells like a bug in the ioctl?
> 
> The setmaxmem API is not performance critical, so it sounds like we
> should first try setting it via XenD, and use Hypervisor as the
> fallback instead.
I tried that and it worked as you suggested. However, checking the "old"
method of using HV out of a sudden works too now, no idea why that
reliably failed the last time and doesn't do so now (the machine has
been rebooted in the meantime though). I keep the patched package around
in case this pops up again.

> 
> > 2.) virsh list:
> > 
> > Sometimes (didn't find a pattern yet) when shutting down a running
> > domains and restarting it I'm seeing:
> > 
> >  Id Name                 State
> > ----------------------------------
> >   0 Domain-0             running
> >   2 foo                 idle
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> >  7 bar                  idle
> > 
> > Note that the number of errors the corresponds to the number of
> > shutdowns. VirXen_getdomaininfolist returns 7 in the above case.
> > virDomainLookupByID later on fails for these "additional" domains.
> 
> This is basically a XenD bug. What's happening is that the domain
> has been shutdown, and got most of the way through cleanup, as far
> as the hypervisor is concerned. But something is still hanging around
> keeping the domain from being completely terminated. In this case
> XenD takes the dubious approach of just pretending the domain does
> not exist. So libvirt sees it exists in the hypervisor, but when
> asking XenD for more data, it gets that error. This really really
> sucks.
> 
> THere's not really much we can do about it when XenD is just plain
> lieing about what exists. We explicitly don't ask XenD for the list
> of domain IDs because it is incredibly slow, hence we use the HV.
> 
> The only idea I can think of is to ask XenStore for the list of
> domain IDs. This is still dramatically faster than asking XenD,
> but not quite as fast as the Hypervisor. 
> 
> > 
> > 3.) virsh list: Duplicate domains:
> > 
> >  Id Name                 State
> > ----------------------------------
> >   0 Domain-0             running
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> 2A> libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> >  14 bar              no state
> > libvir: Xen Daemon error : GET operation failed: xend_get: error from xen daemon:
> >  16 bar              idle
> > 
> > Domain 14 can't be shut down (xm list only lists domain 16).
> > 
> > Could be a similar problem as the above.
> 
> Yeha, this is almost certainly just another example of XenD not properly
> cleaning up / destroying domains. If you still have a machine which
> shows this behaviour, then I'd recommend trying this change to our Xen
> impl
> 
> In xen_unified.c, find the method xenUnifiedListDomains and make it first
> call xenStoreListDomains() and then fallback to trying HV & XenD drivers.
> If we're lucky this will help....
Yes this helps indeed, thanks a lot. Possible patch attached.
 -- Guido
>From 215f2c99da9b086d8d506f359a1e4cfcc64670b6 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx sigxcpu org>
Date: Wed, 26 Nov 2008 10:54:51 +0100
Subject: [PATCH] prefer xenstore driver for listDomains

at least Debian's xen 3.2.1 reports ghost ids of already shutdown domains when
using the HV driver.
---
 src/proxy_internal.c |    3 +--
 src/proxy_internal.h |    2 ++
 src/xen_unified.c    |   29 +++++++++++++++++++++++------
 src/xend_internal.c  |    2 +-
 src/xend_internal.h  |    1 +
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/proxy_internal.c b/src/proxy_internal.c
index 1378559..daf1193 100644
--- a/src/proxy_internal.c
+++ b/src/proxy_internal.c
@@ -37,7 +37,6 @@ static int xenProxyOpen(virConnectPtr conn, xmlURIPtr uri, virConnectAuthPtr aut
 static int xenProxyGetVersion(virConnectPtr conn, unsigned long *hvVer);
 static int xenProxyNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
 static char *xenProxyGetCapabilities(virConnectPtr conn);
-static int xenProxyListDomains(virConnectPtr conn, int *ids, int maxids);
 static int xenProxyNumOfDomains(virConnectPtr conn);
 static unsigned long xenProxyDomainGetMaxMemory(virDomainPtr domain);
 static int xenProxyDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info);
@@ -581,7 +580,7 @@ xenProxyGetVersion(virConnectPtr conn, unsigned long *hvVer)
  *
  * Returns the number of domain found or -1 in case of error
  */
-static int
+int
 xenProxyListDomains(virConnectPtr conn, int *ids, int maxids)
 {
     virProxyPacket req;
diff --git a/src/proxy_internal.h b/src/proxy_internal.h
index 0e66c1c..693b10b 100644
--- a/src/proxy_internal.h
+++ b/src/proxy_internal.h
@@ -94,4 +94,6 @@ extern virDomainPtr xenProxyLookupByName(virConnectPtr conn,
 
 extern char *       xenProxyDomainDumpXML(virDomainPtr domain,
                                           int flags);
+extern int xenProxyListDomains(virConnectPtr conn, int *ids,
+                               int maxids);
 #endif /* __LIBVIR_PROXY_H__ */
diff --git a/src/xen_unified.c b/src/xen_unified.c
index 5807391..0fb5d73 100644
--- a/src/xen_unified.c
+++ b/src/xen_unified.c
@@ -485,14 +485,31 @@ static int
 xenUnifiedListDomains (virConnectPtr conn, int *ids, int maxids)
 {
     GET_PRIVATE(conn);
-    int i, ret;
+    int ret;
 
-    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
-        if (priv->opened[i] && drivers[i]->listDomains) {
-            ret = drivers[i]->listDomains (conn, ids, maxids);
-            if (ret >= 0) return 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;
+    }
+
+    /* Try proxy. */
+    if (priv->opened[XEN_UNIFIED_PROXY_OFFSET]) {
+        ret = xenProxyListDomains (conn, ids, maxids);
+        if (ret >= 0) return ret;
+    }
     return -1;
 }
 
diff --git a/src/xend_internal.c b/src/xend_internal.c
index 2e1a8d1..9f1ad42 100644
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -3455,7 +3455,7 @@ xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer)
  *
  * Returns the number of domain found or -1 in case of error
  */
-static int
+int
 xenDaemonListDomains(virConnectPtr conn, int *ids, int maxids)
 {
     struct sexpr *root = NULL;
diff --git a/src/xend_internal.h b/src/xend_internal.h
index 12fa379..af90290 100644
--- a/src/xend_internal.h
+++ b/src/xend_internal.h
@@ -178,5 +178,6 @@ int xenDaemonDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cook
 int xenDaemonDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long resource);
 
 int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer);
+int xenDaemonListDomains(virConnectPtr conn, int *ids, int maxids);
 
 #endif /* __XEND_INTERNAL_H_ */
-- 
1.6.0.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]