| Message ID | 1459941065-17573-1-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 06, 2016 at 01:11:05PM +0200, Roger Pau Monne wrote: > The internals of the uuid_t struct don't match a big endian octet stream on > *BSD systems, which means that it cannot be directly casted to a > uint8_t[16]. > > In order to solve that change the type to be an unsigned char[16], which > doesn't imply any other change on Linux. On *BSDs change the helpers so that > the uuid is always stored as a big endian byte stream. > > NB: tested on FreeBSD and Linux only. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Discussed-with: Ian Jackson <Ian.Jackson@eu.citrix.com> > Discussed-with: Wei Liu <wei.liu2@citrix.com> > --- > Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > NB2: AFAICT the NetBSD version of libxl_uuid_from_string *could* be switched > to the FreeBSD one (because NetBSD also has uuid_from_string), but I don't > have a NetBSD box in order to test it. Haven't looked at the code but maybe it's a good idea to CC netbsd port-xen list? Wei.
On Wed, Apr 06, 2016 at 01:11:05PM +0200, Roger Pau Monne wrote: > The internals of the uuid_t struct don't match a big endian octet stream on > *BSD systems, which means that it cannot be directly casted to a > uint8_t[16]. > > In order to solve that change the type to be an unsigned char[16], which > doesn't imply any other change on Linux. On *BSDs change the helpers so that > the uuid is always stored as a big endian byte stream. > > NB: tested on FreeBSD and Linux only. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Discussed-with: Ian Jackson <Ian.Jackson@eu.citrix.com> > Discussed-with: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
On Wed, Apr 06, 2016 at 01:11:05PM +0200, Roger Pau Monne wrote: > The internals of the uuid_t struct don't match a big endian octet stream on > *BSD systems, which means that it cannot be directly casted to a > uint8_t[16]. > > In order to solve that change the type to be an unsigned char[16], which > doesn't imply any other change on Linux. On *BSDs change the helpers so that > the uuid is always stored as a big endian byte stream. > > NB: tested on FreeBSD and Linux only. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Discussed-with: Ian Jackson <Ian.Jackson@eu.citrix.com> > Discussed-with: Wei Liu <wei.liu2@citrix.com> > --- > Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > NB2: AFAICT the NetBSD version of libxl_uuid_from_string *could* be switched > to the FreeBSD one (because NetBSD also has uuid_from_string), but I don't > have a NetBSD box in order to test it. > --- > Changes since v1: > - Readd a line that was deleted by error. > --- > tools/libxl/libxl_osdeps.h | 3 +++ > tools/libxl/libxl_uuid.c | 28 ++++++++++++++++++++-------- > tools/libxl/libxl_uuid.h | 21 ++++++++------------- A thought: maybe it is worth to have a #define LIBXL_HAVE_UNIFIED_UUID in libxl.h? /* If this is defined, libxl_uuid is big endian 16-octet stream on all * platform. The libxl_uuid API family will handle transformation * between libxl_uuid format and OS specific format. */ #define LBIXL_HAVE_UNIFIED_UUID 1 Wei.
Wei Liu writes ("Re: [PATCH v2] libxl: replace the usage of uuid_t with a char array"):
> A thought: maybe it is worth to have a #define LIBXL_HAVE_UNIFIED_UUID
> in libxl.h?
This is a good idea.
> /* If this is defined, libxl_uuid is big endian 16-octet stream on all
> * platform. The libxl_uuid API family will handle transformation
> * between libxl_uuid format and OS specific format.
> */
> #define LBIXL_HAVE_UNIFIED_UUID 1
But the wording here isn't, quite. The "transformation between
libxl_uuid format and OS specific format" is entirely hidden within
libxl and exists only to implement the functions provided by libxl.
Ian.
On Thu, 7 Apr 2016, Ian Jackson wrote: > Wei Liu writes ("Re: [PATCH v2] libxl: replace the usage of uuid_t with a char array"): > > A thought: maybe it is worth to have a #define LIBXL_HAVE_UNIFIED_UUID > > in libxl.h? > > This is a good idea. > > > /* If this is defined, libxl_uuid is big endian 16-octet stream on all > > * platform. The libxl_uuid API family will handle transformation > > * between libxl_uuid format and OS specific format. > > */ > > #define LBIXL_HAVE_UNIFIED_UUID 1 > > But the wording here isn't, quite. The "transformation between > libxl_uuid format and OS specific format" is entirely hidden within > libxl and exists only to implement the functions provided by libxl. What about: /* * LIBXL_HAVE_BYTEARRAY_UUID * * If this is defined, the internal member of libxl_uuid is defined * as a 16 byte array that contains the UUID in big endian format. * Also, the same structure layout is used across all OSes. */ #define LIBXL_HAVE_BYTEARRAY_UUID 1
Roger Pau Monné writes ("Re: [PATCH v2] libxl: replace the usage of uuid_t with a char array"):
> What about:
>
> /*
> * LIBXL_HAVE_BYTEARRAY_UUID
> *
> * If this is defined, the internal member of libxl_uuid is defined
> * as a 16 byte array that contains the UUID in big endian format.
> * Also, the same structure layout is used across all OSes.
> */
> #define LIBXL_HAVE_BYTEARRAY_UUID 1
Yes.
Ian.
diff --git a/tools/libxl/libxl_osdeps.h b/tools/libxl/libxl_osdeps.h index 802c762..10ce703 100644 --- a/tools/libxl/libxl_osdeps.h +++ b/tools/libxl/libxl_osdeps.h @@ -30,6 +30,7 @@ #define SYSFS_PCIBACK_DRIVER "/kern/xen/pci" #define NETBACK_NIC_NAME "xvif%ui%d" #include <util.h> +#include <uuid.h> #elif defined(__OpenBSD__) #include <util.h> #elif defined(__linux__) @@ -39,6 +40,7 @@ #define SYSFS_PCIBACK_DRIVER "/sys/bus/pci/drivers/pciback" #define NETBACK_NIC_NAME "vif%u.%d" #include <pty.h> +#include <uuid/uuid.h> #elif defined(__sun__) #include <stropts.h> #elif defined(__FreeBSD__) @@ -49,6 +51,7 @@ #define NETBACK_NIC_NAME "xnb%u.%d" #include <libutil.h> #include <sys/endian.h> +#include <uuid.h> #endif #ifndef SYSFS_USBBACK_DRIVER diff --git a/tools/libxl/libxl_uuid.c b/tools/libxl/libxl_uuid.c index 7d4a032..dadb79b 100644 --- a/tools/libxl/libxl_uuid.c +++ b/tools/libxl/libxl_uuid.c @@ -64,27 +64,35 @@ uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid) int libxl_uuid_is_nil(const libxl_uuid *uuid) { uint32_t status; + uuid_t nat_uuid; - return uuid_is_nil(&uuid->uuid, &status); + uuid_dec_be(uuid->uuid, &nat_uuid); + + return uuid_is_nil(&nat_uuid, &status); } void libxl_uuid_generate(libxl_uuid *uuid) { uint32_t status; + uuid_t nat_uuid; - BUILD_BUG_ON(sizeof(libxl_uuid) != sizeof(uuid_t)); - uuid_create(&uuid->uuid, &status); + uuid_create(&nat_uuid, &status); assert(status == uuid_s_ok); + + uuid_enc_be(uuid->uuid, &nat_uuid); } #ifdef __FreeBSD__ int libxl_uuid_from_string(libxl_uuid *uuid, const char *in) { uint32_t status; + uuid_t nat_uuid; - uuid_from_string(in, &uuid->uuid, &status); + uuid_from_string(in, &nat_uuid, &status); if (status != uuid_s_ok) - return -1; + return ERROR_FAIL; + uuid_enc_be(uuid->uuid, &nat_uuid); + return 0; } #else @@ -115,8 +123,12 @@ void libxl_uuid_clear(libxl_uuid *uuid) #ifdef __FreeBSD__ int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2) { + uuid_t nat_uuid1, nat_uuid2; - return uuid_compare(&uuid1->uuid, &uuid2->uuid, NULL); + uuid_dec_be(uuid1->uuid, &nat_uuid1); + uuid_dec_be(uuid2->uuid, &nat_uuid2); + + return uuid_compare(&nat_uuid1, &nat_uuid2, NULL); } #else int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2) @@ -128,13 +140,13 @@ int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2) const uint8_t *libxl_uuid_bytearray_const(const libxl_uuid *uuid) { - return uuid->uuid_raw; + return uuid->uuid; } uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid) { - return uuid->uuid_raw; + return uuid->uuid; } #else diff --git a/tools/libxl/libxl_uuid.h b/tools/libxl/libxl_uuid.h index c5041c7..994320d 100644 --- a/tools/libxl/libxl_uuid.h +++ b/tools/libxl/libxl_uuid.h @@ -21,18 +21,19 @@ 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 LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES((arg).uuid) +typedef struct { + /* UUID as an octet stream in big-endian byte-order. */ + unsigned char uuid[16]; +} libxl_uuid; + +#if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040700 #if defined(__linux__) #include <uuid/uuid.h> #include <stdint.h> -typedef struct { - uuid_t uuid; -} libxl_uuid; - -#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES(((uint8_t *)arg.uuid)) - #elif defined(__FreeBSD__) || defined(__NetBSD__) #include <uuid.h> @@ -42,18 +43,12 @@ typedef struct { #include <stdio.h> #include <assert.h> -typedef union { - uuid_t uuid; - uint8_t uuid_raw[16]; -} libxl_uuid; - -#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES(arg.uuid_raw) - #else #error "Please update libxl_uuid.h for your OS" #endif +#endif int libxl_uuid_is_nil(const libxl_uuid *uuid); void libxl_uuid_generate(libxl_uuid *uuid);
The internals of the uuid_t struct don't match a big endian octet stream on *BSD systems, which means that it cannot be directly casted to a uint8_t[16]. In order to solve that change the type to be an unsigned char[16], which doesn't imply any other change on Linux. On *BSDs change the helpers so that the uuid is always stored as a big endian byte stream. NB: tested on FreeBSD and Linux only. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Discussed-with: Ian Jackson <Ian.Jackson@eu.citrix.com> Discussed-with: Wei Liu <wei.liu2@citrix.com> --- Cc: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- NB2: AFAICT the NetBSD version of libxl_uuid_from_string *could* be switched to the FreeBSD one (because NetBSD also has uuid_from_string), but I don't have a NetBSD box in order to test it. --- Changes since v1: - Readd a line that was deleted by error. --- tools/libxl/libxl_osdeps.h | 3 +++ tools/libxl/libxl_uuid.c | 28 ++++++++++++++++++++-------- tools/libxl/libxl_uuid.h | 21 ++++++++------------- 3 files changed, 31 insertions(+), 21 deletions(-)