[libvirt] [PATCH] log: include hostname in initial log message
Laine Stump
laine at laine.org
Fri Dec 4 18:01:53 UTC 2015
On 12/04/2015 12:38 PM, 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 : 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>
> ---
Would it be possible to avoid all the duplicated code by adding a
"quiet" arg to virGetHostname(), then turning virLogHostname() into:
hostname = virGetHostname(true);
if (hostname) {
...
virLogFormatString(blah .... );
...
}
?
> cfg.mk | 2 +-
> src/util/virlog.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 98 insertions(+), 18 deletions(-)
>
> diff --git a/cfg.mk b/cfg.mk
> index cf3f36c..2e3af1a 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -1160,7 +1160,7 @@ _src2=src/(util/vircommand|libvirt|lxc/lxc_controller|locking/lock_daemon|loggin
> exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
> (^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$)
>
> -exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/virutil\.c$$
> +exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/vir(log|util)\.c$$
>
> exclude_file_name_regexp--sc_prohibit_internal_functions = \
> ^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 627f4cb..34e7f2a 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -39,6 +39,7 @@
> #if HAVE_SYS_UN_H
> # include <sys/un.h>
> #endif
> +#include <netdb.h>
>
> #include "virerror.h"
> #include "virlog.h"
> @@ -94,7 +95,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 +403,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 +453,73 @@ 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)
> +{
> + int r;
> + char hostname[HOST_NAME_MAX+1];
> + struct addrinfo hints, *info;
> + char *result = NULL;
> +
> + *rawmsg = *msg = NULL;
> + r = gethostname(hostname, sizeof(hostname));
> + if (r == -1)
> + return -1;
> + NUL_TERMINATE(hostname);
> +
> + if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) {
> + /* in this case, gethostname returned localhost (meaning we can't
> + * do any further canonicalization), or it returned an FQDN (and
> + * we don't need to do any further canonicalization). Return the
> + * string as-is; it's up to callers to check whether "localhost"
> + * is allowed.
> + */
> + ignore_value(VIR_STRDUP_QUIET(result, hostname));
> + goto cleanup;
> + }
> +
> + /* otherwise, it's a shortened, non-localhost, hostname. Attempt to
> + * canonicalize the hostname by running it through getaddrinfo
> + */
> +
> + memset(&hints, 0, sizeof(hints));
> + hints.ai_flags = AI_CANONNAME|AI_CANONIDN;
> + hints.ai_family = AF_UNSPEC;
> + r = getaddrinfo(hostname, NULL, &hints, &info);
> + if (r != 0) {
> + ignore_value(VIR_STRDUP_QUIET(result, hostname));
> + goto cleanup;
> + }
> +
> + /* Tell static analyzers about getaddrinfo semantics. */
> + sa_assert(info);
> +
> + if (info->ai_canonname == NULL ||
> + STRPREFIX(info->ai_canonname, "localhost"))
> + /* in this case, we tried to canonicalize and we ended up back with
> + * localhost. Ignore the canonicalized name and just return the
> + * original hostname
> + */
> + ignore_value(VIR_STRDUP_QUIET(result, hostname));
> + else
> + /* Caller frees this string. */
> + ignore_value(VIR_STRDUP_QUIET(result, info->ai_canonname));
> +
> + freeaddrinfo(info);
> +
> + cleanup:
> + if (virLogFormatString(msg, 0, NULL, VIR_LOG_INFO, result) < 0) {
> + VIR_FREE(result);
> + return -1;
> + }
> + *rawmsg = result;
> + return 0;
> +}
> +
>
> static void
> virLogSourceUpdate(virLogSourcePtr source)
> @@ -533,7 +601,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 +651,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 +675,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,
More information about the libvir-list
mailing list