Re: [libvirt] [PATCH] Generate RFC4122 compliant UUIDs

On Mon, Apr 08, 2013 at 12:35:40PM -0400, Milos Vyletel wrote: > Even though http://libvirt.org/formatdomain.html#elementsMetadata > states that it requires RFC4122 compliance UUIDs that are generated > by virUUIDGenerate() are not. Neither does virUUIDIsValid() check > for RFC4122 compliance. Following patch modifies virUUIDGenerate() > to generate valid UUIDs and adds check to virUUIDIsValid() to validate > UUIDs. > > Signed-off-by: Milos Vyletel <milos.vyletel(a)sde.cz&gt; > --- > src/util/viruuid.c | 31 +++++++++++++++++++++++++++++++ > 1 files changed, 31 insertions(+), 0 deletions(-) > > diff --git a/src/util/viruuid.c b/src/util/viruuid.c > index 7250543..2dc9a56 100644 > --- a/src/util/viruuid.c > +++ b/src/util/viruuid.c > @@ -114,6 +114,25 @@ virUUIDGenerate(unsigned char *uuid) > err = virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN); > } > > + /* > + * Make UUID RFC 4122 compliant. Following form will be used: > + * > + * xxxxxxxx-xxxx-Axxx-Bxxx-xxxxxxxxxxxx > + * > + * where > + * A is version defined in 4.1.3 of RFC > + * Msb0 Msb1 Msb2 Msb3 Version Description > + * 0 1 0 0 4 The randomly or pseudo- > + * randomly generated version > + * specified in this document. > + * > + * B is variant defined in 4.1.1 of RFC > + * Msb0 Msb1 Msb2 Description > + * 1 0 x The variant specified in this document. > + */ > + uuid[6] = (uuid[6] & 0x0F) | (4 << 4); > + uuid[8] = (uuid[8] & 0x3F) | (2 << 6); > + > return err; > } ACK to this bit of the patch

> > @@ -209,6 +228,7 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr) > * Do some basic tests to check whether the given UUID is > * valid as a host UUID. > * Basic tests: > + * - Validate RFC4122 compliance > * - Not all of the digits may be equal > */ > int > @@ -216,10 +236,21 @@ virUUIDIsValid(unsigned char *uuid) > { > unsigned int i, ctr = 1; > unsigned char c; > + unsigned char version; > + unsigned char variant; > > if (!uuid) > return 0; > > + /* > + * RFC4122 defines version 1 to 5 (section 4.1.3) > + * RFC4122 defined variant is desribed in section 4.1.1 > + */ > + version = (uuid[6] >> 4); > + variant = (uuid[8] >> 6); > + if (!(version > 0 && version <= 5) || variant != 2) > + return 0; > + > c = uuid[0]; > > for (i = 1; i < VIR_UUID_BUFLEN; i++) but NACk to this part What you're checking here is just one possible valid scheme for UUIDs. We shouldn't reject UUIDs just because they use a different scheme than the one we do.