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

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

[Fam Zheng]

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.

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 (7):
  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

 arch_init.c               | 19 -------------
 block/Makefile.objs       |  2 +-
 block/iscsi.c             |  2 +-
 block/vdi.c               | 49 +++++++--------------------------
 block/vhdx-endian.c       |  3 --
 block/vhdx.c              |  7 ++---
 block/vpc.c               |  8 ++----
 configure                 | 70 +++++------------------------------------------
 crypto/block-luks.c       | 26 +++++-------------
 hw/smbios/smbios.c        |  1 +
 include/qemu/uuid.h       | 37 +++++++++++++++++++++++++
 include/sysemu/sysemu.h   |  4 ---
 qmp.c                     |  1 +
 stubs/uuid.c              |  2 +-
 tests/test-crypto-block.c |  2 +-
 util/Makefile.objs        |  1 +
 util/uuid.c               | 63 ++++++++++++++++++++++++++++++++++++++++++
 vl.c                      |  1 +
 18 files changed, 137 insertions(+), 161 deletions(-)
 create mode 100644 include/qemu/uuid.h
 create mode 100644 util/uuid.c

-- 
2.7.4

[Qemu-devel] [PATCH 1/7] 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.

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     | 37 +++++++++++++++++++++++++++++
 include/sysemu/sysemu.h |  4 ----
 qmp.c                   |  1 +
 stubs/uuid.c            |  2 +-
 util/Makefile.objs      |  1 +
 util/uuid.c             | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c                    |  1 +
 10 files changed, 106 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..53d572f
--- /dev/null
+++ b/include/qemu/uuid.h
@@ -0,0 +1,37 @@
+/*
+ *  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"
+
+typedef unsigned char qemu_uuid_t[16];
+
+#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"
+
+void qemu_uuid_generate(qemu_uuid_t out);
+
+int qemu_uuid_is_null(const qemu_uuid_t uu);
+
+void qemu_uuid_unparse(const qemu_uuid_t uu, char *out);
+
+int qemu_uuid_parse(const char *str, uint8_t *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..7fbde29 100644
--- a/qmp.c
+++ b/qmp.c
@@ -35,6 +35,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)
 {
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..ceca343
--- /dev/null
+++ b/util/uuid.c
@@ -0,0 +1,63 @@
+/*
+ *  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 <glib.h>
+
+void qemu_uuid_generate(qemu_uuid_t out)
+{
+    /* Version 4 UUID, RFC4122 4.4. */
+    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
+    *((guint32 *)out + 0) = g_random_int();
+    *((guint32 *)out + 1) = g_random_int();
+    *((guint32 *)out + 2) = g_random_int();
+    *((guint32 *)out + 3) = g_random_int();
+    out[7] = (out[7] & 0xf) | 0x40;
+    out[8] = (out[8] & 0x3f) | 0x80;
+}
+
+int qemu_uuid_is_null(const qemu_uuid_t uu)
+{
+    qemu_uuid_t null_uuid = { 0 };
+    return memcmp(uu, null_uuid, sizeof(qemu_uuid_t)) == 0;
+}
+
+void qemu_uuid_unparse(const qemu_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]);
+}
+
+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;
+}
diff --git a/vl.c b/vl.c
index e7c2c62..8b12f34 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 2/7] 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        |  7 +++----
 configure           | 27 +++------------------------
 4 files changed, 7 insertions(+), 32 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..a11f0ca 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,10 +212,10 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size, int crc_offset)
  */
 void vhdx_guid_generate(MSGUID *guid)
 {
-    uuid_t uuid;
+    qemu_uuid_t uuid;
     assert(guid != NULL);
 
-    uuid_generate(uuid);
+    qemu_uuid_generate(uuid);
     memcpy(guid, uuid, sizeof(MSGUID));
 }
 
diff --git a/configure b/configure
index f57fcc6..cbb96d5 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"
+  ;;
   --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 3/7] vdi: Use QEMU UUID API

[Fam Zheng]

The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
compatible with libuuid.  Use it, and remove the unused code.

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

