Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Unconditionally invoke the xenHypervisorLookupDomainByID,
> xenHypervisorLookupDomainByUUID or xenDaemonLookupByName
> for looking up domains. Fallback to xenXMDomainLookupByUUID
> and xenXMDomainLookupByName for legacy XenD without inactive
> domain support
>
Do you think there are any Xen installations running such an old xend
toolstack, and if so wanting to use e.g. libvirt 1.0.6? Seems all of the
XEND_CONFIG_VERSION_3_0_3 logic could be removed from the code.
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> 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(-)
>
I spent some time testing this one and didn't notice any problems.
ACK.
Regards,
Jim
> 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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list