From: Jonas Eriksson <jonas.j.eriksson(a)ericsson.com&gt; The xenUnifiedNumOfDomains and xenUnifiedListDomains methods work together as a pair, so it is critical they both apply the same logic. With the current mis-matched logic it is possible to sometimes get into a state when you miss certain active guests. * src/xen/xen_driver.c: Change xenUnifiedNumOfDomains ordering to match xenUnifiedListDomains. --- src/xen/proxy_internal.c | 3 +-- src/xen/proxy_internal.h | 1 + src/xen/xen_driver.c | 30 ++++++++++++++++++++++++------ src/xen/xend_internal.c | 2 +- src/xen/xend_internal.h | 1 + 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c index f9d2fad..22ff172 100644 --- a/src/xen/proxy_internal.c +++ b/src/xen/proxy_internal.c @@ -40,7 +40,6 @@ static virDrvOpenStatus xenProxyOpen(virConnectPtr conn, virConnectAuthPtr auth, static int xenProxyGetVersion(virConnectPtr conn, unsigned long *hvVer); static int xenProxyNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); static char *xenProxyGetCapabilities(virConnectPtr conn); -static int xenProxyNumOfDomains(virConnectPtr conn); static unsigned long xenProxyDomainGetMaxMemory(virDomainPtr domain); static int xenProxyDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info); static char *xenProxyDomainGetOSType(virDomainPtr domain); @@ -607,7 +606,7 @@ xenProxyListDomains(virConnectPtr conn, int *ids, int maxids) * * Returns the number of domain found or -1 in case of error */ -static int +int xenProxyNumOfDomains(virConnectPtr conn) { virProxyPacket req; diff --git a/src/xen/proxy_internal.h b/src/xen/proxy_internal.h index 185fa67..19df751 100644 --- a/src/xen/proxy_internal.h +++ b/src/xen/proxy_internal.h @@ -96,4 +96,5 @@ extern char * xenProxyDomainDumpXML(virDomainPtr domain, int flags); extern int xenProxyListDomains(virConnectPtr conn, int *ids, int maxids); +extern int xenProxyNumOfDomains(virConnectPtr conn); #endif /* __LIBVIR_PROXY_H__ */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 76b896a..fc8bbe6 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -576,13 +576,31 @@ static int xenUnifiedNumOfDomains (virConnectPtr conn) { GET_PRIVATE(conn); - int i, ret; + int ret; - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) - if (priv->opened[i] && drivers[i]->numOfDomains) { - ret = drivers[i]->numOfDomains (conn); - if (ret >= 0) return ret; - } + /* Try xenstore. */ + if (priv->opened[XEN_UNIFIED_XS_OFFSET]) { + ret = xenStoreNumOfDomains (conn); + if (ret >= 0) return ret; + } + + /* Try HV. */ + if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { + ret = xenHypervisorNumOfDomains (conn); + if (ret >= 0) return ret; + } + + /* Try xend. */ + if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { + ret = xenDaemonNumOfDomains (conn); + if (ret >= 0) return ret; + } + + /* Try proxy. */ + if (priv->opened[XEN_UNIFIED_PROXY_OFFSET]) { + ret = xenProxyNumOfDomains (conn); + if (ret >= 0) return ret; + } return -1; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 49bdba9..f362ae6 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3685,7 +3685,7 @@ error: * * Returns the number of domain found or -1 in case of error */ -static int +int xenDaemonNumOfDomains(virConnectPtr conn) { struct sexpr *root = NULL; diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 9d2571b..8b00737 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -187,5 +187,6 @@ int xenDaemonDomainMigratePerform (virDomainPtr domain, const char *cookie, int int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer); int xenDaemonListDomains(virConnectPtr conn, int *ids, int maxids); +int xenDaemonNumOfDomains(virConnectPtr conn); #endif /* __XEND_INTERNAL_H_ */

The xenstore database sometimes has stale domain IDs which are not present in the hypervisor anymore. Filter these out to avoid causing confusion * src/xen/xs_internal.c: Filter domain IDs against HV's list * src/xen/xen_hypervisor.h, src/xen/xen_hypervisor.c: Add new xenHypervisorHasDomain() method for checking ID validity --- src/xen/xen_hypervisor.c | 22 ++++++++++++++++++++++ src/xen/xen_hypervisor.h | 4 +++- src/xen/xs_internal.c | 35 ++++++++++++++++++++++++++--------- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 3aa3c30..6ab2431 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2780,6 +2780,28 @@ xenHypervisorDomainGetOSType (virDomainPtr dom) return strdup("linux"); } +int +xenHypervisorHasDomain(virConnectPtr conn, + int id) +{ + xenUnifiedPrivatePtr priv; + xen_getdomaininfo dominfo; + + priv = (xenUnifiedPrivatePtr) conn->privateData; + if (priv->handle < 0) + return 0; + + XEN_GETDOMAININFO_CLEAR(dominfo); + + if (virXen_getdomaininfo(priv->handle, id, &dominfo) < 0) + return 0; + + if (XEN_GETDOMAININFO_DOMAIN(dominfo) != id) + return 0; + + return 1; +} + virDomainPtr xenHypervisorLookupDomainByID(virConnectPtr conn, int id) diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h index 766f676..5971a90 100644 --- a/src/xen/xen_hypervisor.h +++ b/src/xen/xen_hypervisor.h @@ -23,7 +23,9 @@ int xenHypervisorInit (void); virCapsPtr xenHypervisorMakeCapabilities (virConnectPtr conn); /* The following calls are made directly by the Xen proxy: */ - +int + xenHypervisorHasDomain(virConnectPtr conn, + int id); virDomainPtr xenHypervisorLookupDomainByID (virConnectPtr conn, int id); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 0fabcf8..c83cfda 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -545,8 +545,9 @@ int xenStoreNumOfDomains(virConnectPtr conn) { unsigned int num; - char **idlist; - int ret = -1; + char **idlist = NULL, *endptr; + int i, ret = -1, realnum = 0; + long id; xenUnifiedPrivatePtr priv; if (conn == NULL) { @@ -559,10 +560,22 @@ xenStoreNumOfDomains(virConnectPtr conn) virXenStoreError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); return(-1); } + idlist = xs_directory(priv->xshandle, 0, "/local/domain", &num); if (idlist) { - free(idlist); - ret = num; + for (i = 0; i < num; i++) { + id = strtol(idlist[i], &endptr, 10); + if ((endptr == idlist[i]) || (*endptr != 0)) + goto out; + + /* Sometimes xenstore has stale domain IDs, so filter + against the hypervisor's info */ + if (xenHypervisorHasDomain(conn, (int)id)) + realnum++; + } +out: + VIR_FREE (idlist); + ret = realnum; } return(ret); } @@ -579,7 +592,7 @@ xenStoreNumOfDomains(virConnectPtr conn) * Returns the number of domain found or -1 in case of error */ static int -xenStoreDoListDomains(xenUnifiedPrivatePtr priv, int *ids, int maxids) +xenStoreDoListDomains(virConnectPtr conn, xenUnifiedPrivatePtr priv, int *ids, int maxids) { char **idlist = NULL, *endptr; unsigned int num, i; @@ -597,7 +610,11 @@ xenStoreDoListDomains(xenUnifiedPrivatePtr priv, int *ids, int maxids) id = strtol(idlist[i], &endptr, 10); if ((endptr == idlist[i]) || (*endptr != 0)) goto out; - ids[ret++] = (int) id; + + /* Sometimes xenstore has stale domain IDs, so filter + against the hypervisor's info */ + if (xenHypervisorHasDomain(conn, (int)id)) + ids[ret++] = (int) id; } out: @@ -629,7 +646,7 @@ xenStoreListDomains(virConnectPtr conn, int *ids, int maxids) priv = (xenUnifiedPrivatePtr) conn->privateData; xenUnifiedLock(priv); - ret = xenStoreDoListDomains(priv, ids, maxids); + ret = xenStoreDoListDomains(conn, priv, ids, maxids); xenUnifiedUnlock(priv); return(ret); @@ -1275,7 +1292,7 @@ retry: virReportOOMError(NULL); return -1; } - nread = xenStoreDoListDomains(priv, new_domids, new_domain_cnt); + nread = xenStoreDoListDomains(conn, priv, new_domids, new_domain_cnt); if (nread != new_domain_cnt) { // mismatch. retry this read VIR_FREE(new_domids); @@ -1356,7 +1373,7 @@ retry: virReportOOMError(NULL); return -1; } - nread = xenStoreDoListDomains(priv, new_domids, new_domain_cnt); + nread = xenStoreDoListDomains(conn, priv, new_domids, new_domain_cnt); if (nread != new_domain_cnt) { // mismatch. retry this read VIR_FREE(new_domids);