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 <f...@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 <f...@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 <f...@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
>
>
