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

Re: [libvirt] [PATCH 1/8] hellolibvirt: Update hellolibvirt example



On 19.02.2013 03:14, John Ferlan wrote:
> Add a list of active domains, list of active/inactive networks, and
> list of active/inactive storage pools
> ---
>  examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++-----
>  1 file changed, 173 insertions(+), 28 deletions(-)
> 
> diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c
> index 234637e..f191782 100644
> --- a/examples/hellolibvirt/hellolibvirt.c
> +++ b/examples/hellolibvirt/hellolibvirt.c
> @@ -85,65 +85,200 @@ out:
>      return ret;
>  }
>  
> -
> +typedef int (*virFunction)(virConnectPtr conn,
> +                           char **nameList,
> +                           int maxnames);
>  static int
> -showDomains(virConnectPtr conn)
> +listObject(virConnectPtr conn, int maxnames, const char *objNameStr,
> +           virFunction fcn)
>  {
> -    int ret = 0, i, numNames, numInactiveDomains, numActiveDomains;
> +    int ret = 0, i, numNames;
>      char **nameList = NULL;
>  
> -    numActiveDomains = virConnectNumOfDomains(conn);
> -    if (-1 == numActiveDomains) {
> +    nameList = malloc(sizeof(*nameList) * maxnames);
> +
> +    if (NULL == nameList) {

If blue is the sky .... These Yoda conditions should really be made
inverted. But there are some even outside of the hellolibvirt.c.

>          ret = 1;
> -        printf("Failed to get number of active domains\n");
> +        printf("Could not allocate memory for list of %s\n", objNameStr);
> +        goto out;
> +    }
> +
> +    numNames = (*fcn)(conn, nameList, maxnames);
> +
> +    if (-1 == numNames) {
> +        ret = 1;
> +        printf("Could not get list of %s from hypervisor\n", objNameStr);
>          showError(conn);
>          goto out;
>      }
>  
> -    numInactiveDomains = virConnectNumOfDefinedDomains(conn);
> -    if (-1 == numInactiveDomains) {
> +    printf("    %s: \n", objNameStr);
> +    for (i = 0; i < numNames; i++) {
> +        printf("\t%s\n", *(nameList + i));
> +        /* The API documentation doesn't say so, but the names
> +         * returned by are strdup'd and must be freed here.
> +         */

The docs should be fixed then.

> +        free(*(nameList + i));
> +    }
> +
> +out:
> +    free(nameList);
> +    return ret;
> +}

I wonder if we should advise users to use the other list APIs that are
around for a while (virConnectListAll. I guess it all boils down to
question if the hellolibvirt binary is supposed to work with ancient
libvirts or is just an example shipped within a release.
In case it is supposed to work with prehistoric versions, we must add a
fallback code. If we are satisfied with the example working with current
libvirt, there's no need for fallbacking code then.

Michal


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