diff --git a/block/vdi.c b/block/vdi.c
index 8a1cf97..ed63184 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;
+    qemu_uuid_t uuid_image;
+    qemu_uuid_t uuid_last_snap;
+    qemu_uuid_t uuid_link;
+    qemu_uuid_t uuid_parent;
     uint64_t unused2[7];
 } QEMU_PACKED VdiHeader;
 
@@ -209,7 +180,7 @@ typedef struct {
 /* 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)
+static void uuid_convert(qemu_uuid_t uuid)
 {
     bswap32s((uint32_t *)&uuid[0]);
     bswap16s((uint16_t *)&uuid[4]);
@@ -469,11 +440,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 +792,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 4/7] vpc: Use QEMU UUID API

[Fam Zheng]

Previously we conditionally generate if footer->uuid, when libuuid is
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 | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 43707ed..4a60438 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"
 
 /**************************************************************/
 
@@ -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 5/7] 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..92b5ee5 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
+    qemu_uuid_t uuid;
+    qemu_uuid_generate(uuid);
+    qemu_uuid_unparse(uuid, (char *)uuidstr);
 }
 
 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 6/7] 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 7/7] 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 cbb96d5..d6d71f3 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"
   ;;
+  --enable-uuid|--disable-uuid)
+      echo "$0: $opt is obsolete, UUID support is always built"
+  ;;
   --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

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

[Paolo Bonzini]

----- Original Message -----
> From: "Fam Zheng" <famz@redhat.com>
> To: qemu-devel@nongnu.org
> Cc: famz@redhat.com, berrange@redhat.com, pbonzini@redhat.com, kwolf@redhat.com, mreitz@redhat.com,
> mdroth@linux.vnet.ibm.com, armbru@redhat.com, sw@weilnetz.de, qemu-block@nongnu.org
> Sent: Tuesday, August 2, 2016 11:18:32 AM
> Subject: [PATCH 1/7] util: Add UUID API
> 
> 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.

How is g_random_* seeded?

Paolo

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

[Fam Zheng]

On Tue, 08/02 15:45, Paolo Bonzini wrote:
> 
> 
> ----- Original Message -----
> > From: "Fam Zheng" <famz@redhat.com>
> > To: qemu-devel@nongnu.org
> > Cc: famz@redhat.com, berrange@redhat.com, pbonzini@redhat.com, kwolf@redhat.com, mreitz@redhat.com,
> > mdroth@linux.vnet.ibm.com, armbru@redhat.com, sw@weilnetz.de, qemu-block@nongnu.org
> > Sent: Tuesday, August 2, 2016 11:18:32 AM
> > Subject: [PATCH 1/7] util: Add UUID API
> > 
> > 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.
> 
> How is g_random_* seeded?

According to glib doc:

> GLib changed the seeding algorithm for the pseudo-random number generator
> Mersenne Twister, as used by GRand.

The urandom source is /dev/urandom (or time based if unavailable).

(RFC 4122 explicitly accepts pseudo-random.)

Fam

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

[Jeff Cody]

On Wed, Aug 03, 2016 at 10:36:40AM +0800, Fam Zheng wrote:
> On Tue, 08/02 15:45, Paolo Bonzini wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Fam Zheng" <famz@redhat.com>
> > > To: qemu-devel@nongnu.org
> > > Cc: famz@redhat.com, berrange@redhat.com, pbonzini@redhat.com, kwolf@redhat.com, mreitz@redhat.com,
> > > mdroth@linux.vnet.ibm.com, armbru@redhat.com, sw@weilnetz.de, qemu-block@nongnu.org
> > > Sent: Tuesday, August 2, 2016 11:18:32 AM
> > > Subject: [PATCH 1/7] util: Add UUID API
> > > 
> > > 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.
> > 
> > How is g_random_* seeded?
> 
> According to glib doc:
> 
> > GLib changed the seeding algorithm for the pseudo-random number generator
> > Mersenne Twister, as used by GRand.
> 
> The urandom source is /dev/urandom (or time based if unavailable).
> 
> (RFC 4122 explicitly accepts pseudo-random.)
> 
> Fam
>

