[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v6 1/9] util: Add UUID API
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v6 1/9] util: Add UUID API |
Date: |
Sat, 17 Sep 2016 16:29:53 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 08/17/2016 02:28 AM, 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
s/the functions so/functions/
> 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 <address@hidden>
> ---
> +++ b/include/qemu/uuid.h
> @@ -0,0 +1,59 @@
> +/*
> + * QEMU UUID functions
> + *
> + * Copyright 2016 Red Hat, Inc.,
Why the trailing comma?
> +++ b/util/uuid.c
> @@ -0,0 +1,92 @@
> +/*
> + * QEMU UUID functions
> + *
> + * Copyright 2016 Red Hat, Inc.,
copy-and-paste? :)
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/uuid.h"
> +#include "qemu/bswap.h"
> +#include <glib.h>
Do you still need the <glib.h> header here? And if so, shouldn't it be
right after osdep.h?
> +
> +void qemu_uuid_generate(QemuUUID *uuid)
> +{
> + int i;
> + uint32_t tmp[4];
> +
> + QEMU_BUILD_BUG_ON(sizeof(QemuUUID) != 16);
> +
> + for (i = 0; i < 4; ++i) {
> + tmp[i] = g_random_int();
> + }
> + memcpy(uuid, tmp, sizeof(tmp));
> + /* 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;
> +}
Looks okay.
> +
> +int qemu_uuid_is_null(const QemuUUID *uu)
> +{
> + QemuUUID null_uuid = { 0 };
Could make this static, so that it doesn't have to be zeroed on every
entry to this function (but in a separate patch, since this one just
moved it, right?)
> + return memcmp(uu, &null_uuid, sizeof(QemuUUID)) == 0;
> +}
> +
Once the nits are cleaned up, you can add:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [Qemu-devel] [PATCH v6 1/9] util: Add UUID API,
Eric Blake <=