Re: [libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Unconditionally invoke the xenHypervisorLookupDomainByID, xenHypervisorLookupDomainByUUID or xenDaemonLookupByName for looking up domains. Fallback to xenXMDomainLookupByUUID and xenXMDomainLookupByName for legacy XenD without inactive domain support

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/xen/xen_driver.c | 99 +++++++++++-------------------------------------- src/xen/xend_internal.c | 89 -------------------------------------------- src/xen/xend_internal.h | 14 ------- src/xen/xs_internal.c | 62 ------------------------------- src/xen/xs_internal.h | 2 - 5 files changed, 22 insertions(+), 244 deletions(-)

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 82058b7..080045c 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -601,38 +601,17 @@ xenUnifiedDomainCreateXML(virConnectPtr conn, return xenDaemonCreateXML(conn, xmlDesc); } -/* Assumption made in underlying drivers: - * If the domain is "not found" and there is no other error, then - * the Lookup* functions return a NULL but do not set virterror. - */ static virDomainPtr xenUnifiedDomainLookupByID(virConnectPtr conn, int id) { - xenUnifiedPrivatePtr priv = conn->privateData; - virDomainPtr ret; + virDomainPtr ret = NULL; - /* Reset any connection-level errors in virterror first, in case - * there is one hanging around from a previous call. - */ - virConnResetLastError(conn); + ret = xenHypervisorLookupDomainByID(conn, id); - /* Try hypervisor/xenstore combo. */ - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { - ret = xenHypervisorLookupDomainByID(conn, id); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } - - /* Try xend. */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - ret = xenDaemonLookupByID(conn, id); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); - /* Not found. */ - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); - return NULL; + return ret; } static virDomainPtr @@ -642,35 +621,20 @@ xenUnifiedDomainLookupByUUID(virConnectPtr conn, xenUnifiedPrivatePtr priv = conn->privateData; virDomainPtr ret; - /* Reset any connection-level errors in virterror first, in case - * there is one hanging around from a previous call. - */ - virConnResetLastError(conn); - - /* Try hypervisor/xenstore combo. */ - if (priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]) { - ret = xenHypervisorLookupDomainByUUID(conn, uuid); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } - - /* Try xend. */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - ret = xenDaemonLookupByUUID(conn, uuid); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } + ret = xenHypervisorLookupDomainByUUID(conn, uuid); /* Try XM for inactive domains. */ - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { - ret = xenXMDomainLookupByUUID(conn, uuid); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; + if (!ret) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) + ret = xenXMDomainLookupByUUID(conn, uuid); + else + return xenDaemonLookupByUUID(conn, uuid); } - /* Not found. */ - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); - return NULL; + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; } static virDomainPtr @@ -680,35 +644,16 @@ xenUnifiedDomainLookupByName(virConnectPtr conn, xenUnifiedPrivatePtr priv = conn->privateData; virDomainPtr ret; - /* Reset any connection-level errors in virterror first, in case - * there is one hanging around from a previous call. - */ - virConnResetLastError(conn); - - /* Try xend. */ - if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) { - ret = xenDaemonLookupByName(conn, name); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } - - /* Try xenstore for inactive domains. */ - if (priv->opened[XEN_UNIFIED_XS_OFFSET]) { - ret = xenStoreLookupByName(conn, name); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } + ret = xenDaemonLookupByName(conn, name); /* Try XM for inactive domains. */ - if (priv->opened[XEN_UNIFIED_XM_OFFSET]) { + if (priv->xendConfigVersion <= XEND_CONFIG_VERSION_3_0_3) ret = xenXMDomainLookupByName(conn, name); - if (ret || conn->err.code != VIR_ERR_OK) - return ret; - } - /* Not found. */ - virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); - return NULL; + if (!ret && virGetLastError() == NULL) + virReportError(VIR_ERR_NO_DOMAIN, __FUNCTION__); + + return ret; } @@ -719,7 +664,7 @@ xenUnifiedDomainIsActive(virDomainPtr dom) int ret = -1; /* ID field in dom may be outdated, so re-lookup */ - currdom = xenUnifiedDomainLookupByUUID(dom->conn, dom->uuid); + currdom = xenHypervisorLookupDomainByUUID(dom->conn, dom->uuid); if (currdom) { ret = currdom->id == -1 ? 0 : 1; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 2e6a47e..4ad30fa 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -855,63 +855,6 @@ xenDaemonDomainLookupByName_ids(virConnectPtr xend, } -/** - * xenDaemonDomainLookupByID: - * @xend: A xend instance - * @id: The id of the domain - * @name: return value for the name if not NULL - * @uuid: return value for the UUID if not NULL - * - * This method looks up the name of a domain based on its id - * - * Returns the 0 on success; -1 (with errno) on error - */ -int -xenDaemonDomainLookupByID(virConnectPtr xend, - int id, - char **domname, - unsigned char *uuid) -{ - const char *name = NULL; - struct sexpr *root; - - memset(uuid, 0, VIR_UUID_BUFLEN); - - root = sexpr_get(xend, "/xend/domain/%d?detail=1", id); - if (root == NULL) - goto error; - - name = sexpr_node(root, "domain/name"); - if (name == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("domain information incomplete, missing name")); - goto error; - } - if (domname) { - *domname = strdup(name); - if (*domname == NULL) { - virReportOOMError(); - goto error; - } - } - - if (sexpr_uuid(uuid, root, "domain/uuid") < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("domain information incomplete, missing uuid")); - goto error; - } - - sexpr_free(root); - return 0; - -error: - sexpr_free(root); - if (domname) - VIR_FREE(*domname); - return -1; -} - - static int xend_detect_config_version(virConnectPtr conn) { @@ -1863,38 +1806,6 @@ xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps) /** - * xenDaemonLookupByID: - * @conn: pointer to the hypervisor connection - * @id: the domain ID number - * - * Try to find a domain based on the hypervisor ID number - * - * Returns a new domain object or NULL in case of failure - */ -virDomainPtr -xenDaemonLookupByID(virConnectPtr conn, int id) -{ - char *name = NULL; - unsigned char uuid[VIR_UUID_BUFLEN]; - virDomainPtr ret; - - if (xenDaemonDomainLookupByID(conn, id, &name, uuid) < 0) { - goto error; - } - - ret = virGetDomain(conn, name, uuid); - if (ret == NULL) goto error; - - ret->id = id; - VIR_FREE(name); - return ret; - - error: - VIR_FREE(name); - return NULL; -} - -/** * xenDaemonDomainSetVcpusFlags: * @domain: pointer to domain object * @nvcpus: the new number of virtual CPUs for this domain diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h index 5f82f04..e8713a7 100644 --- a/src/xen/xend_internal.h +++ b/src/xen/xend_internal.h @@ -69,19 +69,6 @@ int xenDaemonDomainLookupByName_ids(virConnectPtr xend, const char *name, unsigned char *uuid); -/** - * \brief Lookup the name of a domain - * \param xend A xend instance - * \param id The id of the domain - * \param name pointer to store a copy of the name - * \param uuid pointer to store a copy of the uuid - * - * This method looks up the name/uuid of a domain - */ -int xenDaemonDomainLookupByID(virConnectPtr xend, - int id, - char **name, unsigned char *uuid); - virDomainDefPtr xenDaemonDomainFetch(virConnectPtr xend, @@ -153,7 +140,6 @@ extern struct xenUnifiedDriver xenDaemonDriver; int xenDaemonInit (void); virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc); -virDomainPtr xenDaemonLookupByID(virConnectPtr conn, int id); virDomainPtr xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virDomainPtr xenDaemonLookupByName(virConnectPtr conn, const char *domname); int xenDaemonDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cookielen, const char *uri_in, char **uri_out, unsigned long flags, const char *dname, unsigned long resource); diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index dbb4ae4..7926535 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -580,68 +580,6 @@ xenStoreListDomains(virConnectPtr conn, int *ids, int maxids) return ret; } -/** - * xenStoreLookupByName: - * @conn: A xend instance - * @name: The name of the domain - * - * Try to lookup a domain on the Xen Store based on its name. - * - * Returns a new domain object or NULL in case of failure - */ -virDomainPtr -xenStoreLookupByName(virConnectPtr conn, const char *name) -{ - virDomainPtr ret = NULL; - unsigned int num, i, len; - long id = -1; - char **idlist = NULL, *endptr; - char prop[200], *tmp; - int found = 0; - struct xend_domain *xenddomain = NULL; - xenUnifiedPrivatePtr priv = conn->privateData; - - if (priv->xshandle == NULL) - return NULL; - - idlist = xs_directory(priv->xshandle, 0, "/local/domain", &num); - if (idlist == NULL) - goto done; - - for (i = 0; i < num; i++) { - id = strtol(idlist[i], &endptr, 10); - if ((endptr == idlist[i]) || (*endptr != 0)) { - goto done; - } -#if 0 - if (virConnectCheckStoreID(conn, (int) id) < 0) - continue; -#endif - snprintf(prop, 199, "/local/domain/%s/name", idlist[i]); - prop[199] = 0; - tmp = xs_read(priv->xshandle, 0, prop, &len); - if (tmp != NULL) { - found = STREQ(name, tmp); - VIR_FREE(tmp); - if (found) - break; - } - } - if (!found) - goto done; - - ret = virGetDomain(conn, name, NULL); - if (ret == NULL) - goto done; - - ret->id = id; - -done: - VIR_FREE(xenddomain); - VIR_FREE(idlist); - - return ret; -} /** * xenStoreDomainShutdown: diff --git a/src/xen/xs_internal.h b/src/xen/xs_internal.h index 29f0165..fc7798d 100644 --- a/src/xen/xs_internal.h +++ b/src/xen/xs_internal.h @@ -43,8 +43,6 @@ int xenStoreNumOfDomains (virConnectPtr conn); int xenStoreListDomains (virConnectPtr conn, int *ids, int maxids); -virDomainPtr xenStoreLookupByName(virConnectPtr conn, - const char *name); unsigned long xenStoreGetMaxMemory (virDomainPtr domain); int xenStoreDomainSetMemory (virDomainPtr domain, unsigned long memory);