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
|
Home