To piggyback on Fam's answer:

It is as if qemu called g_rand_new() [1] for a global static GRand struct.

The g_random_* functions use the glib default global GRand struct. If
you don't set the global seed yourself with g_random_set_seed(), then the
first call into one of the g_random_ functions will create the
global GRand struct seeded from /dev/urandom (if available), or the current
time (if /dev/urandom is not available).

[1] https://developer.gnome.org/glib/stable/glib-Random-Numbers.html#g-rand-new


-Jeff

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

[Paolo Bonzini]

On 03/08/2016 04:36, Fam Zheng wrote:
> On Tue, 08/02 15:45, Paolo Bonzini wrote:
>>
>>
>> ----- Original Message -----
>>> From: "Fam Zheng" <famz@redhat.com>
>>> To: qemu-devel@nongnu.org
>>> Cc: famz@redhat.com, berrange@redhat.com, pbonzini@redhat.com, kwolf@redhat.com, mreitz@redhat.com,
>>> mdroth@linux.vnet.ibm.com, armbru@redhat.com, sw@weilnetz.de, qemu-block@nongnu.org
>>> Sent: Tuesday, August 2, 2016 11:18:32 AM
>>> Subject: [PATCH 1/7] util: Add UUID API
>>>
>>> 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.
>>
>> How is g_random_* seeded?
> 
> According to glib doc:
> 
>> GLib changed the seeding algorithm for the pseudo-random number generator
>> Mersenne Twister, as used by GRand.
> 
> The urandom source is /dev/urandom (or time based if unavailable).

That's great then.

Paolo

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

[Marc-André Lureau]

Hi

On Thu, Aug 4, 2016 at 4:44 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

>
>
> On 03/08/2016 04:36, Fam Zheng wrote:
> > On Tue, 08/02 15:45, Paolo Bonzini wrote:
> >>
> >>
> >> ----- Original Message -----
> >>> From: "Fam Zheng" <famz@redhat.com>
> >>> To: qemu-devel@nongnu.org
> >>> Cc: famz@redhat.com, berrange@redhat.com, pbonzini@redhat.com,
> kwolf@redhat.com, mreitz@redhat.com,
> >>> mdroth@linux.vnet.ibm.com, armbru@redhat.com, sw@weilnetz.de,
> qemu-block@nongnu.org
> >>> Sent: Tuesday, August 2, 2016 11:18:32 AM
> >>> Subject: [PATCH 1/7] util: Add UUID API
> >>>
> >>> 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.
>

For info, there has been glib UUID proposal for a while:

https://bugzilla.gnome.org/show_bug.cgi?id=639078

Although quite ready, the discussion stopped because some dev believe
g_uuid_string_random() is all you need. Anyway, if ever accepted, it would
take another 5y or so to be acceptable for qemu :)

-- 
Marc-André Lureau

Re: [Qemu-devel] [Qemu-block] [PATCH 1/7] util: Add UUID API

[Jeff Cody]

On Tue, Aug 02, 2016 at 05:18:32PM +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.
> 
> 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     | 37 +++++++++++++++++++++++++++++
>  include/sysemu/sysemu.h |  4 ----
>  qmp.c                   |  1 +
>  stubs/uuid.c            |  2 +-
>  util/Makefile.objs      |  1 +
>  util/uuid.c             | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>  vl.c                    |  1 +
>  10 files changed, 106 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..53d572f
> --- /dev/null
> +++ b/include/qemu/uuid.h
> @@ -0,0 +1,37 @@
> +/*
> + *  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"
> +
> +typedef unsigned char qemu_uuid_t[16];
> +
> +#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"
> +
> +void qemu_uuid_generate(qemu_uuid_t out);
> +
> +int qemu_uuid_is_null(const qemu_uuid_t uu);
> +
> +void qemu_uuid_unparse(const qemu_uuid_t uu, char *out);
> +
> +int qemu_uuid_parse(const char *str, uint8_t *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..7fbde29 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -35,6 +35,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)
>  {
> 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..ceca343
> --- /dev/null
> +++ b/util/uuid.c
> @@ -0,0 +1,63 @@
> +/*
> + *  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 <glib.h>
> +
> +void qemu_uuid_generate(qemu_uuid_t out)
> +{
> +    /* Version 4 UUID, RFC4122 4.4. */
> +    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
> +    *((guint32 *)out + 0) = g_random_int();
> +    *((guint32 *)out + 1) = g_random_int();
> +    *((guint32 *)out + 2) = g_random_int();
> +    *((guint32 *)out + 3) = g_random_int();
> +    out[7] = (out[7] & 0xf) | 0x40;

