[v3] libxl: replace the usage of uuid_t with a char array

Commit Message

Roger Pau Monné April 8, 2016, 2 p.m. UTC

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é <[e-mail]>
Reviewed-by: Wei Liu <[e-mail]>
Discussed-with: Ian Jackson <[e-mail]>
Discussed-with: Wei Liu <[e-mail]>
---
Cc: Ian Jackson <[e-mail]>
Cc: Wei Liu <[e-mail]>
---
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 v2:
 - Add LIBXL_HAVE_BYTEARRAY_UUID define.

Changes since v1:
 - Readd a line that was deleted by error.
---
 tools/libxl/libxl.h        |  9 +++++++++
 tools/libxl/libxl_osdeps.h |  3 +++
 tools/libxl/libxl_uuid.c   | 28 ++++++++++++++++++++--------
 tools/libxl/libxl_uuid.h   | 21 ++++++++-------------
 4 files changed, 40 insertions(+), 21 deletions(-)

Comments

Ian Jackson April 8, 2016, 2:04 p.m. UTC | #1

Roger Pau Monne writes ("[PATCH v3] libxl: replace the usage of uuid_t with a char array"):
> 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.

Queued, thanks.

Ian.

Patch

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 677e060..8935a8f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -932,6 +932,15 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
  */
 #define LIBXL_HAVE_VGA_INTERFACE_TYPE_UNKNOWN 1
 
+/*
+ * 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
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
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);