[libvirt] [PATCH 10/40] Simplify the Xen domain lookup driver methods
Jim Fehlig
jfehlig at suse.com
Tue May 7 03:40:43 UTC 2013
Jim Fehlig wrote:
> Daniel P. Berrange wrote:
>
>> From: "Daniel P. Berrange" <berrange at redhat.com>
>>
>> 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 at redhat.com>
>> ---
>> 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.
>
Apparently "some" time was not enough time. With this patch, I noticed
'virsh undefine dom' failing because the tri-state virDomainIsActive()
is returning -1.
>> 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);
>>
xenUnifiedDomainLookupByUUID will try the xend or xm drivers if the
hypervisor driver fails. Invoking the hypervisor driver only will result
in a NULL 'currdom' for inactive domains, causing this function (and
hence virDomainIsActive) to return -1, which in turn causes cmdUndefine
in tools/virsh-domain.c to bail with an error.
Regards,
Jim
>> + 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
>
More information about the libvir-list
mailing list