[libvirt] [PATCH v2] log: include hostname in initial log message
Laine Stump
laine at laine.org
Tue Dec 8 20:04:59 UTC 2015
On 12/08/2015 07:05 AM, Daniel P. Berrange wrote:
> On the very first log message we send to any output, we include
> the libvirt version number and package string. In some bug reports
> we have been given libvirtd.log files that came from a different
> host than the corresponding /var/log/libvirt/qemu log files. So
> extend the initial log message to include the hostname too.
>
> eg on first log message we would now see:
>
> $ libvirtd
> 2015-12-04 17:35:36.610+0000: 20917: info : libvirt version: 1.3.0
> 2015-12-04 17:35:36.610+0000: 20917: info : hostname: dhcp-1-180.lcy.redhat.com
> 2015-12-04 17:35:36.610+0000: 20917: error : qemuMonitorIO:687 : internal error: End of file from monitor
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
> src/libvirt_private.syms | 1 +
> src/util/virlog.c | 72 ++++++++++++++++++++++++++++++++++++------------
> src/util/virutil.c | 34 ++++++++++++++++-------
> src/util/virutil.h | 1 +
> 4 files changed, 81 insertions(+), 27 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index dd085c3..f483c4c 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2387,6 +2387,7 @@ virGetGroupID;
> virGetGroupList;
> virGetGroupName;
> virGetHostname;
> +virGetHostnameQuiet;
> virGetListenFDs;
> virGetSCSIHostNameByParentaddr;
> virGetSCSIHostNumber;
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 627f4cb..7d5a266 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -94,7 +94,7 @@ static int virLogNbFilters;
> * after filtering, multiple output can be used simultaneously
> */
> struct _virLogOutput {
> - bool logVersion;
> + bool logInitMessage;
> void *data;
> virLogOutputFunc f;
> virLogCloseFunc c;
> @@ -402,7 +402,7 @@ virLogDefineOutput(virLogOutputFunc f,
> goto cleanup;
> }
> ret = virLogNbOutputs++;
> - virLogOutputs[ret].logVersion = true;
> + virLogOutputs[ret].logInitMessage = true;
> virLogOutputs[ret].f = f;
> virLogOutputs[ret].c = c;
> virLogOutputs[ret].data = data;
> @@ -452,6 +452,32 @@ virLogVersionString(const char **rawmsg,
> return virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, VIR_LOG_VERSION_STRING);
> }
>
> +/* Similar to virGetHostname() but avoids use of error
> + * reporting APIs or logging APIs, to prevent recursion
> + */
> +static int
> +virLogHostnameString(const char **rawmsg,
> + char **msg)
> +{
> + char *hostname = virGetHostnameQuiet();
> + char *hoststr;
> +
> + if (!hostname)
> + return -1;
> +
> + if (virAsprintfQuiet(&hoststr, "hostname: %s", hostname) < 0) {
> + VIR_FREE(hostname);
> + return -1;
> + }
> +
> + if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, hoststr) < 0) {
> + VIR_FREE(hoststr);
> + return -1;
> + }
> + *rawmsg = hoststr;
> + return 0;
> +}
> +
>
> static void
> virLogSourceUpdate(virLogSourcePtr source)
> @@ -533,7 +559,7 @@ virLogVMessage(virLogSourcePtr source,
> const char *fmt,
> va_list vargs)
> {
> - static bool logVersionStderr = true;
> + static bool logInitMessageStderr = true;
> char *str = NULL;
> char *msg = NULL;
> char timestamp[VIR_TIME_STRING_BUFLEN];
> @@ -583,16 +609,22 @@ virLogVMessage(virLogSourcePtr source,
> */
> for (i = 0; i < virLogNbOutputs; i++) {
> if (priority >= virLogOutputs[i].priority) {
> - if (virLogOutputs[i].logVersion) {
> - const char *rawver;
> - char *ver = NULL;
> - if (virLogVersionString(&rawver, &ver) >= 0)
> + if (virLogOutputs[i].logInitMessage) {
> + const char *rawinitmsg;
> + char *initmsg = NULL;
> + if (virLogVersionString(&rawinitmsg, &initmsg) >= 0)
> + virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO,
> + __FILE__, __LINE__, __func__,
> + timestamp, NULL, 0, rawinitmsg, initmsg,
> + virLogOutputs[i].data);
> + VIR_FREE(initmsg);
> + if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0)
> virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO,
> __FILE__, __LINE__, __func__,
> - timestamp, NULL, 0, rawver, ver,
> + timestamp, NULL, 0, rawinitmsg, initmsg,
> virLogOutputs[i].data);
> - VIR_FREE(ver);
> - virLogOutputs[i].logVersion = false;
> + VIR_FREE(initmsg);
> + virLogOutputs[i].logInitMessage = false;
> }
> virLogOutputs[i].f(source, priority,
> filename, linenr, funcname,
> @@ -601,16 +633,22 @@ virLogVMessage(virLogSourcePtr source,
> }
> }
> if (virLogNbOutputs == 0) {
> - if (logVersionStderr) {
> - const char *rawver;
> - char *ver = NULL;
> - if (virLogVersionString(&rawver, &ver) >= 0)
> + if (logInitMessageStderr) {
> + const char *rawinitmsg;
> + char *initmsg = NULL;
> + if (virLogVersionString(&rawinitmsg, &initmsg) >= 0)
> + virLogOutputToFd(&virLogSelf, VIR_LOG_INFO,
> + __FILE__, __LINE__, __func__,
> + timestamp, NULL, 0, rawinitmsg, initmsg,
> + (void *) STDERR_FILENO);
> + VIR_FREE(initmsg);
> + if (virLogHostnameString(&rawinitmsg, &initmsg) >= 0)
> virLogOutputToFd(&virLogSelf, VIR_LOG_INFO,
> __FILE__, __LINE__, __func__,
> - timestamp, NULL, 0, rawver, ver,
> + timestamp, NULL, 0, rawinitmsg, initmsg,
> (void *) STDERR_FILENO);
> - VIR_FREE(ver);
> - logVersionStderr = false;
> + VIR_FREE(initmsg);
> + logInitMessageStderr = false;
> }
> virLogOutputToFd(source, priority,
> filename, linenr, funcname,
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index fe65faf..e83def1 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -662,16 +662,17 @@ char *virIndexToDiskName(int idx, const char *prefix)
> * we got from getaddrinfo(). Return the value from gethostname()
> * and hope for the best.
> */
> -char *virGetHostname(void)
> +static char *virGetHostnameImpl(bool quiet)
Since you're touching this, may as well put the return type on a
separate line. Otherwise ACK.
> {
> int r;
> - char hostname[HOST_NAME_MAX+1], *result;
> + char hostname[HOST_NAME_MAX+1], *result = NULL;
> struct addrinfo hints, *info;
>
> r = gethostname(hostname, sizeof(hostname));
> if (r == -1) {
> - virReportSystemError(errno,
> - "%s", _("failed to determine host name"));
> + if (!quiet)
> + virReportSystemError(errno,
> + "%s", _("failed to determine host name"));
> return NULL;
> }
> NUL_TERMINATE(hostname);
> @@ -683,7 +684,7 @@ char *virGetHostname(void)
> * string as-is; it's up to callers to check whether "localhost"
> * is allowed.
> */
> - ignore_value(VIR_STRDUP(result, hostname));
> + ignore_value(VIR_STRDUP_QUIET(result, hostname));
> goto cleanup;
> }
>
> @@ -696,9 +697,10 @@ char *virGetHostname(void)
> hints.ai_family = AF_UNSPEC;
> r = getaddrinfo(hostname, NULL, &hints, &info);
> if (r != 0) {
> - VIR_WARN("getaddrinfo failed for '%s': %s",
> - hostname, gai_strerror(r));
> - ignore_value(VIR_STRDUP(result, hostname));
> + if (!quiet)
> + VIR_WARN("getaddrinfo failed for '%s': %s",
> + hostname, gai_strerror(r));
> + ignore_value(VIR_STRDUP_QUIET(result, hostname));
> goto cleanup;
> }
>
> @@ -711,17 +713,29 @@ char *virGetHostname(void)
> * localhost. Ignore the canonicalized name and just return the
> * original hostname
> */
> - ignore_value(VIR_STRDUP(result, hostname));
> + ignore_value(VIR_STRDUP_QUIET(result, hostname));
> else
> /* Caller frees this string. */
> - ignore_value(VIR_STRDUP(result, info->ai_canonname));
> + ignore_value(VIR_STRDUP_QUIET(result, info->ai_canonname));
>
> freeaddrinfo(info);
>
> cleanup:
> + if (!result) {
> + virReportOOMError();
> + }
> return result;
> }
>
> +char *virGetHostname(void)
> +{
> + return virGetHostnameImpl(false);
> +}
> +
> +char *virGetHostnameQuiet(void)
> +{
> + return virGetHostnameImpl(true);
> +}
>
> char *
> virGetUserDirectory(void)
> diff --git a/src/util/virutil.h b/src/util/virutil.h
> index 02387e0..535807c 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -132,6 +132,7 @@ static inline int pthread_sigmask(int how,
> # endif
>
> char *virGetHostname(void);
> +char *virGetHostnameQuiet(void);
>
> char *virGetUserDirectory(void);
> char *virGetUserDirectoryByUID(uid_t uid);
More information about the libvir-list
mailing list