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

Re: [libvirt] [PATCH v2] daemon: add option to read host uuid from /etc/machine-id



On 05/02/2016 08:30 AM, Nikolay Shirokovskiy wrote:
> Daemon config parameter switch between reading host uuid
> either from smbios or machine-id:
> 
> host_uuid_source = "smbios|machine-id"
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy virtuozzo com>
> ---
> 
> difference from version 1:
> 
> * move file reading function to virfile.c and generalize it
> * change comments and documentation as suggested by reviewer (see NOTES)
> 
> NOTES
> 
> Cole, I eventually found the way you suggested to document host_uuid 
> slightly misguided. It sounds like it is applied only if
> dmidecode fail. So documentation is slightly different from
> that is your reply.
> 

Yup, I like yours better.

>  daemon/libvirtd-config.c |  2 ++
>  daemon/libvirtd-config.h |  1 +
>  daemon/libvirtd.c        | 38 +++++++++++++++++++++++++++++++++++---
>  daemon/libvirtd.conf     | 15 +++++++++++----
>  src/libvirt_private.syms |  2 ++
>  src/util/virfile.c       | 24 ++++++++++++++++++++++++
>  src/util/virfile.h       |  2 ++
>  src/util/viruuid.c       | 11 ++---------
>  8 files changed, 79 insertions(+), 16 deletions(-)
> 
> diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
> index 7a448f9..45280e9 100644
> --- a/daemon/libvirtd-config.c
> +++ b/daemon/libvirtd-config.c
> @@ -374,6 +374,7 @@ daemonConfigFree(struct daemonConfig *data)
>      VIR_FREE(data->crl_file);
>  
>      VIR_FREE(data->host_uuid);
> +    VIR_FREE(data->host_uuid_source);
>      VIR_FREE(data->log_filters);
>      VIR_FREE(data->log_outputs);
>  
> @@ -463,6 +464,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>      GET_CONF_UINT(conf, filename, audit_logging);
>  
>      GET_CONF_STR(conf, filename, host_uuid);
> +    GET_CONF_STR(conf, filename, host_uuid_source);
>  
>      GET_CONF_UINT(conf, filename, log_level);
>      GET_CONF_STR(conf, filename, log_filters);
> diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
> index 3e1971d..672e9ad 100644
> --- a/daemon/libvirtd-config.h
> +++ b/daemon/libvirtd-config.h
> @@ -28,6 +28,7 @@
>  
>  struct daemonConfig {
>      char *host_uuid;
> +    char *host_uuid_source;
>  
>      int listen_tls;
>      int listen_tcp;
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 7ec02ad..a489136 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1076,6 +1076,39 @@ static int migrateProfile(void)
>      return ret;
>  }
>  
> +static int
> +daemonSetupHostUUID(const struct daemonConfig *config)
> +{
> +    static const char *machine_id = "/etc/machine-id";
> +    char buf[VIR_UUID_STRING_BUFLEN];
> +    const char *uuid;
> +
> +    if (config->host_uuid) {
> +        uuid = config->host_uuid;
> +    } else if (!config->host_uuid_source ||
> +               STREQ(config->host_uuid_source, "smbios")) {
> +        /* smbios UUID is fetched on demand in virGetHostUUID */
> +        return 0;
> +    } else if (STREQ(config->host_uuid_source, "machine-id")) {
> +        if (virFileReadBufQuiet(machine_id, buf, sizeof(buf)) < 0) {
> +            VIR_ERROR(_("Can't read %s"), machine_id);
> +            return -1;
> +        }
> +
> +        uuid = buf;
> +    } else {
> +        VIR_ERROR(_("invalid UUID source: %s"), config->host_uuid_source);
> +        return -1;
> +    }
> +
> +    if (virSetHostUUIDStr(uuid)) {
> +        VIR_ERROR(_("invalid host UUID: %s"), uuid);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  /* Print command-line usage. */
>  static void
>  daemonUsage(const char *argv0, bool privileged)
> @@ -1295,9 +1328,8 @@ int main(int argc, char **argv) {
>          exit(EXIT_FAILURE);
>      }
>  
> -    if (config->host_uuid &&
> -        virSetHostUUIDStr(config->host_uuid) < 0) {
> -        VIR_ERROR(_("invalid host UUID: %s"), config->host_uuid);
> +    if (daemonSetupHostUUID(config) < 0) {
> +        VIR_ERROR(_("Can't setup host uuid"));
>          exit(EXIT_FAILURE);
>      }
>  
> diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
> index d2c439c..1c1fa7f 100644
> --- a/daemon/libvirtd.conf
> +++ b/daemon/libvirtd.conf
> @@ -410,10 +410,16 @@
>  
>  ###################################################################
>  # 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.
> +# Host UUID is read from one of the sources specified in host_uuid_source.
> +#
> +# - 'smbios': fetch the UUID from 'dmidecode -s system-uuid'
> +# - 'machine-id': fetch the UUID from /etc/machine-id
> +#
> +# The host_uuid_source default is 'smbios'. If 'dmidecode' does not provide
> +# a valid UUID a temporary UUID will be generated.
> +#
> +# Another option is to specify host UUID in host_uuid.
> +#
>  # Keep the format of the example UUID below. UUID must not have all digits
>  # be the same.
>  
> @@ -421,6 +427,7 @@
>  # it with the output of the 'uuidgen' command and then
>  # uncomment this entry
>  #host_uuid = "00000000-0000-0000-0000-000000000000"
> +#host_uuid_source = "smbios"
>  
>  ###################################################################
>  # Keepalive protocol:
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 0de35ef..98e9ac5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1510,6 +1510,7 @@ virFileOpenTty;
>  virFilePrintf;
>  virFileReadAll;
>  virFileReadAllQuiet;
> +virFileReadBufQuiet;
>  virFileReadHeaderFD;
>  virFileReadLimFD;
>  virFileRelLinkPointsTo;
> @@ -2537,6 +2538,7 @@ virUUIDFormat;
>  virUUIDGenerate;
>  virUUIDIsValid;
>  virUUIDParse;
> +virUUIDRead;
>  

This is no longer relevant.

>  
>  # util/virxml.h
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 730c08d..4d7b510 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -1401,6 +1401,30 @@ virFileReadAllQuiet(const char *path, int maxlen, char **buf)
>      return len;
>  }
>  
> +/* Read @file into preallocated buffer @buf of size @len.
> + * Return value is -errno in case of errors and size
> + * of data read (no trailing zero) in case of success.
> + * If there is more data then @len - 1 then data will be
> + * truncated. */
> +int
> +virFileReadBufQuiet(const char *file, char *buf, int len)
> +{
> +    int fd;
> +    ssize_t sz;
> +
> +    fd = open(file, O_RDONLY);
> +    if (fd < 0)
> +        return -errno;
> +
> +    sz = saferead(fd, buf, len - 1);
> +    VIR_FORCE_CLOSE(fd);
> +    if (sz < 0)
> +        return -errno;
> +
> +    buf[sz] = '\0';
> +    return sz;
> +}
> +
>  /* Truncate @path and write @str to it.  If @mode is 0, ensure that
>     @path exists; otherwise, use @mode if @path must be created.
>     Return 0 for success, nonzero for failure.
> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index 312f226..dc62eab 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -131,6 +131,8 @@ int virFileReadAll(const char *path, int maxlen, char **buf)
>      ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
>  int virFileReadAllQuiet(const char *path, int maxlen, char **buf)
>      ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
> +int virFileReadBufQuiet(const char *file, char *buf, int len)
> +    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
>  int virFileWriteStr(const char *path, const char *str, mode_t mode)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
> diff --git a/src/util/viruuid.c b/src/util/viruuid.c
> index 16e57db..3cbaae0 100644
> --- a/src/util/viruuid.c
> +++ b/src/util/viruuid.c
> @@ -231,15 +231,8 @@ getDMISystemUUID(char *uuid, int len)
>      };
>  
>      while (paths[i]) {
> -        int fd = open(paths[i], O_RDONLY);
> -        if (fd >= 0) {
> -            if (saferead(fd, uuid, len - 1) == len - 1) {
> -                uuid[len - 1] = '\0';
> -                VIR_FORCE_CLOSE(fd);
> -                return 0;
> -            }
> -            VIR_FORCE_CLOSE(fd);
> -        }
> +        if (virFileReadBufQuiet(paths[i], uuid, len) == len - 1)
> +            return 0;
>          i++;
>      }
>  
> 

Can you split the virfile/viruuid to a separate patch?

Also the qemu.conf bits need to adjust the augeas files. Just 'git grep
host_uuid' and follow the existing pattern in the files it shows

Aside from that the patch looks good and works for me

Thanks,
Cole


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