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,v7]: xl: randomly generate UUID's

To: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH,v7]: xl: randomly generate UUID's
From: Christoph Egger <Christoph.Egger@xxxxxxx>
Date: Thu, 2 Sep 2010 17:53:26 +0200
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Thu, 02 Sep 2010 08:55:59 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1283437746.20276.65.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>
References: <1283437746.20276.65.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.10
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@xxxxxxxxxx>

Acked-By: Christoph Egger <Christoph.Egger@xxxxxxx>

>
> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel