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