[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] Add host UUID (to libvirt capabilities)



On Mon, May 24, 2010 at 09:56:22AM -0400, Stefan Berger wrote:
> This patch adds the host UUID (to the capabilities of libvirt). The user
> may provide it in libvirtd.conf overriding whatever sysfs may
> return. If none or no valid UUID is provided in libvirtd.conf, reading the
> UUID from sysfs is attempted. If that function doesn't provide a valid 
> (not all digits may be equal), generate a temporary one.
> virSetHostUUIDStr() should be called first with the UUID read from
> libvirtd.conf, but may only be called once. Subsequently the functions
> virGetHostUUID[Str]() can be called to get the UUID of the host.
> 
> Changes from V1 to V2:
> - addressing comments from Daniel Berrange:
>  - rewrite/ code to get the UUID from 2 possible sysfs files
>  - got rid of dmidecide dependency
>  - call the newly added virGetHostUUID() function in udevGetDMIData() to get
>   the UUID of the host ( even if it's just a temporary UUID )
> 
> Besides that, this patch
> - adds uuid to the capabilties XML schema
> - displays the UUID in in 'virsh capabilities'
> - adds 3 public functions to uuid.h for setting and getting the UUID of
> the host
> 
> This patch contains recycled code from Scott Feldman
> (getDMISystemUUID()).
> 
> Signed-off-by: Stefan Berger <stefanb us ibm com>
> 
> ---
>  daemon/libvirtd.c       |    5 +
>  src/conf/capabilities.c |    4 +
>  src/util/uuid.c         |  126 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/uuid.h         |    4 +
>  4 files changed, 139 insertions(+)
> 
> Index: libvirt-acl/daemon/libvirtd.c
> ===================================================================
> --- libvirt-acl.orig/daemon/libvirtd.c
> +++ libvirt-acl/daemon/libvirtd.c
> @@ -57,6 +57,7 @@
>  #include "dispatch.h"
>  
>  #include "util.h"
> +#include "uuid.h"
>  #include "remote_driver.h"
>  #include "conf.h"
>  #include "event.h"
> @@ -2718,6 +2719,7 @@ remoteReadConfigFile (struct qemud_serve
>      char *unix_sock_rw_perms = NULL;
>      char *unix_sock_group = NULL;
>      char *buf = NULL;
> +    char *host_uuid = NULL;
>  
>  #if HAVE_POLKIT
>      /* Change the default back to no auth for non-root */
> @@ -2840,6 +2842,10 @@ remoteReadConfigFile (struct qemud_serve
>      GET_CONF_INT (conf, filename, max_requests);
>      GET_CONF_INT (conf, filename, max_client_requests);
>  
> +    GET_CONF_STR (conf, filename, host_uuid);
> +    virSetHostUUIDStr(host_uuid);
> +    VIR_FREE(host_uuid);
> +
>      virConfFree (conf);
>      return 0;
>  
> Index: libvirt-acl/src/util/uuid.h
> ===================================================================
> --- libvirt-acl.orig/src/util/uuid.h
> +++ libvirt-acl/src/util/uuid.h
> @@ -22,6 +22,10 @@
>  #ifndef __VIR_UUID_H__
>  # define __VIR_UUID_H__
>  
> +int virSetHostUUIDStr(const char *host_uuid);
> +int virGetHostUUID(unsigned char *host_uuid);
> +int virGetHostUUIDStr(char *host_uuid);

I don't think we need  the virGetHostUUIDStr() variant.
Internally all code should use the raw UUID encoding.
If it desires a printable format, it can use the
existing virUUIDFormat() API directly.

> Index: libvirt-acl/src/util/uuid.c
> ===================================================================
> --- libvirt-acl.orig/src/util/uuid.c
> +++ libvirt-acl/src/util/uuid.c
> @@ -38,11 +38,14 @@
>  #include "util.h"
>  #include "virterror_internal.h"
>  #include "logging.h"
> +#include "memory.h"
>  
>  #ifndef ENODATA
>  # define ENODATA EIO
>  #endif
>  
> +static unsigned char host_uuid[VIR_UUID_BUFLEN];
> +
>  static int
>  virUUIDGenerateRandomBytes(unsigned char *buf,
>                             int buflen)
> @@ -208,3 +211,134 @@ void virUUIDFormat(const unsigned char *
>               uuid[12], uuid[13], uuid[14], uuid[15]);
>      uuidstr[VIR_UUID_STRING_BUFLEN-1] = '\0';
>  }
> +
> +
> +
> +/**
> + * isValidHostUUID
> + *
> + * @uuid: The UUID to test
> + *
> + * Do some basic tests to check whether the given UUID is
> + * valid as a host UUID.
> + * Basic tests:
> + *  - Not all of the digits may be equal
> + */
> +static int
> +isValidHostUUID(unsigned char *uuid)
> +{
> +    unsigned int i, ctr = 1;
> +    unsigned char c;
> +
> +    if (!uuid)
> +        return 0;
> +
> +    c = uuid[0];
> +
> +    for (i = 1; i < VIR_UUID_BUFLEN; i++)
> +        if (uuid[i] == c)
> +            ctr++;
> +
> +    return (ctr != VIR_UUID_BUFLEN);
> +}
> +
> +static int
> +getDMISystemUUID(char *uuid, int len)
> +{
> +    unsigned int i = 0;
> +    const char *paths[] = {
> +        "/sys/devices/virtual/dmi/id/product_uuid",
> +        "/sys/class/dmi/id/product_uuid",
> +        NULL
> +    };
> +
> +    while (paths[i]) {
> +        int fd = open(paths[i], O_RDONLY);
> +        if (fd > 0) {
> +            if (saferead(fd, uuid, len) == len) {
> +                close(fd);
> +                return 0;
> +            }
> +            close(fd);
> +        }
> +        i++;
> +    }
> +
> +    return -1;
> +}

This looks good now.

> +/**
> + * setHostUUID
> + *
> + * @host_uuid: UUID that the host is supposed to have
> + *
> + * Set the UUID of the host if it hasn't been set, yet
> + * Returns 0 in case of success, an error code in case of error.
> + */
> +int
> +virSetHostUUIDStr(const char *uuid)
> +{
> +    char dmiuuid[VIR_UUID_STRING_BUFLEN];
> +
> +    if (isValidHostUUID(host_uuid))
> +        return EEXIST;
> +
> +    if (!uuid) {
> +        if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) {
> +            if (!virUUIDParse(dmiuuid, host_uuid)) {
> +                return 0;
> +            }
> +        }
> +    } else {
> +        if (!virUUIDParse(uuid, host_uuid))
> +            return 0;
> +    }
> +
> +    if (!isValidHostUUID(host_uuid))
> +        return virUUIDGenerate(host_uuid);
> +    return 0;
> +}
> +
> +/**
> + * getHostUUID:
> + *
> + * @host_uuid: memory to store the host_uuid into
> + *
> + * Get the UUID of the host. Returns 0 in case of success,
> + * an error code otherwise.
> + * Returns 0 in case of success, an error code in case of error.
> + */
> +int virGetHostUUID(unsigned char *uuid)
> +{
> +    int ret = 0;
> +
> +    if (!isValidHostUUID(host_uuid))
> +        ret = virSetHostUUIDStr(NULL);
> +
> +    memcpy(uuid, host_uuid, sizeof(host_uuid));
> +
> +    return ret;
> +}
> +
> +
> +/**
> + * getHostUUID:
> + *
> + * @host_uuid: memory to store the host_uuid into
> + *
> + * Get the UUID of the host. Returns 0 in case of success,
> + * an error code otherwise.
> + * Returns 0 in case of success, an error code in case of error.
> + */
> +int virGetHostUUIDStr(char *uuid)
> +{
> +    int ret = 0;
> +
> +    if (!isValidHostUUID(host_uuid))
> +        ret = virSetHostUUIDStr(NULL);
> +
> +    virUUIDFormat(host_uuid, uuid);
> +
> +    return ret;
> +}
> Index: libvirt-acl/src/conf/capabilities.c
> ===================================================================
> --- libvirt-acl.orig/src/conf/capabilities.c
> +++ libvirt-acl/src/conf/capabilities.c
> @@ -27,6 +27,7 @@
>  #include "buf.h"
>  #include "memory.h"
>  #include "util.h"
> +#include "uuid.h"
>  #include "cpu_conf.h"
>  
>  /**
> @@ -662,9 +663,12 @@ virCapabilitiesFormatXML(virCapsPtr caps
>  {
>      virBuffer xml = VIR_BUFFER_INITIALIZER;
>      int i, j, k;
> +    char host_uuid[VIR_UUID_STRING_BUFLEN];
> +    virGetHostUUIDStr(host_uuid);
>  
>      virBufferAddLit(&xml, "<capabilities>\n\n");
>      virBufferAddLit(&xml, "  <host>\n");
> +    virBufferVSprintf(&xml,"    <uuid>%s</uuid>\n", host_uuid);
>      virBufferAddLit(&xml, "    <cpu>\n");
>      virBufferVSprintf(&xml, "      <arch>%s</arch>\n",
>                        caps->host.arch);

Actually scrub my point about the capabilitiestest case
needing changing. This addition here is actually the
problem, since it assumes that the host running libvirt
is the host for which the UUID is needed. This is true
for drivers like QEMU, but not for VMWare.

What we need is for  the virCapsHost struct to get an
extra field 'char uuid[VIR_UUID_BUFLEN]'. Then the
QEMU driver should initialize this filed to match the
virGetHostUUID() data, while the VMWare driver can
initialize it in whatever way it desires.

virCapabilitiesFormatXML() should then br printing the
caps->host.uuid field instead.

> Index: libvirt-acl/docs/schemas/capability.rng
> ===================================================================
> --- libvirt-acl.orig/docs/schemas/capability.rng
> +++ libvirt-acl/docs/schemas/capability.rng
> @@ -18,6 +18,9 @@
>  
>    <define name='hostcaps'>
>      <element name='host'>
> +      <element name='uuid'>
> +          <ref name='UUID'/>
> +      </element>
>        <element name='cpu'>
>          <element name='arch'>
>            <ref name='archnames'/>
> @@ -349,4 +352,15 @@
>        <param name='pattern'>[a-zA-Z0-9\-_]+</param>
>      </data>
>    </define>
> +
> +  <define name="UUID">
> +    <choice>
> +      <data type="string">
> +        <param name="pattern">[a-fA-F0-9]{32}</param>
> +      </data>
> +      <data type="string">
> +        <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param>
> +      </data>
> +    </choice>
> +  </define>
>  </grammar>
> Index: libvirt-acl/src/libvirt_private.syms
> ===================================================================
> --- libvirt-acl.orig/src/libvirt_private.syms
> +++ libvirt-acl/src/libvirt_private.syms
> @@ -708,6 +708,9 @@ usbDeviceFileIterate;
>  virUUIDFormat;
>  virUUIDGenerate;
>  virUUIDParse;
> +virSetHostUUIDStr;
> +virGetHostUUID;
> +virGetHostUUIDStr;
>  
>  
>  # virterror_internal.h
> Index: libvirt-acl/daemon/libvirtd.conf
> ===================================================================
> --- libvirt-acl.orig/daemon/libvirtd.conf
> +++ libvirt-acl/daemon/libvirtd.conf
> @@ -312,3 +312,12 @@
>  # e.g.:
>  # log_outputs="3:syslog:libvirtd"
>  # to log all warnings and errors to syslog under the libvirtd ident
> +
> +# UUID of the host:
> +# Provide the UUID of the host here in case the command
> +# 'dmidecode -s system-uuid' does not provide a valid uuid. In case
> +# 'dmidecode' does not provide a valid UUID and none is provided here, a
> +# temporary UUID will be generated.
> +# Keep the format of the example UUID below.
> +
> +#host_uuid = '8510b1a1-1afa-4da6-8111-785fae202c1d'

Can you also add appropriate rules to the daemon/libvirtd.aug 
and daemon/test_libvirtd.aug files

> Index: libvirt-acl/src/node_device/node_device_udev.c
> ===================================================================
> --- libvirt-acl.orig/src/node_device/node_device_udev.c
> +++ libvirt-acl/src/node_device/node_device_udev.c
> @@ -1473,12 +1473,8 @@ udevGetDMIData(union _virNodeDevCapData 
>          goto out;
>      }
>  
> -    if (udevGetStringSysfsAttr(device,
> -                               "product_uuid",
> -                               &tmp) == PROPERTY_ERROR) {
> +    if (virGetHostUUID(data->system.hardware.uuid))
>          goto out;
> -    }
> -    virUUIDParse(tmp, data->system.hardware.uuid);
>  
>      if (udevGetStringSysfsAttr(device,
>                                 "bios_vendor",
> 


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]