Hi Fam,

I think this gets the endianness wrong of the time_hi_and_version field.

I believe this should be:
    out[6] = (out[6] & 0xf) | 0x40;

(but I don't think we should generate it this way, see below)


Per RFC 4122, Section 4.1.2:

   The fields are presented with the most significant one first.
    [...]

   time_hi_and_version    unsigned 16   6-7    The high field of the
                          bit integer          timestamp multiplexed
                                               with the version number

For random UUID generation, Section 4.4:

      Set the four most significant bits (bits 12 through 15) of the
      time_hi_and_version field to the 4-bit version number from
      Section 4.1.3.


So octet 7 is really offset 6 in your array.


It makes sense to represent the uuid internally as a struct with the octet
groupings of uint32_t, uint16_t, etc.., for endianness:

https://github.com/karelzak/util-linux/blob/master/libuuid/src/uuidP.h#L48

(Note, in block/vhdx.h we also represent it that way, to make sure all
platforms copy the UUID out correctly to the image format header).

I'd recommend doing something similar to libuuid:

https://github.com/karelzak/util-linux/blob/master/libuuid/src/gen_uuid.c#L498

BTW, here is a UUID generated with this patch, compared to one that was
generated via libuuid (pulled from qemu-img create -f vhdx):

Current patch:
797cbee7 ffd0 5445  a6 fe ee 22 c2 eb 32 79
              ^^
libuuid:
3f2c08ba 9e5a 4c99  8b bc d4 bd d8 3a 6e e9 
              ^^
The underlined bytes should have their most significant nibble set to 0x4
for a random uuid.

Thanks,
Jeff

> +    out[8] = (out[8] & 0x3f) | 0x80;
> +}
> +
> +int qemu_uuid_is_null(const qemu_uuid_t uu)
> +{
> +    qemu_uuid_t null_uuid = { 0 };
> +    return memcmp(uu, null_uuid, sizeof(qemu_uuid_t)) == 0;
> +}
> +
> +void qemu_uuid_unparse(const qemu_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]);
> +}
> +
> +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;
> +}
> diff --git a/vl.c b/vl.c
> index e7c2c62..8b12f34 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 1/7] util: Add UUID API

[Daniel P. Berrange]

On Tue, Aug 02, 2016 at 05:18:32PM +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.
> 
> 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     | 37 +++++++++++++++++++++++++++++
>  include/sysemu/sysemu.h |  4 ----
>  qmp.c                   |  1 +
>  stubs/uuid.c            |  2 +-
>  util/Makefile.objs      |  1 +
>  util/uuid.c             | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>  vl.c                    |  1 +
>  10 files changed, 106 insertions(+), 25 deletions(-)
>  create mode 100644 include/qemu/uuid.h
>  create mode 100644 util/uuid.c

It would be nice to see you add a tests/test-uuid.c unit test to
exercise all the new utility APIs you're adding & check their
corner cases.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 4207 bytes --]

Hi,

On 08/02/16 11:18, Fam Zheng wrote:
> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
> compatible with libuuid.  Use it, and remove the unused code.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/vdi.c | 49 ++++++++++---------------------------------------
>  1 file changed, 10 insertions(+), 39 deletions(-)
> 
> diff --git a/block/vdi.c b/block/vdi.c
> index 8a1cf97..ed63184 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;
> +    qemu_uuid_t uuid_image;
> +    qemu_uuid_t uuid_last_snap;
> +    qemu_uuid_t uuid_link;
> +    qemu_uuid_t uuid_parent;

