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

Re: [libvirt] [PATCH 4/9] phyp: Make generic domain listing functions return -1 in case of error



On Fri, Nov 06, 2009 at 04:28:02AM +0100, Matthias Bolte wrote:
> phypNumDomainsGeneric() and phypListDomainsGeneric() return 0 in case
> of an error. This makes it impossible to distinguish between an actual
> error and no domains being defined on the hypervisor. It also turn the
> no domains situation into an error. Return -1 in case of an error to
> fix this problem.
> ---
>  src/phyp/phyp_driver.c |   27 ++++++++++++++++++++++-----
>  1 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index b94d0fa..8d54ae7 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
> @@ -980,7 +980,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
>    err:
>      VIR_FREE(cmd);
>      VIR_FREE(ret);
> -    return 0;
> +    return -1;
>  }
>  
>  static int
> @@ -1065,7 +1065,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
>    err:
>      VIR_FREE(cmd);
>      VIR_FREE(ret);
> -    return 0;
> +    return -1;
>  }
>  
>  static int
> @@ -1130,7 +1130,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames)
>          VIR_FREE(names[i]);
>      VIR_FREE(cmd);
>      VIR_FREE(ret);
> -    return 0;
> +    return -1;
>  }
>  
>  static virDomainPtr
> @@ -1843,17 +1843,34 @@ phypUUIDTable_Init(virConnectPtr conn)
>      int *ids = NULL;
>      unsigned int i = 0;
>  
> -    if ((nids = phypNumDomainsGeneric(conn, 2)) == 0)
> +    if ((nids = phypNumDomainsGeneric(conn, 2)) < 0)
>          goto err;
>  
> +    /* exit early if there are no domains */
> +    if (nids == 0)
> +        return 0;
> +
>      if (VIR_ALLOC_N(ids, nids) < 0) {
>          virReportOOMError(conn);
>          goto err;
>      }
>  
> -    if (phypListDomainsGeneric(conn, ids, nids, 1) == 0)
> +    if ((nids = phypListDomainsGeneric(conn, ids, nids, 1)) < 0)
>          goto err;
>  
> +    /* exit early if there are no domains */
> +    /* FIXME: phypNumDomainsGeneric() returned > 0 but phypListDomainsGeneric()
> +     *        returned 0. indicates this an error condition?
> +     *        an even stricter check would be to treat
> +     *
> +     *          phypNumDomainsGeneric() != phypListDomainsGeneric()
> +     *
> +     *        as an error */
> +    if (nids == 0) {
> +        VIR_FREE(ids);
> +        return 0;
> +    }
> +
>      phyp_driver = conn->privateData;
>      uuid_table = phyp_driver->uuid_table;
>      uuid_table->nlpars = nids;

  Yes I think it's needed to fix the semantic of phypListDefinedDomains
and phypNumDefinedDomains which  are just wrappers around this static
function, and those are public entry points,

 ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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