[Qemu-devel] [PATCH v3 0/9] UUID clean ups for 2.8

[Qemu-devel] [PATCH v3 0/9] UUID clean ups for 2.8

[Fam Zheng]

v3: Negative tests. [Daniel]
    Update QemuUUID type from array to struct. [Markus]
    Add qemu_uuid_unparse_strdup.
    Add qemu_uuid_convert.
    Redirect configure obsoleted option message to stderr. [Jeff]
    vdi: uuid_convert -> qemu_uuid_convert.
    Fix commit message syntax errors in vpc patch. [Jeff]
    Further cleanup around qemu_uuid.

v2: Fix the endianness wrongness. [Jeff]
    Add tests/test-uuid.c. [Denial]
    Add r-b of Stefan Weil to the VDI patch.

The facts how we use libuuid now are not particularly pleasant.

- VHDX driver depends on uuid, but is unconditionally checked in iotests 109.
  If it is not built, the test would fail, leaving no hint about that. In fact
  this even makes the feature implementation a bit ugly because it compromises
  the write protection on block 0 with image probing, silently.

- A few other libuuid users, in block/ and crypto/, use it as an optional
  dependency, and compromises certain functionalities if it is not available.

- On the other hand, there are already a few uuid function fallbacks in those
  callers, as well as some duplicated sscanf, snprintf and g_strdup_printf
  around UUID_FMT, which can be replaced with UUID parse/unparse routines.

So instead of making libuuid a hard requirement, let's simply move the
fallbacks to util and implement the random version of uuid generation, and be
done with it.

Fam

Fam Zheng (9):
  util: Add UUID API
  vhdx: Use QEMU UUID API
  vdi: Use QEMU UUID API
  vpc: Use QEMU UUID API
  crypto: Switch to QEMU UUID API
  tests: No longer dependent on CONFIG_UUID
  configure: Remove detection code for UUID
  vl: Switch qemu_uuid to QemuUUID
  tests: Add uuid tests

 arch_init.c                |  19 -----
 block/Makefile.objs        |   2 +-
 block/iscsi.c              |   2 +-
 block/vdi.c                |  73 +++++--------------
 block/vhdx-endian.c        |   3 -
 block/vhdx.c               |   9 ++-
 block/vpc.c                |  10 +--
 configure                  |  70 ++----------------
 crypto/block-luks.c        |  26 ++-----
 hw/ipmi/ipmi_bmc_sim.c     |   2 +-
 hw/nvram/fw_cfg.c          |   2 +-
 hw/ppc/spapr.c             |   7 +-
 hw/smbios/smbios.c         |  11 +--
 hw/xenpv/xen_domainbuild.c |   6 +-
 include/qemu/uuid.h        |  48 +++++++++++++
 include/sysemu/sysemu.h    |   7 +-
 qmp.c                      |  11 +--
 stubs/uuid.c               |   2 +-
 tests/Makefile.include     |   2 +
 tests/test-crypto-block.c  |   2 +-
 tests/test-uuid.c          | 173 +++++++++++++++++++++++++++++++++++++++++++++
 util/Makefile.objs         |   1 +
 util/uuid.c                |  92 ++++++++++++++++++++++++
 vl.c                       |   7 +-
 24 files changed, 377 insertions(+), 210 deletions(-)
 create mode 100644 include/qemu/uuid.h
 create mode 100644 tests/test-uuid.c
 create mode 100644 util/uuid.c

-- 
2.7.4

[Qemu-devel] [PATCH v3 1/9] util: Add UUID API

[Fam Zheng]

A number of different places across the code base use CONFIG_UUID. Some
of them are soft dependency, some are not built if libuuid is not
available, some come with dummy fallback, some throws runtime error.

It is hard to maintain, and hard to reason for users.

Since UUID is a simple standard with only a small number of operations,
it is cleaner to have a central support in libqemuutil. This patch adds
qemu_uuid_* the functions so that all uuid users in the code base can
rely on. Except for qemu_uuid_generate which is new code, all other
functions are just copy from existing fallbacks from other files.

Note that qemu_uuid_parse is moved without updating the function
signature to use QemuUUID, to keep this patch simple.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 arch_init.c             | 19 -----------
 block/iscsi.c           |  2 +-
 hw/smbios/smbios.c      |  1 +
 include/qemu/uuid.h     | 48 ++++++++++++++++++++++++++
 include/sysemu/sysemu.h |  4 ---
 qmp.c                   |  1 +
 stubs/uuid.c            |  2 +-
 util/Makefile.objs      |  1 +
 util/uuid.c             | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c                    |  1 +
 10 files changed, 145 insertions(+), 25 deletions(-)
 create mode 100644 include/qemu/uuid.h
 create mode 100644 util/uuid.c

diff --git a/arch_init.c b/arch_init.c
index fa05973..5cc58b2 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -235,25 +235,6 @@ void audio_init(void)
     }
 }
 
-int qemu_uuid_parse(const char *str, uint8_t *uuid)
-{
-    int ret;
-
-    if (strlen(str) != 36) {
-        return -1;
-    }
-
-    ret = sscanf(str, UUID_FMT, &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 (ret != 16) {
-        return -1;
-    }
-    return 0;
-}
-
 void do_acpitable_option(const QemuOpts *opts)
 {
 #ifdef TARGET_I386
diff --git a/block/iscsi.c b/block/iscsi.c
index 95ce9e1..961ac76 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -36,7 +36,7 @@
 #include "block/block_int.h"
 #include "block/scsi.h"
 #include "qemu/iov.h"
-#include "sysemu/sysemu.h"
+#include "qemu/uuid.h"
 #include "qmp-commands.h"
 #include "qapi/qmp/qstring.h"
 #include "crypto/secret.h"
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 74c7102..0705eb1 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -20,6 +20,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "qemu/uuid.h"
 #include "sysemu/cpus.h"
 #include "hw/smbios/smbios.h"
 #include "hw/loader.h"
diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
new file mode 100644
index 0000000..80a75c2
--- /dev/null
+++ b/include/qemu/uuid.h
@@ -0,0 +1,48 @@
+/*
+ *  QEMU UUID functions
+ *
+ *  Copyright 2016 Red Hat, Inc.,
+ *
+ *  Authors:
+ *   Fam Zheng <famz@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ */
+
+#ifndef QEMU_UUID_H
+#define QEMU_UUID_H
+
+#include "qemu-common.h"
+
+/* Version 4 UUID (pseudo random numbers), RFC4122 4.4. */
+
+typedef struct {
+    unsigned char data[16];
+} QemuUUID;
+
+#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
+                 "%02hhx%02hhx-%02hhx%02hhx-" \
+                 "%02hhx%02hhx-" \
+                 "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
+
+#define UUID_FMT_LEN 36
+
+#define UUID_NONE "00000000-0000-0000-0000-000000000000"
+
+void qemu_uuid_generate(QemuUUID *out);
+
+int qemu_uuid_is_null(const QemuUUID *uu);
+
+void qemu_uuid_unparse(char *out, const QemuUUID *uuid);
+
+char *qemu_uuid_unparse_strdup(const QemuUUID *uuid);
+
+int qemu_uuid_parse(const char *str, uint8_t *uuid);
+
+void qemu_uuid_convert(QemuUUID *uuid);
+
+#endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7c760..6111950 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -18,10 +18,6 @@ extern const char *bios_name;
 extern const char *qemu_name;
 extern uint8_t qemu_uuid[];
 extern bool qemu_uuid_set;
-int qemu_uuid_parse(const char *str, uint8_t *uuid);
-
-#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
-#define UUID_NONE "00000000-0000-0000-0000-000000000000"
 
 bool runstate_check(RunState state);
 void runstate_set(RunState new_state);
diff --git a/qmp.c b/qmp.c
index b6d531e..0b65ba3 100644
--- a/qmp.c
+++ b/qmp.c
@@ -18,6 +18,7 @@
 #include "qemu/cutils.h"
 #include "monitor/monitor.h"
 #include "sysemu/sysemu.h"
+#include "qemu/uuid.h"
 #include "qmp-commands.h"
 #include "sysemu/char.h"
 #include "ui/qemu-spice.h"
diff --git a/stubs/uuid.c b/stubs/uuid.c
index 92ad717..a880de8 100644
--- a/stubs/uuid.c
+++ b/stubs/uuid.c
@@ -1,6 +1,6 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "sysemu/sysemu.h"
+#include "qemu/uuid.h"
 #include "qmp-commands.h"
 
 UuidInfo *qmp_query_uuid(Error **errp)
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 96cb1e0..31bba15 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -20,6 +20,7 @@ util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
 util-obj-y += qemu-option.o qemu-progress.o
 util-obj-y += hexdump.o
 util-obj-y += crc32c.o
+util-obj-y += uuid.o
 util-obj-y += throttle.o
 util-obj-y += getauxval.o
 util-obj-y += readline.o
diff --git a/util/uuid.c b/util/uuid.c
new file mode 100644
index 0000000..80c89f0
--- /dev/null
+++ b/util/uuid.c
@@ -0,0 +1,91 @@
+/*
+ *  QEMU UUID functions
+ *
+ *  Copyright 2016 Red Hat, Inc.,
+ *
+ *  Authors:
+ *   Fam Zheng <famz@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/uuid.h"
+#include "qemu/bswap.h"
+#include <glib.h>
+
+void qemu_uuid_generate(QemuUUID *uuid)
+{
+    int i;
+    uint32_t *out = (uint32_t *)&uuid->data[0];
+
+    QEMU_BUILD_BUG_ON(sizeof(QemuUUID) != 16);
+
+    for (i = 0; i < 4; ++i) {
+        out[i] = g_random_int();
+    }
+    /* Set the two most significant bits (bits 6 and 7) of the
+      clock_seq_hi_and_reserved to zero and one, respectively. */
+    uuid->data[8] = (uuid->data[8] & 0x3f) | 0x80;
+    /* Set the four most significant bits (bits 12 through 15) of the
+      time_hi_and_version field to the 4-bit version number.
+      */
+    uuid->data[6] = (uuid->data[6] & 0xf) | 0x40;
+}
+
+int qemu_uuid_is_null(const QemuUUID *uu)
+{
+    QemuUUID null_uuid = { 0 };
+    return memcmp(uu, &null_uuid, sizeof(QemuUUID)) == 0;
+}
+
+void qemu_uuid_unparse(char *out, const QemuUUID *uuid)
+{
+    const unsigned char *uu = &uuid->data[0];
+    snprintf(out, UUID_FMT_LEN + 1, UUID_FMT,
+             uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7],
+             uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]);
+}
+
+char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
+{
+    const unsigned char *uu = &uuid->data[0];
+    return g_strdup_printf(UUID_FMT,
+                           uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6],
+                           uu[7], uu[8], uu[9], uu[10], uu[11], uu[12],
+                           uu[13], uu[14], uu[15]);
+}
+
+int qemu_uuid_parse(const char *str, uint8_t *uuid)
+{
+    int ret;
+
+    if (strlen(str) != 36) {
+        return -1;
+    }
+
+    ret = sscanf(str, UUID_FMT, &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 (ret != 16) {
+        return -1;
+    }
+    return 0;
+}
+
+/* Change UUID from CPU endian to big endian format (network byte order,
+ * standard, see RFC 4122) and vice versa.
+ */
+void qemu_uuid_convert(QemuUUID *uuid)
+{
+    be32_to_cpus((uint32_t *)&uuid->data[0]);
+    be16_to_cpus((uint16_t *)&uuid->data[4]);
+    be16_to_cpus((uint16_t *)&uuid->data[6]);
+}
diff --git a/vl.c b/vl.c
index c4eeaff..3d96684 100644
--- a/vl.c
+++ b/vl.c
@@ -25,6 +25,7 @@
 #include "qemu-version.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
+#include "qemu/uuid.h"
 
 #ifdef CONFIG_SECCOMP
 #include "sysemu/seccomp.h"
-- 
2.7.4

[Qemu-devel] [PATCH v3 2/9] vhdx: Use QEMU UUID API

[Fam Zheng]

This removes our dependency to libuuid, so that the driver can always be
built.

Similar to how we handled data plane configure options, --enable-vhdx
and --disable-vhdx are also changed to a nop with a message saying it's
obsolete.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/Makefile.objs |  2 +-
 block/vhdx-endian.c |  3 ---
 block/vhdx.c        |  9 ++++-----
 configure           | 27 +++------------------------
 4 files changed, 8 insertions(+), 33 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 2593a2f..013abb8 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -2,7 +2,7 @@ block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
-block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
+block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-y += quorum.o
 block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
 block-obj-y += block-backend.o snapshot.o qapi.o
diff --git a/block/vhdx-endian.c b/block/vhdx-endian.c
index c306b90..429d755 100644
--- a/block/vhdx-endian.c
+++ b/block/vhdx-endian.c
@@ -21,9 +21,6 @@
 #include "qemu/bswap.h"
 #include "block/vhdx.h"
 
-#include <uuid/uuid.h>
-
-
 /*
  * All the VHDX formats on disk are little endian - the following
  * are helper import/export functions to correctly convert
diff --git a/block/vhdx.c b/block/vhdx.c
index 75ef2b1..0ba2f0a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -25,8 +25,7 @@
 #include "qemu/bswap.h"
 #include "block/vhdx.h"
 #include "migration/migration.h"
-
-#include <uuid/uuid.h>
+#include "qemu/uuid.h"
 
 /* Options for VHDX creation */
 
@@ -213,11 +212,11 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size, int crc_offset)
  */
 void vhdx_guid_generate(MSGUID *guid)
 {
-    uuid_t uuid;
+    QemuUUID uuid;
     assert(guid != NULL);
 
-    uuid_generate(uuid);
-    memcpy(guid, uuid, sizeof(MSGUID));
+    qemu_uuid_generate(&uuid);
+    memcpy(guid, &uuid, sizeof(MSGUID));
 }
 
 /* Check for region overlaps inside the VHDX image */
