[libvirt] [PATCH 08/40] Simplify the Xen count/list domains driver methods
Jim Fehlig
jfehlig at suse.com
Mon May 6 16:40:16 UTC 2013
Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
>
> The XenStore driver is mandatory, so it can be used unconditonally
> for the xenUnifiedConnectListDomains & xenUnifiedConnectNumOfDomains
> drivers. Delete the unused XenD and Hypervisor driver code for
> listing / counting domains
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
> src/xen/xen_driver.c | 46 +--------------------
> src/xen/xen_hypervisor.c | 101 -----------------------------------------------
> src/xen/xen_hypervisor.h | 4 --
> src/xen/xend_internal.c | 81 -------------------------------------
> src/xen/xend_internal.h | 2 -
> src/xen/xs_internal.c | 8 ----
> 6 files changed, 2 insertions(+), 240 deletions(-)
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index b6cf6ca..25fb7bb 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -583,55 +583,13 @@ xenUnifiedConnectGetCapabilities(virConnectPtr conn)
> static int
> xenUnifiedConnectListDomains(virConnectPtr conn, int *ids, int maxids)
> {
> - xenUnifiedPrivatePtr priv = conn->privateData;
> - int 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;
> - }
> -
> - return -1;
> + return xenStoreListDomains(conn, ids, maxids);
>
Probably not likely, but what if xenStoreListDomains() failed, e.g.
xshandle somehow became stale? The existing code would try the other
drivers if xenstore one failed.
Regards,
Jim
> }
>
> static int
> xenUnifiedConnectNumOfDomains(virConnectPtr conn)
> {
> - xenUnifiedPrivatePtr priv = conn->privateData;
> - int 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;
> - }
> -
> - return -1;
> + return xenStoreNumOfDomains(conn);
> }
>
> static virDomainPtr
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 012cb0e..2068a8a 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -2729,107 +2729,6 @@ xenHypervisorGetCapabilities(virConnectPtr conn)
> }
>
>
> -/**
> - * xenHypervisorNumOfDomains:
> - * @conn: pointer to the connection block
> - *
> - * Provides the number of active domains.
> - *
> - * Returns the number of domain found or -1 in case of error
> - */
> -int
> -xenHypervisorNumOfDomains(virConnectPtr conn)
> -{
> - xen_getdomaininfolist dominfos;
> - int ret, nbids;
> - static int last_maxids = 2;
> - int maxids = last_maxids;
> - xenUnifiedPrivatePtr priv = conn->privateData;
> -
> - retry:
> - if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
> - virReportOOMError();
> - return -1;
> - }
> -
> - XEN_GETDOMAININFOLIST_CLEAR(dominfos, maxids);
> -
> - ret = virXen_getdomaininfolist(priv->handle, 0, maxids, &dominfos);
> -
> - XEN_GETDOMAININFOLIST_FREE(dominfos);
> -
> - if (ret < 0)
> - return -1;
> -
> - nbids = ret;
> - /* Can't possibly have more than 65,000 concurrent guests
> - * so limit how many times we try, to avoid increasing
> - * without bound & thus allocating all of system memory !
> - * XXX I'll regret this comment in a few years time ;-)
> - */
> - if (nbids == maxids) {
> - if (maxids < 65000) {
> - last_maxids *= 2;
> - maxids *= 2;
> - goto retry;
> - }
> - nbids = -1;
> - }
> - if ((nbids < 0) || (nbids > maxids))
> - return -1;
> - return nbids;
> -}
> -
> -/**
> - * xenHypervisorListDomains:
> - * @conn: pointer to the connection block
> - * @ids: array to collect the list of IDs of active domains
> - * @maxids: size of @ids
> - *
> - * Collect the list of active domains, and store their ID in @maxids
> - *
> - * Returns the number of domain found or -1 in case of error
> - */
> -int
> -xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids)
> -{
> - xen_getdomaininfolist dominfos;
> - int ret, nbids, i;
> - xenUnifiedPrivatePtr priv = conn->privateData;
> -
> - if (maxids == 0)
> - return 0;
> -
> - if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
> - virReportOOMError();
> - return -1;
> - }
> -
> - XEN_GETDOMAININFOLIST_CLEAR(dominfos, maxids);
> - memset(ids, 0, maxids * sizeof(int));
> -
> - ret = virXen_getdomaininfolist(priv->handle, 0, maxids, &dominfos);
> -
> - if (ret < 0) {
> - XEN_GETDOMAININFOLIST_FREE(dominfos);
> - return -1;
> - }
> -
> - nbids = ret;
> - if ((nbids < 0) || (nbids > maxids)) {
> - XEN_GETDOMAININFOLIST_FREE(dominfos);
> - return -1;
> - }
> -
> - for (i = 0;i < nbids;i++) {
> - ids[i] = XEN_GETDOMAININFOLIST_DOMAIN(dominfos, i);
> - }
> -
> - XEN_GETDOMAININFOLIST_FREE(dominfos);
> - return nbids;
> -}
> -
> -
> char *
> xenHypervisorDomainGetOSType(virDomainPtr dom)
> {
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index 86dca88..949311d 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -70,10 +70,6 @@ char *
> unsigned long
> xenHypervisorGetDomMaxMemory (virConnectPtr conn,
> int id);
> -int xenHypervisorNumOfDomains (virConnectPtr conn);
> -int xenHypervisorListDomains (virConnectPtr conn,
> - int *ids,
> - int maxids);
> int xenHypervisorGetMaxVcpus (virConnectPtr conn,
> const char *type);
> int xenHypervisorDestroyDomain (virDomainPtr domain)
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index eb11408..952eb3f 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -1863,87 +1863,6 @@ xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps)
>
>
> /**
> - * xenDaemonListDomains:
> - * @conn: pointer to the hypervisor connection
> - * @ids: array to collect the list of IDs of active domains
> - * @maxids: size of @ids
> - *
> - * Collect the list of active domains, and store their ID in @maxids
> - * TODO: this is quite expensive at the moment since there isn't one
> - * xend RPC providing both name and id for all domains.
> - *
> - * Returns the number of domain found or -1 in case of error
> - */
> -int
> -xenDaemonListDomains(virConnectPtr conn, int *ids, int maxids)
> -{
> - struct sexpr *root = NULL;
> - int ret = -1;
> - struct sexpr *_for_i, *node;
> - long id;
> -
> - if (maxids == 0)
> - return 0;
> -
> - root = sexpr_get(conn, "/xend/domain");
> - if (root == NULL)
> - goto error;
> -
> - ret = 0;
> -
> - /* coverity[copy_paste_error] */
> - for (_for_i = root, node = root->u.s.car; _for_i->kind == SEXPR_CONS;
> - _for_i = _for_i->u.s.cdr, node = _for_i->u.s.car) {
> - if (node->kind != SEXPR_VALUE)
> - continue;
> - id = xenDaemonDomainLookupByName_ids(conn, node->u.value, NULL);
> - if (id >= 0)
> - ids[ret++] = (int) id;
> - if (ret >= maxids)
> - break;
> - }
> -
> -error:
> - sexpr_free(root);
> - return ret;
> -}
> -
> -/**
> - * xenDaemonNumOfDomains:
> - * @conn: pointer to the hypervisor connection
> - *
> - * Provides the number of active domains.
> - *
> - * Returns the number of domain found or -1 in case of error
> - */
> -int
> -xenDaemonNumOfDomains(virConnectPtr conn)
> -{
> - struct sexpr *root = NULL;
> - int ret = -1;
> - struct sexpr *_for_i, *node;
> -
> - root = sexpr_get(conn, "/xend/domain");
> - if (root == NULL)
> - goto error;
> -
> - ret = 0;
> -
> - /* coverity[copy_paste_error] */
> - for (_for_i = root, node = root->u.s.car; _for_i->kind == SEXPR_CONS;
> - _for_i = _for_i->u.s.cdr, node = _for_i->u.s.car) {
> - if (node->kind != SEXPR_VALUE)
> - continue;
> - ret++;
> - }
> -
> -error:
> - sexpr_free(root);
> - return ret;
> -}
> -
> -
> -/**
> * xenDaemonLookupByID:
> * @conn: pointer to the hypervisor connection
> * @id: the domain ID number
> diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
> index 41d8341..f6760a2 100644
> --- a/src/xen/xend_internal.h
> +++ b/src/xen/xend_internal.h
> @@ -161,7 +161,5 @@ 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);
> -int xenDaemonNumOfDomains(virConnectPtr conn);
>
> #endif /* __XEND_INTERNAL_H_ */
> diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
> index eecdcae..dbb4ae4 100644
> --- a/src/xen/xs_internal.c
> +++ b/src/xen/xs_internal.c
> @@ -496,11 +496,6 @@ xenStoreNumOfDomains(virConnectPtr conn)
> long id;
> xenUnifiedPrivatePtr priv = conn->privateData;
>
> - if (priv->xshandle == NULL) {
> - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> - return -1;
> - }
> -
> idlist = xs_directory(priv->xshandle, 0, "/local/domain", &num);
> if (idlist) {
> for (i = 0; i < num; i++) {
> @@ -542,9 +537,6 @@ xenStoreDoListDomains(virConnectPtr conn,
> int ret = -1;
> long id;
>
> - if (priv->xshandle == NULL)
> - goto out;
> -
> idlist = xs_directory(priv->xshandle, 0, "/local/domain", &num);
> if (idlist == NULL)
> goto out;
>
More information about the libvir-list
mailing list