Re: [PATCH, v3]: xl: randomly generate UUID's

[Christoph Egger <Christoph.Egger@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 1767 bytes --]

On Thursday 26 August 2010 14:02:35 Christoph Egger wrote:
> On Wednesday 25 August 2010 20:38:25 Gianni Tedesco wrote:
> > On Fri, 2010-08-20 at 16:50 +0100, Christoph Egger wrote:
> > > On Friday 20 August 2010 17:15:21 Gianni Tedesco wrote:
> > > > Changes since last time:
> > > >  - Re-based to remove orthogonal concern of UUID string formatting
> > > > fixed in 22001:0b6f82eaaea9 "xl: make libxl_uuid2string internal to
> > > > libxenlight" - Incorporated Christoph Egger's suggestions
> > >
> > > I will give this patch a try next week.
> > > Christoph
> >
> > *ping* - any news? I think it's straightforward so should be good to go.
>
> I'm about reworking the pieces that don't compile. That is mainly
> the use of LIBXL_UUID_FMT causing warnings like
> 'subscripted value is neither array nor pointer'.

Ok, here we go. The patch is against changeset 22068:3c4c3d48a835
and is not ready yet though, it needs to also
fix build for tools/ocaml/libs/xl/xl_stubs.c.

The build fails with
xl_stubs.c:143: error: subscripted value is neither array nor pointer.

I think using libxl_uuid_from_string() should do it but I'm not familiar
with oxenstored.

I am not sure if the use of malloc() in libxl_uuid2string() is correct,
someone familiar with the garbage collector please fix it.

Due to the mentioned caveats, I don't acknowledge this patch.
Nonetheless, I am sending the patch out to not thwart Gianni to do his
development work.
I want to re-test another version of this patch.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_tools_libxluuid.diff --]
[-- Type: text/x-diff, Size: 9466 bytes --]

diff -r 8fc73880aea9 -r 30adef8fa3d2 tools/libxl/Makefile
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -16,6 +16,9 @@ CFLAGS += -I. -fPIC
 CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl)
 
 LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(UTIL_LIBS)
+ifeq ($(CONFIG_Linux),y)
+LIBS += -luuid
+endif
 
 LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o
 ifeq ($(LIBXL_BLKTAP),y)
