WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH] xl: make libxl_uuid2string internal to libxenlig

To: "Gianni Tedesco (3P)" <gianni.tedesco@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] xl: make libxl_uuid2string internal to libxenlight
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Fri, 13 Aug 2010 15:45:01 +0100
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Fri, 13 Aug 2010 07:45:49 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1281710000.18490.325.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <1281710000.18490.325.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
I'm happy for this to replace patches 1+2 of my previous series
(although I think #3 stands alone and is still useful)

However I think this makes string_of_uuid a bit pointless -- it adds
nothing to libxl_uuid2string and only has one caller which could just as
well use libxl_uuid2string.

Ian.


On Fri, 2010-08-13 at 15:33 +0100, Gianni Tedesco (3P) wrote:
> libxenlight exports a function libxl_uuid2string which is used
> internally in several places but has one external caller in xl. The
> function mainly implements policy so should not be part of the
> libxenlight API. The extent to which it can be considered mechanism it
> is not a xen mechanism since UUID's are not a concept exlusive to xen.
> 
> The one caller in xl seems to be an "odd-one-out" since xl has its own
> UUID formatting macros in any case.
> 
> This change fixes the leaks in libxl internal callers which were not
> expecting to have to free() the UUID since the per-api-call-gc-lifetime
> patch.
> 
> Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
> 
> diff -r dc335ebde3b5 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c     Thu Aug 12 18:03:23 2010 +0100
> +++ b/tools/libxl/libxl.c     Fri Aug 13 15:32:31 2010 +0100
> @@ -90,7 +90,7 @@ int libxl_domain_make(libxl_ctx *ctx, li
>      xs_transaction_t t;
>      xen_domain_handle_t handle;
>  
> -    uuid_string = libxl_uuid2string(ctx, info->uuid);
> +    uuid_string = libxl_uuid2string(&gc, info->uuid);
>      if (!uuid_string) {
>          libxl_free_all(&gc);
>          return ERROR_NOMEM;
> @@ -453,7 +453,7 @@ int libxl_domain_preserve(libxl_ctx *ctx
>          return ERROR_NOMEM;
>      }
>  
> -    uuid_string = libxl_uuid2string(ctx, new_uuid);
> +    uuid_string = libxl_uuid2string(&gc, new_uuid);
>      if (!uuid_string) {
>          libxl_free_all(&gc);
>          return ERROR_NOMEM;
> @@ -2785,7 +2785,7 @@ int libxl_set_memory_target(libxl_ctx *c
>      if (rc != 1 || info.domain != domid)
>          goto out;
>      xcinfo2xlinfo(&info, &ptr);
> -    uuid = libxl_uuid2string(ctx, ptr.uuid);
> +    uuid = libxl_uuid2string(&gc, ptr.uuid);
>      libxl_xs_write(&gc, XBT_NULL, libxl_sprintf(&gc, "/vm/%s/memory", uuid), 
> "%"PRIu32, target_memkb / 1024);
>  
>      if (enforce || !domid)
> diff -r dc335ebde3b5 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h     Thu Aug 12 18:03:23 2010 +0100
> +++ b/tools/libxl/libxl.h     Fri Aug 13 15:32:31 2010 +0100
> @@ -362,7 +362,6 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>                           libxl_device_disk *disk,
>                           uint32_t domid);
>  
> -char *libxl_uuid2string(libxl_ctx *ctx, const libxl_uuid uuid);
>    /* 0 means ERROR_ENOMEM, which we have logged */
>  
>  /* events handling */
> diff -r dc335ebde3b5 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c Thu Aug 12 18:03:23 2010 +0100
> +++ b/tools/libxl/libxl_dom.c Fri Aug 13 15:32:31 2010 +0100
> @@ -442,19 +442,12 @@ int save_device_model(libxl_ctx *ctx, ui
>      return 0;
>  }
>  
> -char *libxl_uuid2string(libxl_ctx *ctx, const libxl_uuid uuid)
> +char *libxl_uuid2string(libxl_gc *gc, const libxl_uuid uuid)
>  {
> -    libxl_gc gc = LIBXL_INIT_GC(ctx);
> -    char *s = string_of_uuid(&gc, uuid);
> -    char *ret;
> -    if (!s) {
> -        XL_LOG(ctx, XL_LOG_ERROR, "cannot allocate for uuid");
> -        ret = NULL;
> -    }else{
> -        ret = strdup(s);
> -    }
> -    libxl_free_all(&gc);
> -    return ret;
> +    char *s = string_of_uuid(gc, uuid);
> +    if (!s)
> +        XL_LOG(libxl_gc_owner(gc), XL_LOG_ERROR, "cannot allocate for uuid");
> +    return s;
>  }
>  
>  static const char *userdata_path(libxl_gc *gc, uint32_t domid,
> diff -r dc335ebde3b5 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h    Thu Aug 12 18:03:23 2010 +0100
> +++ b/tools/libxl/libxl_internal.h    Fri Aug 13 15:32:31 2010 +0100
> @@ -249,4 +249,6 @@ _hidden char *libxl_abs_path(libxl_gc *g
>  _hidden char *_libxl_domid_to_name(libxl_gc *gc, uint32_t domid);
>  _hidden char *_libxl_poolid_to_name(libxl_gc *gc, uint32_t poolid);
>  
> +_hidden char *libxl_uuid2string(libxl_gc *gc, const libxl_uuid uuid);
> +
>  #endif
> diff -r dc335ebde3b5 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Thu Aug 12 18:03:23 2010 +0100
> +++ b/tools/libxl/xl_cmdimpl.c        Fri Aug 13 15:32:31 2010 +0100
> @@ -41,6 +41,10 @@
>  #include "xl.h"
>  
>  #define UUID_FMT 
> "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +#define UUID_BYTES(uuid) uuid[0], uuid[1], uuid[2], uuid[3], \
> +            uuid[4], uuid[5], uuid[6], uuid[7], \
> +            uuid[8], uuid[9], uuid[10], uuid[11], \
> +            uuid[12], uuid[13], uuid[14], uuid[15] \
>  
>  #define CHK_ERRNO( call ) ({                                            \
>          int chk_errno = (call);                                         \
> @@ -2169,10 +2173,8 @@ void list_domains(int verbose, const lib
>                  info[i].dying ? 'd' : '-',
>                  ((float)info[i].cpu_time / 1e9));
>          free(domname);
> -        if (verbose) {
> -            char *uuid = libxl_uuid2string(&ctx, info[i].uuid);
> -            printf(" %s", uuid);
> -        }
> +        if (verbose)
> +            printf(" " UUID_FMT, UUID_BYTES(info[i].uuid));
>          putchar('\n');
>      }
>  }
> @@ -2192,11 +2194,7 @@ void list_vm(void)
>      printf("UUID                                  ID    name\n");
>      for (i = 0; i < nb_vm; i++) {
>          domname = libxl_domid_to_name(&ctx, info[i].domid);
> -        printf(UUID_FMT "  %d    %-30s\n",
> -            info[i].uuid[0], info[i].uuid[1], info[i].uuid[2], 
> info[i].uuid[3],
> -            info[i].uuid[4], info[i].uuid[5], info[i].uuid[6], 
> info[i].uuid[7],
> -            info[i].uuid[8], info[i].uuid[9], info[i].uuid[10], 
> info[i].uuid[11],
> -            info[i].uuid[12], info[i].uuid[13], info[i].uuid[14], 
> info[i].uuid[15],
> +        printf(UUID_FMT "  %d    %-30s\n", UUID_BYTES(info[i].uuid),
>              info[i].domid, domname);
>          free(domname);
>      }
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel