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

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

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