diff --git a/configure b/configure
index f57fcc6..2ddca79 100755
--- a/configure
+++ b/configure
@@ -316,7 +316,6 @@ vte=""
 virglrenderer=""
 tpm="yes"
 libssh2=""
-vhdx=""
 numa=""
 tcmalloc="no"
 jemalloc="no"
@@ -1094,6 +1093,9 @@ for opt do
   --disable-virtio-blk-data-plane|--enable-virtio-blk-data-plane)
       echo "$0: $opt is obsolete, virtio-blk data-plane is always on" >&2
   ;;
+  --enable-vhdx|--disable-vhdx)
+      echo "$0: $opt is obsolete, VHDX driver is always built" >&2
+  ;;
   --disable-gtk) gtk="no"
   ;;
   --enable-gtk) gtk="yes"
@@ -1134,10 +1136,6 @@ for opt do
   ;;
   --enable-libssh2) libssh2="yes"
   ;;
-  --enable-vhdx) vhdx="yes"
-  ;;
-  --disable-vhdx) vhdx="no"
-  ;;
   --disable-numa) numa="no"
   ;;
   --enable-numa) numa="yes"
@@ -1376,7 +1374,6 @@ disabled with --disable-FEATURE, default is enabled if available:
   archipelago     Archipelago backend
   tpm             TPM support
   libssh2         ssh block device support
-  vhdx            support for the Microsoft VHDX image format
   numa            libnuma support
   tcmalloc        tcmalloc support
   jemalloc        jemalloc support
@@ -2685,19 +2682,6 @@ EOF
   fi
 fi
 
-if test "$vhdx" = "yes" ; then
-    if test "$uuid" = "no" ; then
-        error_exit "uuid required for VHDX support"
-    fi
-elif test "$vhdx" != "no" ; then
-    if test "$uuid" = "yes" ; then
-        vhdx=yes
-    else
-        vhdx=no
-    fi
-fi
-
-##########################################
 # xfsctl() probe, used for raw-posix
 if test "$xfs" != "no" ; then
   cat > $TMPC << EOF
@@ -4891,7 +4875,6 @@ echo "TPM support       $tpm"
 echo "libssh2 support   $libssh2"
 echo "TPM passthrough   $tpm_passthrough"
 echo "QOM debugging     $qom_cast_debug"
-echo "vhdx              $vhdx"
 echo "lzo support       $lzo"
 echo "snappy support    $snappy"
 echo "bzip2 support     $bzip2"
@@ -5412,10 +5395,6 @@ if test "$libssh2" = "yes" ; then
   echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
 fi
 
-if test "$vhdx" = "yes" ; then
-  echo "CONFIG_VHDX=y" >> $config_host_mak
-fi
-
 # USB host support
 if test "$libusb" = "yes"; then
   echo "HOST_USB=libusb legacy" >> $config_host_mak
-- 
2.7.4

[Qemu-devel] [PATCH v3 3/9] vdi: Use QEMU UUID API

[Fam Zheng]

The UUID operations we need from libuuid is fully supported by QEMU UUID
implementation. Use it, and remove the unused code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vdi.c | 73 ++++++++++++++-----------------------------------------------
 1 file changed, 17 insertions(+), 56 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 8a1cf97..9f2eb0b 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -58,14 +58,7 @@
 #include "migration/migration.h"
 #include "qemu/coroutine.h"
 #include "qemu/cutils.h"
-
-#if defined(CONFIG_UUID)
-#include <uuid/uuid.h>
-#else
-/* TODO: move uuid emulation to some central place in QEMU. */
-#include "sysemu/sysemu.h"     /* UUID_FMT */
-typedef unsigned char uuid_t[16];
-#endif
+#include "qemu/uuid.h"
 
 /* Code configuration options. */
 
@@ -140,28 +133,6 @@ typedef unsigned char uuid_t[16];
 #define VDI_DISK_SIZE_MAX        ((uint64_t)VDI_BLOCKS_IN_IMAGE_MAX * \
                                   (uint64_t)DEFAULT_CLUSTER_SIZE)
 
-#if !defined(CONFIG_UUID)
-static inline void uuid_generate(uuid_t out)
-{
-    memset(out, 0, sizeof(uuid_t));
-}
-
-static inline int uuid_is_null(const uuid_t uu)
-{
-    uuid_t null_uuid = { 0 };
-    return memcmp(uu, null_uuid, sizeof(uuid_t)) == 0;
-}
-
-# if defined(CONFIG_VDI_DEBUG)
-static inline void uuid_unparse(const uuid_t uu, char *out)
-{
-    snprintf(out, 37, UUID_FMT,
-            uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7],
-            uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]);
-}
-# endif
-#endif
-
 typedef struct {
     char text[0x40];
     uint32_t signature;
@@ -182,10 +153,10 @@ typedef struct {
     uint32_t block_extra;       /* unused here */
     uint32_t blocks_in_image;
     uint32_t blocks_allocated;
-    uuid_t uuid_image;
-    uuid_t uuid_last_snap;
-    uuid_t uuid_link;
-    uuid_t uuid_parent;
+    QemuUUID uuid_image;
+    QemuUUID uuid_last_snap;
+    QemuUUID uuid_link;
+    QemuUUID uuid_parent;
     uint64_t unused2[7];
 } QEMU_PACKED VdiHeader;
 
@@ -206,16 +177,6 @@ typedef struct {
     Error *migration_blocker;
 } BDRVVdiState;
 
-/* Change UUID from little endian (IPRT = VirtualBox format) to big endian
- * format (network byte order, standard, see RFC 4122) and vice versa.
- */
-static void uuid_convert(uuid_t uuid)
-{
-    bswap32s((uint32_t *)&uuid[0]);
-    bswap16s((uint16_t *)&uuid[4]);
-    bswap16s((uint16_t *)&uuid[6]);
-}
-
 static void vdi_header_to_cpu(VdiHeader *header)
 {
     le32_to_cpus(&header->signature);
@@ -234,10 +195,10 @@ static void vdi_header_to_cpu(VdiHeader *header)
     le32_to_cpus(&header->block_extra);
     le32_to_cpus(&header->blocks_in_image);
     le32_to_cpus(&header->blocks_allocated);
-    uuid_convert(header->uuid_image);
-    uuid_convert(header->uuid_last_snap);
-    uuid_convert(header->uuid_link);
-    uuid_convert(header->uuid_parent);
+    qemu_uuid_convert(&header->uuid_image);
+    qemu_uuid_convert(&header->uuid_last_snap);
+    qemu_uuid_convert(&header->uuid_link);
+    qemu_uuid_convert(&header->uuid_parent);
 }
 
 static void vdi_header_to_le(VdiHeader *header)
@@ -258,10 +219,10 @@ static void vdi_header_to_le(VdiHeader *header)
     cpu_to_le32s(&header->block_extra);
     cpu_to_le32s(&header->blocks_in_image);
     cpu_to_le32s(&header->blocks_allocated);
-    uuid_convert(header->uuid_image);
-    uuid_convert(header->uuid_last_snap);
-    uuid_convert(header->uuid_link);
-    uuid_convert(header->uuid_parent);
+    qemu_uuid_convert(&header->uuid_image);
+    qemu_uuid_convert(&header->uuid_last_snap);
+    qemu_uuid_convert(&header->uuid_link);
+    qemu_uuid_convert(&header->uuid_parent);
 }
 
 #if defined(CONFIG_VDI_DEBUG)
