Gianni Tedesco
2010-Sep-02 14:29 UTC
[Xen-devel] [PATCH,v7]: xl: randomly generate UUID''s
Changes since v6:
- Non-constify libxl_uuid_bytearray() so that language bindings can use it
to set a UUID.
- Convert ocaml bindings to new API
- Is this patch cursed or what?
Changes since v5:
- There was an error caused due to missing change so the check for bad
parsing of a "uuid" config option did not lead to error message and
exit
as I had stated in v4 announcement. Phew, sorry for the noise
Changes since v4:
- Really fix it for NetBSD? Define our own structure which is a byte
array. Only use uuid_is_nul and uuid_create which should be safe for
our portability purposes. Previous attempt at portability wrapper was
just wrong minded.
- Remove uuid_to_string since our approach is to use a couple of macros
for this
- License under LGPL in line with rest of libxl
Changes since v3:
- Fix LIBXL_UUID_BYTES on NetBSD. Note that the code assumes
uint8_t[16] to always be interchangeable with libxl_uuid_t.
- Return error messages when uuid_parse fails, spotted by Owen Smith
- Implement "uuid" parameter in xl and exit with an error if parse
fails
Changes since v2:
- 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
------8<---------------------------------------------------------------
This patch converts xl to randomly generate UUID''s rather than using a
dodgy time-seeded PRNG. I have ignored various suggestions so far on
auto-generation of MAC addresses and left it as a topic for a future
patch to solve. In other words the behaviour stays the same it''s just
using a true random source. This patch also implements the "uuid"
config
file parameter in xl.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r 573ddf5cc145 tools/libxl/Makefile
--- a/tools/libxl/Makefile Tue Aug 31 19:16:23 2010 +0100
+++ b/tools/libxl/Makefile Thu Sep 02 15:29:01 2010 +0100
@@ -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 573ddf5cc145 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Tue Aug 31 19:16:23 2010 +0100
+++ b/tools/libxl/libxl.c Thu Sep 02 15:29:01 2010 +0100
@@ -131,7 +131,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) {
@@ -1506,8 +1506,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 573ddf5cc145 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Tue Aug 31 19:16:23 2010 +0100
+++ b/tools/libxl/libxl.h Thu Sep 02 15:29:01 2010 +0100
@@ -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 573ddf5cc145 tools/libxl/libxl_uuid.h
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/libxl_uuid.h Thu Sep 02 15:29:01 2010 +0100
@@ -0,0 +1,134 @@
+/*
+ * Copyright (C) 2008,2010 Citrix Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#ifndef __LIBXL_UUID_H__
+#define __LIBXL_UUID_H__
+
+#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]
+
+#if defined(__linux__)
+
+#include <uuid/uuid.h>
+
+typedef struct {
+ uuid_t uuid;
+} libxl_uuid;
+
+#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES(((uint8_t *)arg.uuid))
+
+static inline int libxl_uuid_is_nil(libxl_uuid *uuid)
+{
+ return uuid_is_null(uuid->uuid);
+}
+
+static inline void libxl_uuid_generate(libxl_uuid *uuid)
+{
+ uuid_generate(uuid->uuid);
+}
+
+static inline int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
+{
+ return uuid_parse(in, uuid->uuid);
+}
+
+static inline void libxl_uuid_copy(libxl_uuid *dst, libxl_uuid *src)
+{
+ uuid_copy(dst->uuid, src->uuid);
+}
+
+static inline void libxl_uuid_clear(libxl_uuid *uuid)
+{
+ uuid_clear(uuid->uuid);
+}
+
+static inline int libxl_uuid_compare(libxl_uuid *uuid1, libxl_uuid *uuid2)
+{
+ return uuid_compare(uuid1->uuid, uuid2->uuid);
+}
+
+static inline uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid)
+{
+ return uuid->uuid;
+}
+
+#elif defined(__NetBSD__)
+
+#include <uuid.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+
+#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES(arg.uuid)
+
+typedef struct {
+ uint8_t uuid[16];
+} libxl_uuid;
+
+static inline int libxl_uuid_is_nil(libxl_uuid *uuid)
+{
+ uint32_t status;
+ return uuid_is_nil((uuid_t *)uuid->uuid, &status);
+}
+
+static inline void libxl_uuid_generate(libxl_uuid *uuid)
+{
+ uint32_t status;
+ uuid_create((uuid_t *)uuid->uuid, &status);
+ assert(status == uuid_s_ok);
+}
+
+#define LIBXL__UUID_PTRS(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]
+static inline int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
+{
+ if ( sscanf(in, LIBXL_UUID_FMT, LIBXL__UUID_PTRS(uuid->uuid)) !=
sizeof(uuid->uuid) )
+ return -1;
+ return 0;
+}
+#undef LIBXL__UUID_PTRS
+
+static inline void libxl_uuid_copy(libxl_uuid *dst, libxl_uuid *src)
+{
+ memcpy(dst->uuid, src->uuid, sizeof(dst->uuid));
+}
+
+static inline void libxl_uuid_clear(libxl_uuid *uuid)
+{
+ memset(uuid->uuid, 0, sizeof(uuid->uuid));
+}
+
+static inline int libxl_uuid_compare(libxl_uuid *uuid1, libxl_uuid *uuid2)
+{
+ return memcmp(uuid1->uuid, uuid2->uuid, sizeof(uuid1->uuid));
+}
+
+static inline uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid)
+{
+ return uuid->uuid;
+}
+
+#else
+
+#error "Please update libxl_uuid.h for your OS"
+
+#endif
+
+#endif /* __LIBXL_UUID_H__ */
diff -r 573ddf5cc145 tools/libxl/xl.c
--- a/tools/libxl/xl.c Tue Aug 31 19:16:23 2010 +0100
+++ b/tools/libxl/xl.c Thu Sep 02 15:29:01 2010 +0100
@@ -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 573ddf5cc145 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue Aug 31 19:16:23 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Thu Sep 02 15:29:01 2010 +0100
@@ -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)
{
+ const uint8_t *r;
+ libxl_uuid uuid;
+
+ libxl_uuid_generate(&uuid);
+ r = libxl_uuid_bytearray(&uuid);
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)
{
+ const uint8_t *r;
+ libxl_uuid uuid;
+
+ libxl_uuid_generate(&uuid);
+ r = libxl_uuid_bytearray(&uuid);
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;
@@ -604,8 +607,16 @@ static void parse_config_data(const char
c_info->name = strdup(buf);
else
c_info->name = "test";
- random_uuid(&c_info->uuid);
-
+
+ if (!xlu_cfg_get_string (config, "uuid", &buf) ) {
+ if ( libxl_uuid_from_string(&c_info->uuid, buf) ) {
+ fprintf(stderr, "Failed to parse UUID: %s\n", buf);
+ exit(1);
+ }
+ }else{
+ libxl_uuid_generate(&c_info->uuid);
+ }
+
if (!xlu_cfg_get_long(config, "oos", &l))
c_info->oos = l;
@@ -1206,7 +1217,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);
diff -r 573ddf5cc145 tools/ocaml/libs/xl/xl_stubs.c
--- a/tools/ocaml/libs/xl/xl_stubs.c Tue Aug 31 19:16:23 2010 +0100
+++ b/tools/ocaml/libs/xl/xl_stubs.c Thu Sep 02 15:29:01 2010 +0100
@@ -131,6 +131,7 @@ static int domain_create_info_val (caml_
{
CAMLparam1(v);
CAMLlocal1(a);
+ uint8_t *uuid = libxl_uuid_bytearray(&c_val->uuid);
int i;
c_val->hvm = Bool_val(Field(v, 0));
@@ -140,7 +141,7 @@ static int domain_create_info_val (caml_
c_val->name = dup_String_val(gc, Field(v, 4));
a = Field(v, 5);
for (i = 0; i < 16; i++)
- c_val->uuid[i] = Int_val(Field(a, i));
+ uuid[i] = Int_val(Field(a, i));
string_string_tuple_array_val(gc, &(c_val->xsdata), Field(v, 6));
string_string_tuple_array_val(gc, &(c_val->platformdata), Field(v, 7));
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Christoph Egger
2010-Sep-02 15:53 UTC
[Xen-devel] Re: [PATCH,v7]: xl: randomly generate UUID''s
On Thursday 02 September 2010 16:29:06 Gianni Tedesco wrote:> Changes since v6: > - Non-constify libxl_uuid_bytearray() so that language bindings can use it > to set a UUID. > - Convert ocaml bindings to new API > - Is this patch cursed or what? > Changes since v5: > - There was an error caused due to missing change so the check for bad > parsing of a "uuid" config option did not lead to error message and exit > as I had stated in v4 announcement. Phew, sorry for the noise > Changes since v4: > - Really fix it for NetBSD? Define our own structure which is a byte > array. Only use uuid_is_nul and uuid_create which should be safe for > our portability purposes. Previous attempt at portability wrapper was > just wrong minded. > - Remove uuid_to_string since our approach is to use a couple of macros > for this > - License under LGPL in line with rest of libxl > Changes since v3: > - Fix LIBXL_UUID_BYTES on NetBSD. Note that the code assumes > uint8_t[16] to always be interchangeable with libxl_uuid_t. > - Return error messages when uuid_parse fails, spotted by Owen Smith > - Implement "uuid" parameter in xl and exit with an error if parse > fails > Changes since v2: > - 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 > > ------8<--------------------------------------------------------------- > This patch converts xl to randomly generate UUID''s rather than using a > dodgy time-seeded PRNG. I have ignored various suggestions so far on > auto-generation of MAC addresses and left it as a topic for a future > patch to solve. In other words the behaviour stays the same it''s just > using a true random source. This patch also implements the "uuid" config > file parameter in xl. > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>Acked-By: Christoph Egger <Christoph.Egger@amd.com>> > diff -r 573ddf5cc145 tools/libxl/Makefile > --- a/tools/libxl/Makefile Tue Aug 31 19:16:23 2010 +0100 > +++ b/tools/libxl/Makefile Thu Sep 02 15:29:01 2010 +0100 > @@ -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 573ddf5cc145 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Tue Aug 31 19:16:23 2010 +0100 > +++ b/tools/libxl/libxl.c Thu Sep 02 15:29:01 2010 +0100 > @@ -131,7 +131,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) { > @@ -1506,8 +1506,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 573ddf5cc145 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Tue Aug 31 19:16:23 2010 +0100 > +++ b/tools/libxl/libxl.h Thu Sep 02 15:29:01 2010 +0100 > @@ -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%02h >hx%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 573ddf5cc145 tools/libxl/libxl_uuid.h > --- /dev/null Thu Jan 01 00:00:00 1970 +0000 > +++ b/tools/libxl/libxl_uuid.h Thu Sep 02 15:29:01 2010 +0100 > @@ -0,0 +1,134 @@ > +/* > + * Copyright (C) 2008,2010 Citrix Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Lesser General Public License as > published + * by the Free Software Foundation; version 2.1 only. with the > special + * exception on linking described in file LICENSE. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU Lesser General Public License for more details. > + */ > + > +#ifndef __LIBXL_UUID_H__ > +#define __LIBXL_UUID_H__ > + > +#define LIBXL_UUID_FMT > "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02h >hx%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] + > +#if defined(__linux__) > + > +#include <uuid/uuid.h> > + > +typedef struct { > + uuid_t uuid; > +} libxl_uuid; > + > +#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES(((uint8_t *)arg.uuid)) > + > +static inline int libxl_uuid_is_nil(libxl_uuid *uuid) > +{ > + return uuid_is_null(uuid->uuid); > +} > + > +static inline void libxl_uuid_generate(libxl_uuid *uuid) > +{ > + uuid_generate(uuid->uuid); > +} > + > +static inline int libxl_uuid_from_string(libxl_uuid *uuid, const char *in) > +{ > + return uuid_parse(in, uuid->uuid); > +} > + > +static inline void libxl_uuid_copy(libxl_uuid *dst, libxl_uuid *src) > +{ > + uuid_copy(dst->uuid, src->uuid); > +} > + > +static inline void libxl_uuid_clear(libxl_uuid *uuid) > +{ > + uuid_clear(uuid->uuid); > +} > + > +static inline int libxl_uuid_compare(libxl_uuid *uuid1, libxl_uuid *uuid2) > +{ > + return uuid_compare(uuid1->uuid, uuid2->uuid); > +} > + > +static inline uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid) > +{ > + return uuid->uuid; > +} > + > +#elif defined(__NetBSD__) > + > +#include <uuid.h> > +#include <string.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <assert.h> > + > +#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES(arg.uuid) > + > +typedef struct { > + uint8_t uuid[16]; > +} libxl_uuid; > + > +static inline int libxl_uuid_is_nil(libxl_uuid *uuid) > +{ > + uint32_t status; > + return uuid_is_nil((uuid_t *)uuid->uuid, &status); > +} > + > +static inline void libxl_uuid_generate(libxl_uuid *uuid) > +{ > + uint32_t status; > + uuid_create((uuid_t *)uuid->uuid, &status); > + assert(status == uuid_s_ok); > +} > + > +#define LIBXL__UUID_PTRS(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] > +static inline int libxl_uuid_from_string(libxl_uuid *uuid, const char *in) > +{ > + if ( sscanf(in, LIBXL_UUID_FMT, LIBXL__UUID_PTRS(uuid->uuid)) !> sizeof(uuid->uuid) ) + return -1; > + return 0; > +} > +#undef LIBXL__UUID_PTRS > + > +static inline void libxl_uuid_copy(libxl_uuid *dst, libxl_uuid *src) > +{ > + memcpy(dst->uuid, src->uuid, sizeof(dst->uuid)); > +} > + > +static inline void libxl_uuid_clear(libxl_uuid *uuid) > +{ > + memset(uuid->uuid, 0, sizeof(uuid->uuid)); > +} > + > +static inline int libxl_uuid_compare(libxl_uuid *uuid1, libxl_uuid *uuid2) > +{ > + return memcmp(uuid1->uuid, uuid2->uuid, sizeof(uuid1->uuid)); > +} > + > +static inline uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid) > +{ > + return uuid->uuid; > +} > + > +#else > + > +#error "Please update libxl_uuid.h for your OS" > + > +#endif > + > +#endif /* __LIBXL_UUID_H__ */ > diff -r 573ddf5cc145 tools/libxl/xl.c > --- a/tools/libxl/xl.c Tue Aug 31 19:16:23 2010 +0100 > +++ b/tools/libxl/xl.c Thu Sep 02 15:29:01 2010 +0100 > @@ -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 573ddf5cc145 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Tue Aug 31 19:16:23 2010 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Thu Sep 02 15:29:01 2010 +0100 > @@ -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) > { > + const uint8_t *r; > + libxl_uuid uuid; > + > + libxl_uuid_generate(&uuid); > + r = libxl_uuid_bytearray(&uuid); > 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) > { > + const uint8_t *r; > + libxl_uuid uuid; > + > + libxl_uuid_generate(&uuid); > + r = libxl_uuid_bytearray(&uuid); > 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; > @@ -604,8 +607,16 @@ static void parse_config_data(const char > c_info->name = strdup(buf); > else > c_info->name = "test"; > - random_uuid(&c_info->uuid); > - > + > + if (!xlu_cfg_get_string (config, "uuid", &buf) ) { > + if ( libxl_uuid_from_string(&c_info->uuid, buf) ) { > + fprintf(stderr, "Failed to parse UUID: %s\n", buf); > + exit(1); > + } > + }else{ > + libxl_uuid_generate(&c_info->uuid); > + } > + > if (!xlu_cfg_get_long(config, "oos", &l)) > c_info->oos = l; > > @@ -1206,7 +1217,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); diff -r 573ddf5cc145 > tools/ocaml/libs/xl/xl_stubs.c > --- a/tools/ocaml/libs/xl/xl_stubs.c Tue Aug 31 19:16:23 2010 +0100 > +++ b/tools/ocaml/libs/xl/xl_stubs.c Thu Sep 02 15:29:01 2010 +0100 > @@ -131,6 +131,7 @@ static int domain_create_info_val (caml_ > { > CAMLparam1(v); > CAMLlocal1(a); > + uint8_t *uuid = libxl_uuid_bytearray(&c_val->uuid); > int i; > > c_val->hvm = Bool_val(Field(v, 0)); > @@ -140,7 +141,7 @@ static int domain_create_info_val (caml_ > c_val->name = dup_String_val(gc, Field(v, 4)); > a = Field(v, 5); > for (i = 0; i < 16; i++) > - c_val->uuid[i] = Int_val(Field(a, i)); > + uuid[i] = Int_val(Field(a, i)); > string_string_tuple_array_val(gc, &(c_val->xsdata), Field(v, 6)); > string_string_tuple_array_val(gc, &(c_val->platformdata), Field(v, > 7));-- ---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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Sep-06 09:54 UTC
Re: [Xen-devel] [PATCH,v7]: xl: randomly generate UUID''s
This seems to have exposed a bug in the userdata scheme which breaks
localhost live migration. Attempted fix is below. I''m a little bit
concerned that it might expose leaks or other occasions where we loose
track of userdata but I think it is correct.
Ian.
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1283766752 -3600
# Node ID 75449758ea76cc3fd75a906ba827c51d10e62428
# Parent a4b74331bed1aa2ebec68af2a2d9306d7d41986f
libxl: include domain id in userdata path.
The userdata is specific to a particular incarnation of a domain and
the patch is therefor required to be unique to each incarnation. If
the user has explicitly configured a UUID in their domain
configuration then the path is no longer unique since
22124:22366e13f76d "xl: randomly generate UUIDs" which (correctly)
caused the uuid domain configuration option to be obeyed.
If userdata is not unique to each incarnation of a domain then
localhost live migration is broken because the target is created (and
writes its userdata) before the sender destroys the domain (and
deletes its userdata).
Strictly speaking I think the UUID is unnecessary but it is perhaps
helpful to people looking in the userdata directory, for debugging
etc.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r a4b74331bed1 -r 75449758ea76 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Mon Sep 06 10:47:31 2010 +0100
+++ b/tools/libxl/libxl_dom.c Mon Sep 06 10:52:32 2010 +0100
@@ -466,8 +466,8 @@ static const char *userdata_path(libxl_g
uuid_string = libxl_sprintf(gc, LIBXL_UUID_FMT,
LIBXL_UUID_BYTES(info.uuid));
path = libxl_sprintf(gc, "/var/lib/xen/"
- "userdata-%s.%s.%s",
- wh, uuid_string, userdata_userid);
+ "userdata-%s.%u.%s.%s",
+ wh, domid, uuid_string, userdata_userid);
if (!path)
XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unable to allocate for"
" userdata path");
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Sep-06 10:17 UTC
Re: [Xen-devel] [PATCH,v7]: xl: randomly generate UUID''s
On Mon, 2010-09-06 at 10:54 +0100, Ian Campbell wrote:> I''m a little bit concerned that it might expose leaks or other > occasions where we loose track of userdata but I think it is correct.On a related note is userdata supposed to persist across a host reboot? If not then perhaps we should move it from /var/lib/xen to somewhere that is cleaned on boot? Perhaps /var/run/xen/ or a sub-directory. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2010-Sep-06 10:20 UTC
Re: [Xen-devel] [PATCH,v7]: xl: randomly generate UUID''s
On Monday 06 September 2010 12:17:32 Ian Campbell wrote:> On Mon, 2010-09-06 at 10:54 +0100, Ian Campbell wrote: > > I''m a little bit concerned that it might expose leaks or other > > occasions where we loose track of userdata but I think it is correct. > > On a related note is userdata supposed to persist across a host reboot? > > If not then perhaps we should move it from /var/lib/xen to somewhere > that is cleaned on boot? Perhaps /var/run/xen/ or a sub-directory.Both directories are fine in respect to NetBSD. 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Sep-07 17:54 UTC
Re: [Xen-devel] [PATCH,v7]: xl: randomly generate UUID''s
Ian Campbell writes ("Re: [Xen-devel] [PATCH,v7]: xl: randomly generate
UUID''s"):> On Mon, 2010-09-06 at 10:54 +0100, Ian Campbell wrote:
> > I''m a little bit concerned that it might expose leaks or
other
> > occasions where we loose track of userdata but I think it is correct.
>
> On a related note is userdata supposed to persist across a host reboot?
No.
> If not then perhaps we should move it from /var/lib/xen to somewhere
> that is cleaned on boot? Perhaps /var/run/xen/ or a sub-directory.
That''s probably a good idea.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel