[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 08/40] Simplify the Xen count/list domains driver methods



Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange 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 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;
>   


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]