@@ -469,11 +430,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                    (uint64_t)header.blocks_in_image * header.block_size);
         ret = -ENOTSUP;
         goto fail;
-    } else if (!uuid_is_null(header.uuid_link)) {
+    } else if (!qemu_uuid_is_null(&header.uuid_link)) {
         error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
         ret = -ENOTSUP;
         goto fail;
-    } else if (!uuid_is_null(header.uuid_parent)) {
+    } else if (!qemu_uuid_is_null(&header.uuid_parent)) {
         error_setg(errp, "unsupported VDI image (non-NULL parent UUID)");
         ret = -ENOTSUP;
         goto fail;
@@ -821,8 +782,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
     if (image_type == VDI_TYPE_STATIC) {
         header.blocks_allocated = blocks;
     }
-    uuid_generate(header.uuid_image);
-    uuid_generate(header.uuid_last_snap);
+    qemu_uuid_generate(&header.uuid_image);
+    qemu_uuid_generate(&header.uuid_last_snap);
     /* There is no need to set header.uuid_link or header.uuid_parent here. */
 #if defined(CONFIG_VDI_DEBUG)
     vdi_header_print(&header);
-- 
2.7.4

[Qemu-devel] [PATCH v3 4/9] vpc: Use QEMU UUID API

[Fam Zheng]

Previously we conditionally generated footer->uuid, when libuuid was
available. Now that we have a built-in implementation, we can switch to
it.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vpc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 43707ed..8d5886f 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -30,9 +30,7 @@
 #include "qemu/module.h"
 #include "migration/migration.h"
 #include "qemu/bswap.h"
-#if defined(CONFIG_UUID)
-#include <uuid/uuid.h>
-#endif
+#include "qemu/uuid.h"
 
 /**************************************************************/
 
@@ -89,7 +87,7 @@ typedef struct vhd_footer {
     uint32_t    checksum;
 
     /* UUID used to identify a parent hard disk (backing file) */
-    uint8_t     uuid[16];
+    QemuUUID    uuid;
 
     uint8_t     in_saved_state;
 } QEMU_PACKED VHDFooter;
@@ -980,9 +978,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
 
     footer->type = cpu_to_be32(disk_type);
 
-#if defined(CONFIG_UUID)
-    uuid_generate(footer->uuid);
-#endif
+    qemu_uuid_generate(&footer->uuid);
 
     footer->checksum = cpu_to_be32(vpc_checksum(buf, HEADER_SIZE));
 
-- 
2.7.4

[Qemu-devel] [PATCH v3 5/9] crypto: Switch to QEMU UUID API

[Fam Zheng]

The uuid generation doesn't return error, so update the function
signature and calling code accordingly.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 crypto/block-luks.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index aba4455..7ad6bdc 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -29,10 +29,7 @@
 #include "crypto/pbkdf.h"
 #include "crypto/secret.h"
 #include "crypto/random.h"
-
-#ifdef CONFIG_UUID
-#include <uuid/uuid.h>
-#endif
+#include "qemu/uuid.h"
 
 #include "qemu/coroutine.h"
 
@@ -877,18 +874,12 @@ qcrypto_block_luks_open(QCryptoBlock *block,
 }
 
 
-static int
-qcrypto_block_luks_uuid_gen(uint8_t *uuidstr, Error **errp)
+static void
+qcrypto_block_luks_uuid_gen(uint8_t *uuidstr)
 {
-#ifdef CONFIG_UUID
-    uuid_t uuid;
-    uuid_generate(uuid);
-    uuid_unparse(uuid, (char *)uuidstr);
-    return 0;
-#else
-    error_setg(errp, "Unable to generate uuids on this platform");
-    return -1;
-#endif
+    QemuUUID uuid;
+    qemu_uuid_generate(&uuid);
+    qemu_uuid_unparse((char *)uuidstr, &uuid);
 }
 
 static int
@@ -961,10 +952,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
      * it out to disk
      */
     luks->header.version = QCRYPTO_BLOCK_LUKS_VERSION;
-    if (qcrypto_block_luks_uuid_gen(luks->header.uuid,
-                                    errp) < 0) {
-        goto error;
-    }
+    qcrypto_block_luks_uuid_gen(luks->header.uuid);
 
     cipher_alg = qcrypto_block_luks_cipher_alg_lookup(luks_opts.cipher_alg,
                                                       errp);
-- 
2.7.4

[Qemu-devel] [PATCH v3 6/9] tests: No longer dependent on CONFIG_UUID

[Fam Zheng]

crypto now uses built-in uuid implementation, so this check is not
needed.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/test-crypto-block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-crypto-block.c b/tests/test-crypto-block.c
index a38110d..1957a86 100644
--- a/tests/test-crypto-block.c
+++ b/tests/test-crypto-block.c
@@ -28,7 +28,7 @@
 #include <sys/resource.h>
 #endif
 
-#if defined(CONFIG_UUID) && (defined(_WIN32) || defined RUSAGE_THREAD)
+#if (defined(_WIN32) || defined RUSAGE_THREAD)
 #define TEST_LUKS
 #else
 #undef TEST_LUKS
-- 
2.7.4

[Qemu-devel] [PATCH v3 7/9] configure: Remove detection code for UUID

[Fam Zheng]

All code now uses built-in UUID implementation. Remove the code of
libuuid and make --enable-uuid and --disable-uuid only print a message.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 configure | 43 ++++---------------------------------------
 1 file changed, 4 insertions(+), 39 deletions(-)

diff --git a/configure b/configure
index 2ddca79..5f63111 100755
--- a/configure
+++ b/configure
@@ -212,7 +212,6 @@ sdlabi=""
 virtfs=""
 vnc="yes"
 sparse="no"
-uuid=""
 vde=""
 vnc_sasl=""
 vnc_jpeg=""
@@ -881,10 +880,6 @@ for opt do
   ;;
   --disable-slirp) slirp="no"
   ;;
-  --disable-uuid) uuid="no"
-  ;;
-  --enable-uuid) uuid="yes"
-  ;;
   --disable-vde) vde="no"
   ;;
   --enable-vde) vde="yes"
@@ -1096,6 +1091,9 @@ for opt do
   --enable-vhdx|--disable-vhdx)
       echo "$0: $opt is obsolete, VHDX driver is always built" >&2
   ;;
+  --enable-uuid|--disable-uuid)
+      echo "$0: $opt is obsolete, UUID support is always built" >&2
+  ;;
   --disable-gtk) gtk="no"
   ;;
   --enable-gtk) gtk="yes"
@@ -1350,7 +1348,6 @@ disabled with --disable-FEATURE, default is enabled if available:
   bluez           bluez stack connectivity
   kvm             KVM acceleration support
   rdma            RDMA-based migration support
-  uuid            uuid support
   vde             support for vde network
   netmap          support for netmap network
   linux-aio       Linux AIO support
@@ -2654,34 +2651,6 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
-# uuid_generate() probe, used for vdi block driver
-# Note that on some systems (notably MacOSX) no extra library
-# need be linked to get the uuid functions.
-if test "$uuid" != "no" ; then
-  uuid_libs="-luuid"
-  cat > $TMPC << EOF
-#include <uuid/uuid.h>
-int main(void)
-{
-    uuid_t my_uuid;
-    uuid_generate(my_uuid);
-    return 0;
-}
-EOF
-  if compile_prog "" "" ; then
-    uuid="yes"
-  elif compile_prog "" "$uuid_libs" ; then
-    uuid="yes"
-    libs_softmmu="$uuid_libs $libs_softmmu"
-    libs_tools="$uuid_libs $libs_tools"
-  else
-    if test "$uuid" = "yes" ; then
-      feature_not_found "uuid" "Install libuuid devel"
-    fi
-    uuid=no
-  fi
-fi
-
 # xfsctl() probe, used for raw-posix
 if test "$xfs" != "no" ; then
   cat > $TMPC << EOF
@@ -4054,7 +4023,7 @@ EOF
   if compile_prog "$vss_win32_include" "" ; then
     guest_agent_with_vss="yes"
     QEMU_CFLAGS="$QEMU_CFLAGS $vss_win32_include"
-    libs_qga="-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ -Wl,--enable-stdcall-fixup $libs_qga"
+    libs_qga="-lole32 -loleaut32 -lshlwapi -lstdc++ -Wl,--enable-stdcall-fixup $libs_qga"
     qga_vss_provider="qga/vss-win32/qga-vss.dll qga/vss-win32/qga-vss.tlb"
   else
     if test "$vss_win32_sdk" != "" ; then
@@ -4842,7 +4811,6 @@ echo "preadv support    $preadv"
 echo "fdatasync         $fdatasync"
 echo "madvise           $madvise"
 echo "posix_madvise     $posix_madvise"
-echo "uuid support      $uuid"
 echo "libcap-ng support $cap_ng"
 echo "vhost-net support $vhost_net"
 echo "vhost-scsi support $vhost_scsi"