diff -r 8fc73880aea9 -r 30adef8fa3d2 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -127,7 +127,7 @@ int libxl_domain_make(libxl_ctx *ctx, li
     *domid = -1;
 
     /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
-    memcpy(handle, info->uuid, sizeof(xen_domain_handle_t));
+    libxl_uuid_copy((libxl_uuid *)handle, &info->uuid);
 
     ret = xc_domain_create(ctx->xch, info->ssidref, handle, flags, domid);
     if (ret < 0) {
@@ -1502,8 +1502,8 @@ static int libxl_create_stubdom(libxl_ct
     memset(&c_info, 0x00, sizeof(libxl_domain_create_info));
     c_info.hvm = 0;
     c_info.name = libxl_sprintf(&gc, "%s-dm", _libxl_domid_to_name(&gc, info->domid));
-    for (i = 0; i < 16; i++)
-        c_info.uuid[i] = info->uuid[i];
+
+    libxl_uuid_copy(&c_info.uuid, &info->uuid);
 
     memset(&b_info, 0x00, sizeof(libxl_domain_build_info));
     b_info.max_vcpus = 1;
diff -r 8fc73880aea9 -r 30adef8fa3d2 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -131,13 +131,7 @@
 #include <xs.h>
 #include <sys/wait.h> /* for pid_t */
 
-typedef uint8_t libxl_uuid[16];
-#define LIBXL_UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
-#define LIBXL_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] \
-
+#include "libxl_uuid.h"
 
 typedef uint8_t libxl_mac[6];
 
diff -r 8fc73880aea9 -r 30adef8fa3d2 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -442,11 +442,17 @@ int save_device_model(libxl_ctx *ctx, ui
     return 0;
 }
 
-char *libxl_uuid2string(libxl_gc *gc, const libxl_uuid uuid)
+char *libxl_uuid2string(libxl_gc *gc, libxl_uuid uuid)
 {
-    char *s = libxl_sprintf(gc, LIBXL_UUID_FMT, LIBXL_UUID_BYTES(uuid));
-    if (!s)
+    char *s;
+
+    s = malloc(LIBXL_UUID_LEN);
+    if (!s) {
         XL_LOG(libxl_gc_owner(gc), XL_LOG_ERROR, "cannot allocate for uuid");
+        return NULL;
+    }
+
+    libxl_uuido_string(&uuid, s, sizeof(s));
     return s;
 }
 
@@ -465,7 +471,7 @@ static const char *userdata_path(libxl_g
                      " for domain %"PRIu32, domid);
         return NULL;
     }
-    uuid_string = libxl_sprintf(gc, LIBXL_UUID_FMT, LIBXL_UUID_BYTES(info.uuid));
+    uuid_string = libxl_uuid2string(gc, info.uuid);
 
     path = libxl_sprintf(gc, "/var/lib/xen/"
                          "userdata-%s.%s.%s",
diff -r 8fc73880aea9 -r 30adef8fa3d2 tools/libxl/xl.c
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -74,8 +74,6 @@ int main(int argc, char **argv)
     argc -= optind;
     optind = 1;
 
-    srand(time(0));
-
     cspec = cmdtable_lookup(cmd);
     if (cspec)
         ret = cspec->cmd_impl(argc, argv);
diff -r 8fc73880aea9 -r 30adef8fa3d2 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -286,19 +286,12 @@ static void init_build_info(libxl_domain
     }
 }
 
-static void random_uuid(libxl_uuid *uuid)
-{
-    int i;
-    for (i = 0; i < 16; i++)
-        (*uuid)[i] = rand();
-}
-
 static void init_dm_info(libxl_device_model_info *dm_info,
         libxl_domain_create_info *c_info, libxl_domain_build_info *b_info)
 {
     memset(dm_info, '\0', sizeof(*dm_info));
 
-    random_uuid(&dm_info->uuid);
+    libxl_uuid_generate(&dm_info->uuid);
 
     dm_info->dom_name = c_info->name;
     dm_info->device_model = "qemu-dm";
@@ -325,6 +318,11 @@ static void init_dm_info(libxl_device_mo
 
 static void init_nic_info(libxl_device_nic *nic_info, int devnum)
 {
+    uint8_t r[LIBXL_UUID_LEN];
+    libxl_uuid uuid;
+
+    libxl_uuid_generate(&uuid);
+    libxl_uuido_string(&uuid, (char *)r, sizeof(r));
     memset(nic_info, '\0', sizeof(*nic_info));
 
     nic_info->backend_domid = 0;
@@ -335,9 +333,9 @@ static void init_nic_info(libxl_device_n
     nic_info->mac[0] = 0x00;
     nic_info->mac[1] = 0x16;
     nic_info->mac[2] = 0x3e;
-    nic_info->mac[3] = 1 + (int) (0x7f * (rand() / (RAND_MAX + 1.0)));
-    nic_info->mac[4] = 1 + (int) (0xff * (rand() / (RAND_MAX + 1.0)));
-    nic_info->mac[5] = 1 + (int) (0xff * (rand() / (RAND_MAX + 1.0)));
+    nic_info->mac[3] = r[0] & 0x7f;
+    nic_info->mac[4] = r[1];
+    nic_info->mac[5] = r[2];
     nic_info->ifname = NULL;
     nic_info->bridge = strdup("xenbr0");
     CHK_ERRNO( asprintf(&nic_info->script, "%s/vif-bridge",
@@ -347,21 +345,26 @@ static void init_nic_info(libxl_device_n
 
 static void init_net2_info(libxl_device_net2 *net2_info, int devnum)
 {
+    uint8_t r[LIBXL_UUID_LEN];
+    libxl_uuid uuid;
+
+    libxl_uuid_generate(&uuid);
+    libxl_uuido_string(&uuid, (char *)r, sizeof(r));
     memset(net2_info, '\0', sizeof(*net2_info));
 
     net2_info->devid = devnum;
     net2_info->front_mac[0] = 0x00;
     net2_info->front_mac[1] = 0x16;
     net2_info->front_mac[2] = 0x3e;;
-    net2_info->front_mac[3] = 1 + (int) (0x7f * (rand() / (RAND_MAX + 1.0)));
-    net2_info->front_mac[4] = 1 + (int) (0xff * (rand() / (RAND_MAX + 1.0)));
-    net2_info->front_mac[5] = 1 + (int) (0xff * (rand() / (RAND_MAX + 1.0)));
+    net2_info->front_mac[3] = 0x7f & r[0];
+    net2_info->front_mac[4] = r[1];
+    net2_info->front_mac[5] = r[2];
     net2_info->back_mac[0] = 0x00;
     net2_info->back_mac[1] = 0x16;
     net2_info->back_mac[2] = 0x3e;
-    net2_info->back_mac[3] = 1 + (int) (0x7f * (rand() / (RAND_MAX + 1.0)));
-    net2_info->back_mac[4] = 1 + (int) (0xff * (rand() / (RAND_MAX + 1.0)));
-    net2_info->back_mac[5] = 1 + (int) (0xff * (rand() / (RAND_MAX + 1.0)));
+    net2_info->back_mac[3] = 0x7f & r[3];
+    net2_info->back_mac[4] = r[4];
+    net2_info->back_mac[5] = r[5];
     net2_info->back_trusted = 1;
     net2_info->filter_mac = 1;
     net2_info->max_bypasses = 5;
@@ -402,6 +405,7 @@ static void printf_info(int domid,
                         libxl_device_model_info *dm_info)
 {
     int i;
+    char buf[LIBXL_UUID_LEN];
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
@@ -413,7 +417,8 @@ static void printf_info(int domid,
     printf("\t(oos %d)\n", c_info->oos);
     printf("\t(ssidref %d)\n", c_info->ssidref);
     printf("\t(name %s)\n", c_info->name);
-    printf("\t(uuid " LIBXL_UUID_FMT ")\n", LIBXL_UUID_BYTES(c_info->uuid));
+    libxl_uuido_string(&c_info->uuid, buf, sizeof(buf));
+    printf("\t(uuid %s)\n", buf);
     printf("\t(cpupool %s (%d))\n", c_info->poolname, c_info->poolid);
     if (c_info->xsdata)
         printf("\t(xsdata contains data)\n");
@@ -604,7 +609,7 @@ static void parse_config_data(const char
         c_info->name = strdup(buf);
     else
         c_info->name = "test";
-    random_uuid(&c_info->uuid);
+    libxl_uuid_generate(&c_info->uuid);
 
     if (!xlu_cfg_get_long(config, "oos", &l))
         c_info->oos = l;
@@ -1206,7 +1211,7 @@ static int preserve_domain(libxl_ctx *ct
         return 0;
     }
 
-    random_uuid(&new_uuid);
+    libxl_uuid_generate(&new_uuid);
 
     LOG("Preserving domain %d %s with suffix%s", domid, d_config->c_info.name, stime);
     rc = libxl_domain_preserve(ctx, domid, &d_config->c_info, stime, new_uuid);
@@ -2198,7 +2203,7 @@ static void list_domains_details(const l
     }
 }
 
-static void list_domains(int verbose, const libxl_dominfo *info, int nb_domain)
+static void list_domains(int verbose, libxl_dominfo *info, int nb_domain)
 {
     int i;
 
@@ -2219,8 +2224,11 @@ static void list_domains(int verbose, co
                 info[i].dying ? 'd' : '-',
                 ((float)info[i].cpu_time / 1e9));
         free(domname);
-        if (verbose)
-            printf(" " LIBXL_UUID_FMT, LIBXL_UUID_BYTES(info[i].uuid));
+        if (verbose) {
+            char buf[LIBXL_UUID_LEN];
+            libxl_uuido_string(&info[i].uuid, buf, sizeof(buf));
+            printf(" %s", buf);
+        }
         putchar('\n');
     }
 }
@@ -2230,6 +2238,7 @@ static void list_vm(void)
     libxl_vminfo *info;
     char *domname;
     int nb_vm, i;
+    char uuidstr[LIBXL_UUID_LEN];
 
     info = libxl_list_vm(&ctx, &nb_vm);
 
@@ -2240,7 +2249,8 @@ static 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(LIBXL_UUID_FMT "  %d    %-30s\n", LIBXL_UUID_BYTES(info[i].uuid),
+        libxl_uuido_string(&info[i].uuid, uuidstr, sizeof(uuidstr));
+        printf("%s  %d    %-30s\n", uuidstr,
             info[i].domid, domname);
         free(domname);
     }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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