As far as I remember, _t should be avoided because that postfix is
reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
because the type name uses the qemu_ prefix?


>      uint64_t unused2[7];
>  } QEMU_PACKED VdiHeader;
>  
> @@ -209,7 +180,7 @@ typedef struct {
>  /* 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)
> +static void uuid_convert(qemu_uuid_t uuid)
>  {
>      bswap32s((uint32_t *)&uuid[0]);
>      bswap16s((uint16_t *)&uuid[4]);
> @@ -469,11 +440,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 +792,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);
> 

Either with qemu_uuid_t or with qemu_uuid:

Reviewed-by: Stefan Weil <sw@weilnetz.de>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 08/04/2016 12:58 PM, Stefan Weil wrote:
> Hi,
> 
> On 08/02/16 11:18, Fam Zheng wrote:
>> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
>> compatible with libuuid.  Use it, and remove the unused code.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>  block/vdi.c | 49 ++++++++++---------------------------------------
>>  1 file changed, 10 insertions(+), 39 deletions(-)
>>

>> @@ -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;
>> +    qemu_uuid_t uuid_image;
>> +    qemu_uuid_t uuid_last_snap;
>> +    qemu_uuid_t uuid_link;
>> +    qemu_uuid_t uuid_parent;
> 
> As far as I remember, _t should be avoided because that postfix is
> reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
> because the type name uses the qemu_ prefix?

Correct that POSIX reserved all _t (which is a bit broad, oh well), and
also correct that we can take the risk of using it anyways (but if POSIX
ever standardizes something, we get to keep both of our broken pieces).


> Either with qemu_uuid_t or with qemu_uuid:

I thought our coding standard preferred QemuUUID or something similar in
camelcase, rather than lower case.  But now we are just painting a
bikeshed, so I won't strongly object.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 08/04/2016 12:58 PM, Stefan Weil wrote:
>> Hi,
>> 
>> On 08/02/16 11:18, Fam Zheng wrote:
>>> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
>>> compatible with libuuid.  Use it, and remove the unused code.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>>  block/vdi.c | 49 ++++++++++---------------------------------------
>>>  1 file changed, 10 insertions(+), 39 deletions(-)
>>>
>
>>> @@ -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;
>>> +    qemu_uuid_t uuid_image;
>>> +    qemu_uuid_t uuid_last_snap;
>>> +    qemu_uuid_t uuid_link;
>>> +    qemu_uuid_t uuid_parent;
>> 
>> As far as I remember, _t should be avoided because that postfix is
>> reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
>> because the type name uses the qemu_ prefix?
>
> Correct that POSIX reserved all _t (which is a bit broad, oh well), and
> also correct that we can take the risk of using it anyways (but if POSIX
> ever standardizes something, we get to keep both of our broken pieces).
>
>
>> Either with qemu_uuid_t or with qemu_uuid:
>
> I thought our coding standard preferred QemuUUID or something similar in
> camelcase, rather than lower case.

Correct.  It's ugly (in my opinion), but we should stick to it, so it's
at least consistently ugly.

>                                     But now we are just painting a
> bikeshed, so I won't strongly object.

I'd prefer QemuUUID.

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Fam Zheng]

On Fri, 08/05 08:31, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 08/04/2016 12:58 PM, Stefan Weil wrote:
> >> Hi,
> >> 
> >> On 08/02/16 11:18, Fam Zheng wrote:
> >>> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
> >>> compatible with libuuid.  Use it, and remove the unused code.
> >>>
> >>> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>> ---
> >>>  block/vdi.c | 49 ++++++++++---------------------------------------
> >>>  1 file changed, 10 insertions(+), 39 deletions(-)
> >>>
> >
> >>> @@ -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;
> >>> +    qemu_uuid_t uuid_image;
> >>> +    qemu_uuid_t uuid_last_snap;
> >>> +    qemu_uuid_t uuid_link;
> >>> +    qemu_uuid_t uuid_parent;
> >> 
> >> As far as I remember, _t should be avoided because that postfix is
> >> reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
> >> because the type name uses the qemu_ prefix?
> >
> > Correct that POSIX reserved all _t (which is a bit broad, oh well), and
> > also correct that we can take the risk of using it anyways (but if POSIX
> > ever standardizes something, we get to keep both of our broken pieces).
> >
> >
> >> Either with qemu_uuid_t or with qemu_uuid:
> >
> > I thought our coding standard preferred QemuUUID or something similar in
> > camelcase, rather than lower case.
> 
> Correct.  It's ugly (in my opinion), but we should stick to it, so it's
> at least consistently ugly.
> 
> >                                     But now we are just painting a
> > bikeshed, so I won't strongly object.
> 
> I'd prefer QemuUUID.

Fair enough. If QEMUUUID wasn't so UNREADABLE, I'd have used it.

Will change to QemuUUID. It's a bit more acceptable to me.

Fam

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

[Fam Zheng]

On Thu, 08/04 16:48, Daniel P. Berrange wrote:
> On Tue, Aug 02, 2016 at 05:18:32PM +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.
> > 
> > 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     | 37 +++++++++++++++++++++++++++++
> >  include/sysemu/sysemu.h |  4 ----
> >  qmp.c                   |  1 +
> >  stubs/uuid.c            |  2 +-
> >  util/Makefile.objs      |  1 +
> >  util/uuid.c             | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  vl.c                    |  1 +
> >  10 files changed, 106 insertions(+), 25 deletions(-)
> >  create mode 100644 include/qemu/uuid.h
> >  create mode 100644 util/uuid.c
> 
> It would be nice to see you add a tests/test-uuid.c unit test to
> exercise all the new utility APIs you're adding & check their
> corner cases.

Sure, I'll add a test case.

Fam

Re: [Qemu-devel] [Qemu-block] [PATCH 1/7] util: Add UUID API

[Fam Zheng]

On Thu, 08/04 11:33, Jeff Cody wrote:
> > +void qemu_uuid_generate(qemu_uuid_t out)
> > +{
> > +    /* Version 4 UUID, RFC4122 4.4. */
> > +    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
> > +    *((guint32 *)out + 0) = g_random_int();
> > +    *((guint32 *)out + 1) = g_random_int();
> > +    *((guint32 *)out + 2) = g_random_int();
> > +    *((guint32 *)out + 3) = g_random_int();
> > +    out[7] = (out[7] & 0xf) | 0x40;
> 
> Hi Fam,
> 
> I think this gets the endianness wrong of the time_hi_and_version field.
> 
> I believe this should be:
>     out[6] = (out[6] & 0xf) | 0x40;
> 
> (but I don't think we should generate it this way, see below)

Right, I'm using a wrong endianness. Will fix this.

Fam

Re: [Qemu-devel] [Qemu-block] [PATCH 1/7] util: Add UUID API

[Fam Zheng]

On Thu, 08/04 11:33, Jeff Cody wrote:
> It makes sense to represent the uuid internally as a struct with the octet
> groupings of uint32_t, uint16_t, etc.., for endianness:

BTW I'm not sure it's worth to add that internal representation. libuuid uses
local endianness when generating and setting version bits, then convert output
to BE anyway.

Fam

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

[Stefan Weil]

Am 02.08.2016 um 11:18 schrieb Fam Zheng:
[...]
> +void qemu_uuid_generate(qemu_uuid_t out)
> +{
> +    /* Version 4 UUID, RFC4122 4.4. */
> +    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
> +    *((guint32 *)out + 0) = g_random_int();
> +    *((guint32 *)out + 1) = g_random_int();
> +    *((guint32 *)out + 2) = g_random_int();
> +    *((guint32 *)out + 3) = g_random_int();

I suggest using uint32_t instead of guint32.
Up to now, nearly all QEMU code uses the POSIX data types.

Regards
Stefan

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

[Fam Zheng]

On Mon, 08/08 08:30, Stefan Weil wrote:
> Am 02.08.2016 um 11:18 schrieb Fam Zheng:
> [...]
> > +void qemu_uuid_generate(qemu_uuid_t out)
> > +{
> > +    /* Version 4 UUID, RFC4122 4.4. */
> > +    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
> > +    *((guint32 *)out + 0) = g_random_int();
> > +    *((guint32 *)out + 1) = g_random_int();
> > +    *((guint32 *)out + 2) = g_random_int();
> > +    *((guint32 *)out + 3) = g_random_int();
> 
> I suggest using uint32_t instead of guint32.
> Up to now, nearly all QEMU code uses the POSIX data types.

This is merely to keep consistent with the g_random_int() return type.  If the
two types had any chance to vary (surely they don't), the uint32_t way would
look like this:

        *((uint32_t *)out + 0) = (uint32_t)g_random_int();

So I think the current way is fine.

Fam

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

[Stefan Weil]

Am 08.08.2016 um 08:33 schrieb Fam Zheng:
> On Mon, 08/08 08:30, Stefan Weil wrote:
>> Am 02.08.2016 um 11:18 schrieb Fam Zheng:
>> [...]
>>> +void qemu_uuid_generate(qemu_uuid_t out)
>>> +{
>>> +    /* Version 4 UUID, RFC4122 4.4. */
>>> +    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
>>> +    *((guint32 *)out + 0) = g_random_int();
>>> +    *((guint32 *)out + 1) = g_random_int();
>>> +    *((guint32 *)out + 2) = g_random_int();
>>> +    *((guint32 *)out + 3) = g_random_int();
>>
>> I suggest using uint32_t instead of guint32.
>> Up to now, nearly all QEMU code uses the POSIX data types.
> 
> This is merely to keep consistent with the g_random_int() return type.  If the
> two types had any chance to vary (surely they don't), the uint32_t way would
> look like this:
> 
>         *((uint32_t *)out + 0) = (uint32_t)g_random_int();
> 
> So I think the current way is fine.
> 
> Fam
> 

Yes, technically it is fine, and I know that you had chosen
guint32 to match the type returned by the GLIB function.

I have a similar issue with the Windows API which also uses its
own data types. Many software APIs bring their own data types
with them. Up to now, we obviously avoided using guint32
although we are using the GLIB API for several years.

I also noticed (after I had send my comment) that you just have
made a v2 of your series, so I'm sorry that my comment came so
late.

Stefan

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

[Markus Armbruster]

Stefan Weil <sw@weilnetz.de> writes:

> Am 08.08.2016 um 08:33 schrieb Fam Zheng:
>> On Mon, 08/08 08:30, Stefan Weil wrote:
>>> Am 02.08.2016 um 11:18 schrieb Fam Zheng:
>>> [...]
>>>> +void qemu_uuid_generate(qemu_uuid_t out)
>>>> +{
>>>> +    /* Version 4 UUID, RFC4122 4.4. */
>>>> +    QEMU_BUILD_BUG_ON(sizeof(qemu_uuid_t) != 16);
>>>> +    *((guint32 *)out + 0) = g_random_int();
>>>> +    *((guint32 *)out + 1) = g_random_int();
>>>> +    *((guint32 *)out + 2) = g_random_int();
>>>> +    *((guint32 *)out + 3) = g_random_int();
>>>
>>> I suggest using uint32_t instead of guint32.
>>> Up to now, nearly all QEMU code uses the POSIX data types.
>> 
>> This is merely to keep consistent with the g_random_int() return type.  If the
>> two types had any chance to vary (surely they don't), the uint32_t way would
>> look like this:
>> 
>>         *((uint32_t *)out + 0) = (uint32_t)g_random_int();

Why would you need to cast the value of g_random_int()?

>> So I think the current way is fine.
>> 
>> Fam
>> 
>
> Yes, technically it is fine, and I know that you had chosen
> guint32 to match the type returned by the GLIB function.
>
> I have a similar issue with the Windows API which also uses its
> own data types. Many software APIs bring their own data types
> with them. Up to now, we obviously avoided using guint32
> although we are using the GLIB API for several years.

Yep.  A few uses have crept in here and there, and I hate every one of
them.

[...]