@@ -5030,9 +4998,6 @@ fi
 if test "$fnmatch" = "yes" ; then
   echo "CONFIG_FNMATCH=y" >> $config_host_mak
 fi
-if test "$uuid" = "yes" ; then
-  echo "CONFIG_UUID=y" >> $config_host_mak
-fi
 if test "$xfs" = "yes" ; then
   echo "CONFIG_XFS=y" >> $config_host_mak
 fi
-- 
2.7.4

[Qemu-devel] [PATCH v3 8/9] vl: Switch qemu_uuid to QemuUUID

[Fam Zheng]

Update all qemu_uuid users as well, especially get rid of the duplicated
low level g_strdup_printf, sscanf and snprintf calls with QEMU UUID API.

Since qemu_uuid_parse is quite tangled with qemu_uuid, it's switching to
QemuUUID is done here too to keep everything in sync and avoid code
churn.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/ipmi/ipmi_bmc_sim.c     |  2 +-
 hw/nvram/fw_cfg.c          |  2 +-
 hw/ppc/spapr.c             |  7 +------
 hw/smbios/smbios.c         | 10 +++++-----
 hw/xenpv/xen_domainbuild.c |  6 +-----
 include/qemu/uuid.h        |  2 +-
 include/sysemu/sysemu.h    |  3 ++-
 qmp.c                      | 10 ++--------
 util/uuid.c                | 11 ++++++-----
 vl.c                       |  6 +++---
 10 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index dc9c14c..17c7c0e 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -1773,7 +1773,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp)
     ibs->acpi_power_state[1] = 0;
 
     if (qemu_uuid_set) {
-        memcpy(&ibs->uuid, qemu_uuid, 16);
+        memcpy(&ibs->uuid, &qemu_uuid, 16);
     } else {
         memset(&ibs->uuid, 0, 16);
     }
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 6a68e59..0118299 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -883,7 +883,7 @@ static void fw_cfg_init1(DeviceState *dev)
     qdev_init_nofail(dev);
 
     fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
-    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
+    fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics);
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 57564e5..1eef001 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -375,12 +375,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
         g_free(buf);
     }
 
-    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
-                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
-                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
-                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
-                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
-                          qemu_uuid[14], qemu_uuid[15]);
+    buf = qemu_uuid_unparse_strdup(&qemu_uuid);
 
     _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
     if (qemu_uuid_set) {
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 0705eb1..595f1f2 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -409,7 +409,7 @@ static void smbios_build_type_1_fields(void)
          * BIOS.
          */
         smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
-                         qemu_uuid, 16);
+                         &qemu_uuid, 16);
     }
 }
 
@@ -484,9 +484,9 @@ static void smbios_build_type_0_table(void)
 /* Encode UUID from the big endian encoding described on RFC4122 to the wire
  * format specified by SMBIOS version 2.6.
  */
-static void smbios_encode_uuid(struct smbios_uuid *uuid, const uint8_t *buf)
+static void smbios_encode_uuid(struct smbios_uuid *uuid, QemuUUID *in)
 {
-    memcpy(uuid, buf, 16);
+    memcpy(uuid, &in, 16);
     if (smbios_uuid_encoded) {
         uuid->time_low = bswap32(uuid->time_low);
         uuid->time_mid = bswap16(uuid->time_mid);
@@ -503,7 +503,7 @@ static void smbios_build_type_1_table(void)
     SMBIOS_TABLE_SET_STR(1, version_str, type1.version);
     SMBIOS_TABLE_SET_STR(1, serial_number_str, type1.serial);
     if (qemu_uuid_set) {
-        smbios_encode_uuid(&t->uuid, qemu_uuid);
+        smbios_encode_uuid(&t->uuid, &qemu_uuid);
     } else {
         memset(&t->uuid, 0, 16);
     }
@@ -1002,7 +1002,7 @@ void smbios_entry_add(QemuOpts *opts)
 
             val = qemu_opt_get(opts, "uuid");
             if (val) {
-                if (qemu_uuid_parse(val, qemu_uuid) != 0) {
+                if (qemu_uuid_parse(&qemu_uuid, val) != 0) {
                     error_report("Invalid UUID");
                     exit(1);
                 }
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index 5a9f5ac..deacc5b 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -53,11 +53,7 @@ int xenstore_domain_init1(const char *kernel, const char *ramdisk,
     char *dom, uuid_string[42], vm[256], path[256];
     int i;
 
-    snprintf(uuid_string, sizeof(uuid_string), UUID_FMT,
-             qemu_uuid[0], qemu_uuid[1], qemu_uuid[2], qemu_uuid[3],
-             qemu_uuid[4], qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
-             qemu_uuid[8], qemu_uuid[9], qemu_uuid[10], qemu_uuid[11],
-             qemu_uuid[12], qemu_uuid[13], qemu_uuid[14], qemu_uuid[15]);
+    qemu_uuid_unparse(uuid_string, &qemu_uuid);
     dom = xs_get_domain_path(xenstore, xen_domid);
     snprintf(vm,  sizeof(vm),  "/vm/%s", uuid_string);
 
diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
index 80a75c2..3f51bb8 100644
--- a/include/qemu/uuid.h
+++ b/include/qemu/uuid.h
@@ -41,7 +41,7 @@ void qemu_uuid_unparse(char *out, const QemuUUID *uuid);
 
 char *qemu_uuid_unparse_strdup(const QemuUUID *uuid);
 
-int qemu_uuid_parse(const char *str, uint8_t *uuid);
+int qemu_uuid_parse(QemuUUID *uuid, const char *str);
 
 void qemu_uuid_convert(QemuUUID *uuid);
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6111950..ef2c50b 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -9,6 +9,7 @@
 #include "qemu/notify.h"
 #include "qemu/main-loop.h"
 #include "qemu/bitmap.h"
+#include "qemu/uuid.h"
 #include "qom/object.h"
 
 /* vl.c */
@@ -16,7 +17,7 @@
 extern const char *bios_name;
 
 extern const char *qemu_name;
-extern uint8_t qemu_uuid[];
+extern QemuUUID qemu_uuid;
 extern bool qemu_uuid_set;
 
 bool runstate_check(RunState state);
diff --git a/qmp.c b/qmp.c
index 0b65ba3..c579a7f 100644
--- a/qmp.c
+++ b/qmp.c
@@ -36,6 +36,7 @@
 #include "qom/object_interfaces.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/acpi/acpi_dev_interface.h"
+#include "qemu/uuid.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -85,15 +86,8 @@ KvmInfo *qmp_query_kvm(Error **errp)
 UuidInfo *qmp_query_uuid(Error **errp)
 {
     UuidInfo *info = g_malloc0(sizeof(*info));
-    char uuid[64];
 
-    snprintf(uuid, sizeof(uuid), UUID_FMT, qemu_uuid[0], qemu_uuid[1],
-                   qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
-                   qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
-                   qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
-                   qemu_uuid[14], qemu_uuid[15]);
-
-    info->UUID = g_strdup(uuid);
+    info->UUID = qemu_uuid_unparse_strdup(&qemu_uuid);
     return info;
 }
 
diff --git a/util/uuid.c b/util/uuid.c
index 80c89f0..5d8bc88 100644
--- a/util/uuid.c
+++ b/util/uuid.c
@@ -61,18 +61,19 @@ char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
                            uu[13], uu[14], uu[15]);
 }
 
-int qemu_uuid_parse(const char *str, uint8_t *uuid)
+int qemu_uuid_parse(QemuUUID *uuid, const char *str)
 {
+    unsigned char *uu = &uuid->data[0];
     int ret;
 
     if (strlen(str) != 36) {
         return -1;
     }
 
-    ret = sscanf(str, UUID_FMT, &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]);
+    ret = sscanf(str, UUID_FMT, &uu[0], &uu[1], &uu[2], &uu[3],
+                 &uu[4], &uu[5], &uu[6], &uu[7], &uu[8], &uu[9],
+                 &uu[10], &uu[11], &uu[12], &uu[13], &uu[14],
+                 &uu[15]);
 
     if (ret != 16) {
         return -1;
diff --git a/vl.c b/vl.c
index 3d96684..9108482 100644
--- a/vl.c
+++ b/vl.c
@@ -182,10 +182,10 @@ uint8_t qemu_extra_params_fw[2];
 
 int icount_align_option;
 
-/* The bytes in qemu_uuid[] are in the order specified by RFC4122, _not_ in the
+/* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the
  * little-endian "wire format" described in the SMBIOS 2.6 specification.
  */
-uint8_t qemu_uuid[16];
+QemuUUID qemu_uuid;
 bool qemu_uuid_set;
 
 static NotifierList exit_notifiers =
@@ -3724,7 +3724,7 @@ int main(int argc, char **argv, char **envp)
                 cursor_hide = 0;
                 break;
             case QEMU_OPTION_uuid:
-                if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
+                if (qemu_uuid_parse(&qemu_uuid, optarg) < 0) {
                     error_report("failed to parse UUID string: wrong format");
                     exit(1);
                 }
-- 
2.7.4

[Qemu-devel] [PATCH v3 9/9] tests: Add uuid tests

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/Makefile.include |   2 +
 tests/test-uuid.c      | 173 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 175 insertions(+)
 create mode 100644 tests/test-uuid.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..aa4f832 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -112,6 +112,7 @@ check-unit-y += tests/test-crypto-xts$(EXESUF)
 check-unit-y += tests/test-crypto-block$(EXESUF)
 gcov-files-test-logging-y = tests/test-logging.c
 check-unit-y += tests/test-logging$(EXESUF)
+check-unit-y += tests/test-uuid$(EXESUF)
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -631,6 +632,7 @@ tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
 tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
 tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
+tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
 	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"  LINK  $(TARGET_DIR)$@")
diff --git a/tests/test-uuid.c b/tests/test-uuid.c
new file mode 100644
index 0000000..0a0f08d
--- /dev/null
+++ b/tests/test-uuid.c
@@ -0,0 +1,173 @@
+/*
+ * QEMU UUID Library
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * This library 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; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/uuid.h"
+
+struct {
+    const char *uuidstr;
+    QemuUUID uuid;
+    bool uuidstr_is_valid;
+    bool check_unparse;
+} uuid_test_data[] = {
+    {    /* Normal */
+        "586ece27-7f09-41e0-9e74-e901317e9d42",
+        { {
+             0x58, 0x6e, 0xce, 0x27, 0x7f, 0x09, 0x41, 0xe0,
+             0x9e, 0x74, 0xe9, 0x01, 0x31, 0x7e, 0x9d, 0x42,
+        } },
+        true, true,
+    }, { /* NULL */
+        "00000000-0000-0000-0000-000000000000",
+        { { 0 } },
+        true, true,
+    }, { /* Upper case */
+        "0CC6C752-3961-4028-A286-C05CC616D396",
+        { {
+             0x0c, 0xc6, 0xc7, 0x52, 0x39, 0x61, 0x40, 0x28,
+             0xa2, 0x86, 0xc0, 0x5c, 0xc6, 0x16, 0xd3, 0x96,
+        } },
+        true, false,
+    }, { /* Mixed case */
+        "0CC6C752-3961-4028-a286-c05cc616D396",
+        { {
+             0x0c, 0xc6, 0xc7, 0x52, 0x39, 0x61, 0x40, 0x28,
+             0xa2, 0x86, 0xc0, 0x5c, 0xc6, 0x16, 0xd3, 0x96,
+        } },
+        true, false,
+    }, { /* Empty */
+        ""
+    }, { /* Too short */
+        "abc",
+    }, { /* Non-hex */
+        "abcdefgh-0000-0000-0000-000000000000",
+    }, { /* No '-' */
+        "0cc6c75239614028a286c05cc616d396",
+    }, { /* '-' in wrong position */
+        "0cc6c-7523961-4028-a286-c05cc616d396",
+    }, { /* Double '-' */
+        "0cc6c752--3961-4028-a286-c05cc616d396",
+    }, { /* Too long */
+        "0000000000000000000000000000000000000000000000",
+    }, { /* Invalid char in the beginning */
+        ")cc6c752--3961-4028-a286-c05cc616d396",
+    }, { /* Invalid char in the beginning, in extra */
+        ")0cc6c752--3961-4028-a286-c05cc616d396",
+    }, { /* Invalid char in the middle */
+        "0cc6c752--39*1-4028-a286-c05cc616d396",
+    }, { /* Invalid char in the middle, in extra */
+        "0cc6c752--39*61-4028-a286-c05cc616d396",
+    }, { /* Invalid char in the end */
+        "0cc6c752--3961-4028-a286-c05cc616d39&",
+    }, { /* Invalid char in the end, in extra */
+        "0cc6c752--3961-4028-a286-c05cc616d396&",
+    },
+};
+
+static inline bool uuid_is_valid(QemuUUID *uuid)
+{
+    return qemu_uuid_is_null(uuid) ||
+            ((uuid->data[6] & 0xf0) == 0x40 && (uuid->data[8] & 0xc0) == 0x80);
+}
+
+static void test_uuid_generate(void)
+{
+    QemuUUID uuid;
+    int i;
+
+    for (i = 0; i < 100; ++i) {
+        qemu_uuid_generate(&uuid);
+        g_assert(uuid_is_valid(&uuid));
+    }
+}
+
+static void test_uuid_is_null(void)
+{
+    QemuUUID uuid_null = { };
+    QemuUUID uuid_not_null = { {
+        0x58, 0x6e, 0xce, 0x27, 0x7f, 0x09, 0x41, 0xe0,
+        0x9e, 0x74, 0xe9, 0x01, 0x31, 0x7e, 0x9d, 0x42
+    } };
+    QemuUUID uuid_not_null_2 = { { 1 } };
+
+    g_assert(qemu_uuid_is_null(&uuid_null));
+    g_assert_false(qemu_uuid_is_null(&uuid_not_null));
+    g_assert_false(qemu_uuid_is_null(&uuid_not_null_2));
+}
+
+static void test_uuid_parse(void)
+{
+    int i, r;
+
+    for (i = 0; i < ARRAY_SIZE(uuid_test_data); i++) {
+        QemuUUID uuid;
+        bool is_valid = uuid_test_data[i].uuidstr_is_valid;
+
+        r = qemu_uuid_parse(&uuid, uuid_test_data[i].uuidstr);
+        g_assert_cmpint(!r, ==, is_valid);
+        if (is_valid) {
+            g_assert_cmpint(is_valid, ==, uuid_is_valid(&uuid));
+            g_assert_cmpmem(&uuid_test_data[i].uuid, sizeof(uuid),
+                            &uuid, sizeof(uuid));
+        }
+    }
+}
+
+static void test_uuid_unparse(void)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(uuid_test_data); i++) {
+        char out[37];
+
+        if (!uuid_test_data[i].check_unparse) {
+            continue;
+        }
+        qemu_uuid_unparse(out, &uuid_test_data[i].uuid);
+        g_assert_cmpstr(uuid_test_data[i].uuidstr, ==, out);
+    }
+}
+
+static void test_uuid_unparse_strdup(void)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(uuid_test_data); i++) {
+        char *out;
+
+        if (!uuid_test_data[i].check_unparse) {
+            continue;
+        }
+        out = qemu_uuid_unparse_strdup(&uuid_test_data[i].uuid);
+        g_assert_cmpstr(uuid_test_data[i].uuidstr, ==, out);
+    }
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    g_test_add_func("/uuid/generate", test_uuid_generate);
+    g_test_add_func("/uuid/is_null", test_uuid_is_null);
+    g_test_add_func("/uuid/parse", test_uuid_parse);
+    g_test_add_func("/uuid/unparse", test_uuid_unparse);
+    g_test_add_func("/uuid/unparse_strdup", test_uuid_unparse_strdup);
+
+    return g_test_run();
+}
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 1/9] util: Add UUID API

