[libvirt] [PATCH] Generate RFC4122 compliant UUIDs

[Daniel P. Berrange]

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 at sde.cz>
> ---
>  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.

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