[Jeff Cody]

On Tue, Aug 09, 2016 at 02:49:59PM +0800, Fam Zheng wrote:
> A number of different places across the code base use CONFIG_UUID. Some
> of them are soft dependency, some are not built if libuuid is not
> available, some come with dummy fallback, some throws runtime error.
> 
> It is hard to maintain, and hard to reason for users.
> 
> Since UUID is a simple standard with only a small number of operations,
> it is cleaner to have a central support in libqemuutil. This patch adds
> qemu_uuid_* the functions so that all uuid users in the code base can
> rely on. Except for qemu_uuid_generate which is new code, all other
> functions are just copy from existing fallbacks from other files.
> 
> Note that qemu_uuid_parse is moved without updating the function
> signature to use QemuUUID, to keep this patch simple.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  arch_init.c             | 19 -----------
>  block/iscsi.c           |  2 +-
>  hw/smbios/smbios.c      |  1 +
>  include/qemu/uuid.h     | 48 ++++++++++++++++++++++++++
>  include/sysemu/sysemu.h |  4 ---
>  qmp.c                   |  1 +
>  stubs/uuid.c            |  2 +-
>  util/Makefile.objs      |  1 +
>  util/uuid.c             | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  vl.c                    |  1 +
>  10 files changed, 145 insertions(+), 25 deletions(-)
>  create mode 100644 include/qemu/uuid.h
>  create mode 100644 util/uuid.c
> 
> diff --git a/arch_init.c b/arch_init.c
> index fa05973..5cc58b2 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -235,25 +235,6 @@ void audio_init(void)
>      }
>  }
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> -{
> -    int ret;
> -
> -    if (strlen(str) != 36) {
> -        return -1;
> -    }
> -
> -    ret = sscanf(str, UUID_FMT, &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 (ret != 16) {
> -        return -1;
> -    }
> -    return 0;
> -}
> -
>  void do_acpitable_option(const QemuOpts *opts)
>  {
>  #ifdef TARGET_I386
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 95ce9e1..961ac76 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -36,7 +36,7 @@
>  #include "block/block_int.h"
>  #include "block/scsi.h"
>  #include "qemu/iov.h"
> -#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  #include "qapi/qmp/qstring.h"
>  #include "crypto/secret.h"
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 74c7102..0705eb1 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -20,6 +20,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "sysemu/cpus.h"
>  #include "hw/smbios/smbios.h"
>  #include "hw/loader.h"
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> new file mode 100644
> index 0000000..80a75c2
> --- /dev/null
> +++ b/include/qemu/uuid.h
> @@ -0,0 +1,48 @@
> +/*
> + *  QEMU UUID functions
> + *
> + *  Copyright 2016 Red Hat, Inc.,
> + *
> + *  Authors:
> + *   Fam Zheng <famz@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#ifndef QEMU_UUID_H
> +#define QEMU_UUID_H
> +
> +#include "qemu-common.h"
> +
> +/* Version 4 UUID (pseudo random numbers), RFC4122 4.4. */
> +
> +typedef struct {
> +    unsigned char data[16];
> +} QemuUUID;
> +
> +#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
> +                 "%02hhx%02hhx-%02hhx%02hhx-" \
> +                 "%02hhx%02hhx-" \
> +                 "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +
> +#define UUID_FMT_LEN 36
> +
> +#define UUID_NONE "00000000-0000-0000-0000-000000000000"
> +
> +void qemu_uuid_generate(QemuUUID *out);
> +
> +int qemu_uuid_is_null(const QemuUUID *uu);
> +
> +void qemu_uuid_unparse(char *out, const QemuUUID *uuid);
> +
> +char *qemu_uuid_unparse_strdup(const QemuUUID *uuid);
> +
> +int qemu_uuid_parse(const char *str, uint8_t *uuid);
> +
> +void qemu_uuid_convert(QemuUUID *uuid);
> +
> +#endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ee7c760..6111950 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -18,10 +18,6 @@ extern const char *bios_name;
>  extern const char *qemu_name;
>  extern uint8_t qemu_uuid[];
>  extern bool qemu_uuid_set;
> -int qemu_uuid_parse(const char *str, uint8_t *uuid);
> -
> -#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> -#define UUID_NONE "00000000-0000-0000-0000-000000000000"
>  
>  bool runstate_check(RunState state);
>  void runstate_set(RunState new_state);
> diff --git a/qmp.c b/qmp.c
> index b6d531e..0b65ba3 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -18,6 +18,7 @@
>  #include "qemu/cutils.h"
>  #include "monitor/monitor.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  #include "sysemu/char.h"
>  #include "ui/qemu-spice.h"
> diff --git a/stubs/uuid.c b/stubs/uuid.c
> index 92ad717..a880de8 100644
> --- a/stubs/uuid.c
> +++ b/stubs/uuid.c
> @@ -1,6 +1,6 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
> -#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  
>  UuidInfo *qmp_query_uuid(Error **errp)
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 96cb1e0..31bba15 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -20,6 +20,7 @@ util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
>  util-obj-y += qemu-option.o qemu-progress.o
>  util-obj-y += hexdump.o
>  util-obj-y += crc32c.o
> +util-obj-y += uuid.o
>  util-obj-y += throttle.o
>  util-obj-y += getauxval.o
>  util-obj-y += readline.o
> diff --git a/util/uuid.c b/util/uuid.c
> new file mode 100644
> index 0000000..80c89f0
> --- /dev/null
> +++ b/util/uuid.c
> @@ -0,0 +1,91 @@
> +/*
> + *  QEMU UUID functions
> + *
> + *  Copyright 2016 Red Hat, Inc.,
> + *
> + *  Authors:
> + *   Fam Zheng <famz@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/uuid.h"
> +#include "qemu/bswap.h"
> +#include <glib.h>
> +
> +void qemu_uuid_generate(QemuUUID *uuid)
> +{
> +    int i;
> +    uint32_t *out = (uint32_t *)&uuid->data[0];
> +
> +    QEMU_BUILD_BUG_ON(sizeof(QemuUUID) != 16);
> +
> +    for (i = 0; i < 4; ++i) {
> +        out[i] = g_random_int();
> +    }
> +    /* Set the two most significant bits (bits 6 and 7) of the
> +      clock_seq_hi_and_reserved to zero and one, respectively. */
> +    uuid->data[8] = (uuid->data[8] & 0x3f) | 0x80;
> +    /* Set the four most significant bits (bits 12 through 15) of the
> +      time_hi_and_version field to the 4-bit version number.
> +      */
> +    uuid->data[6] = (uuid->data[6] & 0xf) | 0x40;
> +}
> +
> +int qemu_uuid_is_null(const QemuUUID *uu)
> +{
> +    QemuUUID null_uuid = { 0 };
> +    return memcmp(uu, &null_uuid, sizeof(QemuUUID)) == 0;
> +}
> +
> +void qemu_uuid_unparse(char *out, const QemuUUID *uuid)
> +{
> +    const unsigned char *uu = &uuid->data[0];
> +    snprintf(out, UUID_FMT_LEN + 1, UUID_FMT,
> +             uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7],
> +             uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]);
> +}
> +
> +char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
> +{
> +    const unsigned char *uu = &uuid->data[0];
> +    return g_strdup_printf(UUID_FMT,
> +                           uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6],
> +                           uu[7], uu[8], uu[9], uu[10], uu[11], uu[12],
> +                           uu[13], uu[14], uu[15]);
> +}
> +
> +int qemu_uuid_parse(const char *str, uint8_t *uuid)
> +{
> +    int ret;
> +
> +    if (strlen(str) != 36) {
> +        return -1;
> +    }
> +
> +    ret = sscanf(str, UUID_FMT, &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 (ret != 16) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +


> +/* Change UUID from CPU endian to big endian format (network byte order,
> + * standard, see RFC 4122) and vice versa.
> + */
> +void qemu_uuid_convert(QemuUUID *uuid)
> +{
> +    be32_to_cpus((uint32_t *)&uuid->data[0]);
> +    be16_to_cpus((uint16_t *)&uuid->data[4]);
> +    be16_to_cpus((uint16_t *)&uuid->data[6]);
> +}

Hmm... what's this function for?  The comment says it converts from CPU
endian to BE, but it appears to do the opposite (BE to CPU).

The new qemu_uuid_convert() replaces VDI's uuid_convert() in patch 3, but
that function does something different - it does blind bswaps.  This works
because the driver knows the UUID is always stored in LE format in the file
header, and converts UUID to the RFC BE format.  And since the internal
representation is known to always be in BE format, it can do a blanket bswap
again with the same function to write it back out to the header.

On a BE machine, qemu_uuid_convert() won't do what the VDI driver needs
(be??_to_cpus() is a noop).

So the short of it:  I think this new function should be explicitly called
something like "qemu_uuid_bswap()", and do blind bswaps.  That makes it a
drop in replacement for VDI.


...


Now for some odd stuff, but not directly dealing with your patch series:

This made me double check the VHDX driver, and how it handles the UUIDs.
The VHDX spec declares that everything is stored in LE format.  There
are some "known" UUIDs for metadata IDs defined in the spec, and when I
check those in our qemu-created VHDX files against MS-created VHDX files,
they are indeed both stored LE as expected, per spec:

Known UUID: 8141BF1D-A96F-4709-BA47-F233A8FAAB5F
Stored in file header as:

00300080  1d bf 41 81 6f a9 09 47  ba 47 f2 33 a8 fa ab 5f  |..A.o..G.G.3..._|
                               ^^
                               octet 6, time_hi_version

Yay!

However, thinking through this made me realize there is a bug in the VHDX
driver (my fault, too).  When we _generate_ a new UUID for the "head"
header, the uuid_generate() call will store it internally in BE format.  So
technically, we should be doing what VDI does, and bswap'ing UUIDs we
generate when we write them out; but instead, we are using cpu_to_le(),
which does nothing on a LE machine.  Looking at a qemu created VHDX file, we
can confirm that we are storing our generated header UUIDs incorrectly in
BE format, contrary to the VHDX spec:

00010010  82 8a b6 52 53 3d 48 e6  aa 33 5c 64 9e fe 41 4b  |...RS=H..3\d..AK|
                            ^^^^^
                            (These bytes should be swapped in the vhdx file)


This is where it gets weird - checking the MS Hyper-V created images we have
in sample_images, it appears MS _also_ stores those "head" header UUIDs
incorrectly. The only correctly stored LE UUIDs are the "known" metadata
UUIDs... so they have the same bug as our VHDX driver.  Yay?

MS "head" UUID stored incorrectly by Hyper-V:
===================================================
00010010  6d ea 90 8e 36 b6 49 1c  b7 d4 35 10 9e 60 0c 0c  |m...6.I...5..`..|
                            ^^^^^
                            (These bytes should be swapped in the vhdx file)

I would say "meh", as our goal is to be compatible in the real world for 3rd
party formats, rather than be strictly beholden to the spec.  However, no
celebration yet; MS's tool disk2vhd (to virtualize a physical disk) does
_not_ have that bug! Our sample image test-disk2vhd.vhdx.bz2 shows the
generated UUIDs in the "head" header in the VHDX spec-correct LE format:

Two MS "head" UUIDs stored correctly by disk2vhd:
===================================================
00020010  13 2b 30 81 aa c7 cd 47  8f 27 96 d4 c4 6b f8 ea  |.+0....G.'...k..|
                               ^^
00020020  1c 89 03 fd e5 29 d6 4a  8e e1 71 98 d3 b1 e2 63  |.....).J..q....c|
                               ^^
                               (correct LE position of RFC4122 octet 6)

So, I guess we should fix VHDX at some point, but it doesn't have to be in
this series, since the patches don't affect this VHDX behavior.


-Jeff

> diff --git a/vl.c b/vl.c
> index c4eeaff..3d96684 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -25,6 +25,7 @@
>  #include "qemu-version.h"
>  #include "qemu/cutils.h"
>  #include "qemu/help_option.h"
> +#include "qemu/uuid.h"
>  
>  #ifdef CONFIG_SECCOMP
>  #include "sysemu/seccomp.h"
> -- 
> 2.7.4
> 

Re: [Qemu-devel] [PATCH v3 1/9] util: Add UUID API

[Jeff Cody]

On Tue, Aug 09, 2016 at 02:49:59PM +0800, Fam Zheng wrote:
> A number of different places across the code base use CONFIG_UUID. Some
> of them are soft dependency, some are not built if libuuid is not
> available, some come with dummy fallback, some throws runtime error.
> 
> It is hard to maintain, and hard to reason for users.
> 
> Since UUID is a simple standard with only a small number of operations,
> it is cleaner to have a central support in libqemuutil. This patch adds
> qemu_uuid_* the functions so that all uuid users in the code base can
> rely on. Except for qemu_uuid_generate which is new code, all other
> functions are just copy from existing fallbacks from other files.
> 
> Note that qemu_uuid_parse is moved without updating the function
> signature to use QemuUUID, to keep this patch simple.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  arch_init.c             | 19 -----------
>  block/iscsi.c           |  2 +-
>  hw/smbios/smbios.c      |  1 +
>  include/qemu/uuid.h     | 48 ++++++++++++++++++++++++++
>  include/sysemu/sysemu.h |  4 ---
>  qmp.c                   |  1 +
>  stubs/uuid.c            |  2 +-
>  util/Makefile.objs      |  1 +
>  util/uuid.c             | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  vl.c                    |  1 +
>  10 files changed, 145 insertions(+), 25 deletions(-)
>  create mode 100644 include/qemu/uuid.h
>  create mode 100644 util/uuid.c
> 
> diff --git a/arch_init.c b/arch_init.c
> index fa05973..5cc58b2 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -235,25 +235,6 @@ void audio_init(void)
>      }
>  }
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> -{
> -    int ret;
> -
> -    if (strlen(str) != 36) {
> -        return -1;
> -    }
> -
> -    ret = sscanf(str, UUID_FMT, &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 (ret != 16) {
> -        return -1;
> -    }
> -    return 0;
> -}
> -
>  void do_acpitable_option(const QemuOpts *opts)
>  {
>  #ifdef TARGET_I386
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 95ce9e1..961ac76 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -36,7 +36,7 @@
>  #include "block/block_int.h"
>  #include "block/scsi.h"
>  #include "qemu/iov.h"
> -#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  #include "qapi/qmp/qstring.h"
>  #include "crypto/secret.h"
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 74c7102..0705eb1 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -20,6 +20,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "sysemu/cpus.h"
>  #include "hw/smbios/smbios.h"
>  #include "hw/loader.h"
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> new file mode 100644
> index 0000000..80a75c2
> --- /dev/null
> +++ b/include/qemu/uuid.h
> @@ -0,0 +1,48 @@
> +/*
> + *  QEMU UUID functions
> + *
> + *  Copyright 2016 Red Hat, Inc.,
> + *
> + *  Authors:
> + *   Fam Zheng <famz@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#ifndef QEMU_UUID_H
> +#define QEMU_UUID_H
> +
> +#include "qemu-common.h"
> +
> +/* Version 4 UUID (pseudo random numbers), RFC4122 4.4. */
> +
> +typedef struct {
> +    unsigned char data[16];
> +} QemuUUID;
> +
> +#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
> +                 "%02hhx%02hhx-%02hhx%02hhx-" \
> +                 "%02hhx%02hhx-" \
> +                 "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +
> +#define UUID_FMT_LEN 36
> +
> +#define UUID_NONE "00000000-0000-0000-0000-000000000000"
> +
> +void qemu_uuid_generate(QemuUUID *out);
> +
> +int qemu_uuid_is_null(const QemuUUID *uu);
> +
> +void qemu_uuid_unparse(char *out, const QemuUUID *uuid);

The libuuid version of this function has the prototype
uuid_unparse(uuid_t, char *).  Could we keep the argument order the same as
libuuid, to avoid confusion?


> +
> +char *qemu_uuid_unparse_strdup(const QemuUUID *uuid);
> +
> +int qemu_uuid_parse(const char *str, uint8_t *uuid);
> +
> +void qemu_uuid_convert(QemuUUID *uuid);
> +
> +#endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ee7c760..6111950 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -18,10 +18,6 @@ extern const char *bios_name;
>  extern const char *qemu_name;
>  extern uint8_t qemu_uuid[];
>  extern bool qemu_uuid_set;
> -int qemu_uuid_parse(const char *str, uint8_t *uuid);
> -
> -#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> -#define UUID_NONE "00000000-0000-0000-0000-000000000000"
>  
>  bool runstate_check(RunState state);
>  void runstate_set(RunState new_state);
> diff --git a/qmp.c b/qmp.c
> index b6d531e..0b65ba3 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -18,6 +18,7 @@
>  #include "qemu/cutils.h"
>  #include "monitor/monitor.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  #include "sysemu/char.h"
>  #include "ui/qemu-spice.h"
> diff --git a/stubs/uuid.c b/stubs/uuid.c
> index 92ad717..a880de8 100644
> --- a/stubs/uuid.c
> +++ b/stubs/uuid.c
> @@ -1,6 +1,6 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
> -#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  
>  UuidInfo *qmp_query_uuid(Error **errp)
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 96cb1e0..31bba15 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -20,6 +20,7 @@ util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
>  util-obj-y += qemu-option.o qemu-progress.o
>  util-obj-y += hexdump.o
>  util-obj-y += crc32c.o
> +util-obj-y += uuid.o
>  util-obj-y += throttle.o
>  util-obj-y += getauxval.o
>  util-obj-y += readline.o
> diff --git a/util/uuid.c b/util/uuid.c
> new file mode 100644
> index 0000000..80c89f0
> --- /dev/null
> +++ b/util/uuid.c
> @@ -0,0 +1,91 @@
> +/*
> + *  QEMU UUID functions
> + *
> + *  Copyright 2016 Red Hat, Inc.,
> + *
> + *  Authors:
> + *   Fam Zheng <famz@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/uuid.h"
> +#include "qemu/bswap.h"
> +#include <glib.h>
> +
> +void qemu_uuid_generate(QemuUUID *uuid)
> +{
> +    int i;
> +    uint32_t *out = (uint32_t *)&uuid->data[0];
> +
> +    QEMU_BUILD_BUG_ON(sizeof(QemuUUID) != 16);
> +
> +    for (i = 0; i < 4; ++i) {
> +        out[i] = g_random_int();
> +    }
> +    /* Set the two most significant bits (bits 6 and 7) of the
> +      clock_seq_hi_and_reserved to zero and one, respectively. */
> +    uuid->data[8] = (uuid->data[8] & 0x3f) | 0x80;
> +    /* Set the four most significant bits (bits 12 through 15) of the
> +      time_hi_and_version field to the 4-bit version number.
> +      */
> +    uuid->data[6] = (uuid->data[6] & 0xf) | 0x40;
> +}
> +
> +int qemu_uuid_is_null(const QemuUUID *uu)
> +{
> +    QemuUUID null_uuid = { 0 };
> +    return memcmp(uu, &null_uuid, sizeof(QemuUUID)) == 0;
> +}
> +
> +void qemu_uuid_unparse(char *out, const QemuUUID *uuid)
> +{
> +    const unsigned char *uu = &uuid->data[0];
> +    snprintf(out, UUID_FMT_LEN + 1, UUID_FMT,
> +             uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7],
> +             uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]);
> +}
> +
> +char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
> +{
> +    const unsigned char *uu = &uuid->data[0];
> +    return g_strdup_printf(UUID_FMT,
> +                           uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6],
> +                           uu[7], uu[8], uu[9], uu[10], uu[11], uu[12],
> +                           uu[13], uu[14], uu[15]);
> +}
> +
> +int qemu_uuid_parse(const char *str, uint8_t *uuid)
> +{
> +    int ret;
> +
> +    if (strlen(str) != 36) {
> +        return -1;
> +    }
> +
> +    ret = sscanf(str, UUID_FMT, &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 (ret != 16) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +/* Change UUID from CPU endian to big endian format (network byte order,
> + * standard, see RFC 4122) and vice versa.
> + */
> +void qemu_uuid_convert(QemuUUID *uuid)
> +{
> +    be32_to_cpus((uint32_t *)&uuid->data[0]);
> +    be16_to_cpus((uint16_t *)&uuid->data[4]);
> +    be16_to_cpus((uint16_t *)&uuid->data[6]);
> +}
> diff --git a/vl.c b/vl.c
> index c4eeaff..3d96684 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -25,6 +25,7 @@
>  #include "qemu-version.h"
>  #include "qemu/cutils.h"
>  #include "qemu/help_option.h"
> +#include "qemu/uuid.h"
>  
>  #ifdef CONFIG_SECCOMP
>  #include "sysemu/seccomp.h"
> -- 
> 2.7.4
> 

Re: [Qemu-devel] [PATCH v3 8/9] vl: Switch qemu_uuid to QemuUUID

[Jeff Cody]

On Tue, Aug 09, 2016 at 02:50:06PM +0800, Fam Zheng wrote:
> Update all qemu_uuid users as well, especially get rid of the duplicated
> low level g_strdup_printf, sscanf and snprintf calls with QEMU UUID API.
> 
> Since qemu_uuid_parse is quite tangled with qemu_uuid, it's switching to
> QemuUUID is done here too to keep everything in sync and avoid code
> churn.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/ipmi/ipmi_bmc_sim.c     |  2 +-
>  hw/nvram/fw_cfg.c          |  2 +-
>  hw/ppc/spapr.c             |  7 +------
>  hw/smbios/smbios.c         | 10 +++++-----
>  hw/xenpv/xen_domainbuild.c |  6 +-----
>  include/qemu/uuid.h        |  2 +-
>  include/sysemu/sysemu.h    |  3 ++-
>  qmp.c                      | 10 ++--------
>  util/uuid.c                | 11 ++++++-----
>  vl.c                       |  6 +++---
>  10 files changed, 23 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index dc9c14c..17c7c0e 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -1773,7 +1773,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp)
>      ibs->acpi_power_state[1] = 0;
>  
>      if (qemu_uuid_set) {
> -        memcpy(&ibs->uuid, qemu_uuid, 16);
> +        memcpy(&ibs->uuid, &qemu_uuid, 16);
>      } else {
>          memset(&ibs->uuid, 0, 16);
>      }
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 6a68e59..0118299 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -883,7 +883,7 @@ static void fw_cfg_init1(DeviceState *dev)
>      qdev_init_nofail(dev);
>  
>      fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
> -    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> +    fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
>      fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics);
>      fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>      fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 57564e5..1eef001 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -375,12 +375,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>          g_free(buf);
>      }
>  
> -    buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1],
> -                          qemu_uuid[2], qemu_uuid[3], qemu_uuid[4],
> -                          qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
> -                          qemu_uuid[8], qemu_uuid[9], qemu_uuid[10],
> -                          qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
> -                          qemu_uuid[14], qemu_uuid[15]);
> +    buf = qemu_uuid_unparse_strdup(&qemu_uuid);
>  
>      _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
>      if (qemu_uuid_set) {
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 0705eb1..595f1f2 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -409,7 +409,7 @@ static void smbios_build_type_1_fields(void)
>           * BIOS.
>           */
>          smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
> -                         qemu_uuid, 16);
> +                         &qemu_uuid, 16);
>      }
>  }
>  
> @@ -484,9 +484,9 @@ static void smbios_build_type_0_table(void)
>  /* Encode UUID from the big endian encoding described on RFC4122 to the wire
>   * format specified by SMBIOS version 2.6.
>   */
> -static void smbios_encode_uuid(struct smbios_uuid *uuid, const uint8_t *buf)
> +static void smbios_encode_uuid(struct smbios_uuid *uuid, QemuUUID *in)
>  {
> -    memcpy(uuid, buf, 16);
> +    memcpy(uuid, &in, 16);
>      if (smbios_uuid_encoded) {
>          uuid->time_low = bswap32(uuid->time_low);
>          uuid->time_mid = bswap16(uuid->time_mid);
> @@ -503,7 +503,7 @@ static void smbios_build_type_1_table(void)
>      SMBIOS_TABLE_SET_STR(1, version_str, type1.version);
>      SMBIOS_TABLE_SET_STR(1, serial_number_str, type1.serial);
>      if (qemu_uuid_set) {
> -        smbios_encode_uuid(&t->uuid, qemu_uuid);
> +        smbios_encode_uuid(&t->uuid, &qemu_uuid);
>      } else {
>          memset(&t->uuid, 0, 16);
>      }
> @@ -1002,7 +1002,7 @@ void smbios_entry_add(QemuOpts *opts)
>  
>              val = qemu_opt_get(opts, "uuid");
>              if (val) {
> -                if (qemu_uuid_parse(val, qemu_uuid) != 0) {
> +                if (qemu_uuid_parse(&qemu_uuid, val) != 0) {
>                      error_report("Invalid UUID");
>                      exit(1);
>                  }
> diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
> index 5a9f5ac..deacc5b 100644
> --- a/hw/xenpv/xen_domainbuild.c
> +++ b/hw/xenpv/xen_domainbuild.c
> @@ -53,11 +53,7 @@ int xenstore_domain_init1(const char *kernel, const char *ramdisk,
>      char *dom, uuid_string[42], vm[256], path[256];
>      int i;
>  
> -    snprintf(uuid_string, sizeof(uuid_string), UUID_FMT,
> -             qemu_uuid[0], qemu_uuid[1], qemu_uuid[2], qemu_uuid[3],
> -             qemu_uuid[4], qemu_uuid[5], qemu_uuid[6], qemu_uuid[7],
> -             qemu_uuid[8], qemu_uuid[9], qemu_uuid[10], qemu_uuid[11],
> -             qemu_uuid[12], qemu_uuid[13], qemu_uuid[14], qemu_uuid[15]);
> +    qemu_uuid_unparse(uuid_string, &qemu_uuid);
>      dom = xs_get_domain_path(xenstore, xen_domid);
>      snprintf(vm,  sizeof(vm),  "/vm/%s", uuid_string);
>  
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> index 80a75c2..3f51bb8 100644
> --- a/include/qemu/uuid.h
> +++ b/include/qemu/uuid.h
> @@ -41,7 +41,7 @@ void qemu_uuid_unparse(char *out, const QemuUUID *uuid);
>  
>  char *qemu_uuid_unparse_strdup(const QemuUUID *uuid);
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid);
> +int qemu_uuid_parse(QemuUUID *uuid, const char *str);
>  
>  void qemu_uuid_convert(QemuUUID *uuid);
>  
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 6111950..ef2c50b 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -9,6 +9,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/uuid.h"
>  #include "qom/object.h"
>  
>  /* vl.c */
> @@ -16,7 +17,7 @@
>  extern const char *bios_name;
>  
>  extern const char *qemu_name;
> -extern uint8_t qemu_uuid[];
> +extern QemuUUID qemu_uuid;
>  extern bool qemu_uuid_set;
>  
>  bool runstate_check(RunState state);
> diff --git a/qmp.c b/qmp.c
> index 0b65ba3..c579a7f 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -36,6 +36,7 @@
>  #include "qom/object_interfaces.h"
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/acpi/acpi_dev_interface.h"
> +#include "qemu/uuid.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -85,15 +86,8 @@ KvmInfo *qmp_query_kvm(Error **errp)
>  UuidInfo *qmp_query_uuid(Error **errp)
>  {
>      UuidInfo *info = g_malloc0(sizeof(*info));
> -    char uuid[64];
>  
> -    snprintf(uuid, sizeof(uuid), UUID_FMT, qemu_uuid[0], qemu_uuid[1],
> -                   qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
> -                   qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
> -                   qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
> -                   qemu_uuid[14], qemu_uuid[15]);
> -
> -    info->UUID = g_strdup(uuid);
> +    info->UUID = qemu_uuid_unparse_strdup(&qemu_uuid);
>      return info;
>  }
>  
> diff --git a/util/uuid.c b/util/uuid.c
> index 80c89f0..5d8bc88 100644
> --- a/util/uuid.c
> +++ b/util/uuid.c
> @@ -61,18 +61,19 @@ char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
>                             uu[13], uu[14], uu[15]);
>  }
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> +int qemu_uuid_parse(QemuUUID *uuid, const char *str)

Wait, what's going on here?  Why switch the argument order away from the
libuuid order?


>  {
> +    unsigned char *uu = &uuid->data[0];
>      int ret;
>  
>      if (strlen(str) != 36) {
>          return -1;
>      }
>  
> -    ret = sscanf(str, UUID_FMT, &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]);
> +    ret = sscanf(str, UUID_FMT, &uu[0], &uu[1], &uu[2], &uu[3],
> +                 &uu[4], &uu[5], &uu[6], &uu[7], &uu[8], &uu[9],
> +                 &uu[10], &uu[11], &uu[12], &uu[13], &uu[14],
> +                 &uu[15]);
>  
>      if (ret != 16) {
>          return -1;
> diff --git a/vl.c b/vl.c
> index 3d96684..9108482 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -182,10 +182,10 @@ uint8_t qemu_extra_params_fw[2];
>  
>  int icount_align_option;
>  
> -/* The bytes in qemu_uuid[] are in the order specified by RFC4122, _not_ in the
> +/* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the
>   * little-endian "wire format" described in the SMBIOS 2.6 specification.
>   */
> -uint8_t qemu_uuid[16];
> +QemuUUID qemu_uuid;
>  bool qemu_uuid_set;
>  
>  static NotifierList exit_notifiers =
> @@ -3724,7 +3724,7 @@ int main(int argc, char **argv, char **envp)
>                  cursor_hide = 0;
>                  break;
>              case QEMU_OPTION_uuid:
> -                if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
> +                if (qemu_uuid_parse(&qemu_uuid, optarg) < 0) {
>                      error_report("failed to parse UUID string: wrong format");
>                      exit(1);
>                  }
> -- 
> 2.7.4
> 

Re: [Qemu-devel] [PATCH v3 1/9] util: Add UUID API

[Fam Zheng]

On Tue, 08/09 15:46, Jeff Cody wrote:
> So the short of it:  I think this new function should be explicitly called
> something like "qemu_uuid_bswap()", and do blind bswaps.  That makes it a
> drop in replacement for VDI.

Right, I should make it bswap.

Fam

Re: [Qemu-devel] [PATCH v3 8/9] vl: Switch qemu_uuid to QemuUUID

[Fam Zheng]

On Tue, 08/09 17:45, Jeff Cody wrote:
> > diff --git a/util/uuid.c b/util/uuid.c
> > index 80c89f0..5d8bc88 100644
> > --- a/util/uuid.c
> > +++ b/util/uuid.c
> > @@ -61,18 +61,19 @@ char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
> >                             uu[13], uu[14], uu[15]);
> >  }
> >  
> > -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> > +int qemu_uuid_parse(QemuUUID *uuid, const char *str)
> 
> Wait, what's going on here?  Why switch the argument order away from the
> libuuid order?

I don't see a strong reason to strictly follow libuuid, and thought it would be
more nature to put the output first, but since it's a surprise to you, I can
change it back.

Fam

Re: [Qemu-devel] [PATCH v3 8/9] vl: Switch qemu_uuid to QemuUUID

[Jeff Cody]

On Wed, Aug 10, 2016 at 09:28:56AM +0800, Fam Zheng wrote:
> On Tue, 08/09 17:45, Jeff Cody wrote:
> > > diff --git a/util/uuid.c b/util/uuid.c
> > > index 80c89f0..5d8bc88 100644
> > > --- a/util/uuid.c
> > > +++ b/util/uuid.c
> > > @@ -61,18 +61,19 @@ char *qemu_uuid_unparse_strdup(const QemuUUID *uuid)
> > >                             uu[13], uu[14], uu[15]);
> > >  }
> > >  
> > > -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> > > +int qemu_uuid_parse(QemuUUID *uuid, const char *str)
> > 
> > Wait, what's going on here?  Why switch the argument order away from the
> > libuuid order?
> 
> I don't see a strong reason to strictly follow libuuid, and thought it would be
> more nature to put the output first, but since it's a surprise to you, I can
> change it back.
>

I just wasn't sure if it was intentional (since it wasn't in the commit
message, and it was a difference from v2).  If you feel strongly about
changing the order, I can be OK with it too :)

Re: [Qemu-devel] [PATCH v3 1/9] util: Add UUID API

[Richard Henderson]

On 08/09/2016 12:19 PM, Fam Zheng wrote:
> +/* Version 4 UUID (pseudo random numbers), RFC4122 4.4. */
> +
> +typedef struct {
> +    unsigned char data[16];
> +} QemuUUID;
...
> +void qemu_uuid_generate(QemuUUID *uuid)
> +{
> +    int i;
> +    uint32_t *out = (uint32_t *)&uuid->data[0];

You can't do this cast without adding alignment on the QemuUUID structure.

> +    for (i = 0; i < 4; ++i) {
> +        out[i] = g_random_int();
> +    }

But if there's no other need for uint32_t access to QemuUUID, you could either

(1) write into a local uint32_t[4] array and memcpy over, or
(2) use stl_he_p from qemu/bswap.h, which will handle the unaligned store.


r~

Re: [Qemu-devel] [PATCH v3 1/9] util: Add UUID API

[Fam Zheng]

On Wed, 08/10 16:25, Richard Henderson wrote:
> On 08/09/2016 12:19 PM, Fam Zheng wrote:
> >+/* Version 4 UUID (pseudo random numbers), RFC4122 4.4. */
> >+
> >+typedef struct {
> >+    unsigned char data[16];
> >+} QemuUUID;
> ...
> >+void qemu_uuid_generate(QemuUUID *uuid)
> >+{
> >+    int i;
> >+    uint32_t *out = (uint32_t *)&uuid->data[0];
> 
> You can't do this cast without adding alignment on the QemuUUID structure.
> 
> >+    for (i = 0; i < 4; ++i) {
> >+        out[i] = g_random_int();
> >+    }
> 
> But if there's no other need for uint32_t access to QemuUUID, you could either
> 
> (1) write into a local uint32_t[4] array and memcpy over, or
> (2) use stl_he_p from qemu/bswap.h, which will handle the unaligned store.
> 

Will change to local array. Thanks!

Fam