[libvirt] [PATCH 0/5] Various logging cleanups
The following patches fixup libvirt's logging code, to hopefully make it a little more consistent and predictable. --- Amy Griffis (5): Tighten libvirt's parsing of logging environment variables Several fixes to libvirtd's log setup Cleanup VIR_LOG_DEBUG parsing in eventtest Consolidate code for parsing the logging environment variables Update logging documentation docs/logging.html.in | 22 +++++++- qemud/qemud.c | 90 +++++++++++++++++--------------- src/libvirt.c | 19 ------- src/libvirt_private.syms | 5 ++ src/logging.c | 129 +++++++++++++++++++++++++++++++++++++++------- src/logging.h | 5 ++ tests/eventtest.c | 12 +--- 7 files changed, 190 insertions(+), 92 deletions(-)
Don't convert high priority levels to the debug level. Don't parse LIBVIRT_LOG_FILTERS and LIBVIRT_LOG_OUTPUTS when they're set to the empty string. Warn when the user specifies an invalid value (empty string remains a noop). --- src/libvirt.c | 13 ++++++++----- src/logging.c | 52 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 61a9b7c..9e96410 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -274,21 +274,24 @@ virInitialize(void) return -1; debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && *debugEnv != '0') { - if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) + if (debugEnv && *debugEnv) { + if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) + virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) virLogSetDefaultPriority(VIR_LOG_INFO); else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) virLogSetDefaultPriority(VIR_LOG_WARN); else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) virLogSetDefaultPriority(VIR_LOG_ERROR); else - virLogSetDefaultPriority(VIR_LOG_DEBUG); + VIR_WARN0(_("Ignoring invalid log level setting.")); } debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv) + if (debugEnv && *debugEnv) virLogParseFilters(debugEnv); + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv) + if (debugEnv && *debugEnv) virLogParseOutputs(debugEnv); DEBUG0("register drivers"); diff --git a/src/logging.c b/src/logging.c index 46a4f9f..27d6e4b 100644 --- a/src/logging.c +++ b/src/logging.c @@ -314,8 +314,10 @@ error: * Returns 0 if successful, -1 in case of error. */ int virLogSetDefaultPriority(int priority) { - if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) + if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) { + VIR_WARN0(_("Ignoring invalid log level setting.")); return(-1); + } if (!virLogInitialized) virLogStartup(); virLogDefaultPriority = priority; @@ -681,7 +683,8 @@ int virLogParseOutputs(const char *outputs) { const char *cur = outputs, *str; char *name; int prio; - int ret = 0; + int ret = -1; + int count = 0; if (cur == NULL) return(-1); @@ -690,53 +693,57 @@ int virLogParseOutputs(const char *outputs) { while (*cur != 0) { prio= virParseNumber(&cur); if ((prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR)) - return(-1); + goto cleanup; if (*cur != ':') - return(-1); + goto cleanup; cur++; if (STREQLEN(cur, "stderr", 6)) { cur += 6; if (virLogAddOutputToStderr(prio) == 0) - ret++; + count++; } else if (STREQLEN(cur, "syslog", 6)) { cur += 6; if (*cur != ':') - return(-1); + goto cleanup; cur++; str = cur; while ((*cur != 0) && (!IS_SPACE(cur))) cur++; if (str == cur) - return(-1); + goto cleanup; #if HAVE_SYSLOG_H name = strndup(str, cur - str); if (name == NULL) - return(-1); + goto cleanup; if (virLogAddOutputToSyslog(prio, name) == 0) - ret++; + count++; VIR_FREE(name); #endif /* HAVE_SYSLOG_H */ } else if (STREQLEN(cur, "file", 4)) { cur += 4; if (*cur != ':') - return(-1); + goto cleanup; cur++; str = cur; while ((*cur != 0) && (!IS_SPACE(cur))) cur++; if (str == cur) - return(-1); + goto cleanup; name = strndup(str, cur - str); if (name == NULL) - return(-1); + goto cleanup; if (virLogAddOutputToFile(prio, name) == 0) - ret++; + count++; VIR_FREE(name); } else { - return(-1); + goto cleanup; } virSkipSpaces(&cur); } + ret = count; +cleanup: + if (ret == -1) + VIR_WARN0(_("Ignoring invalid log output setting.")); return(ret); } @@ -762,7 +769,8 @@ int virLogParseFilters(const char *filters) { const char *cur = filters, *str; char *name; int prio; - int ret = 0; + int ret = -1; + int count = 0; if (cur == NULL) return(-1); @@ -771,22 +779,26 @@ int virLogParseFilters(const char *filters) { while (*cur != 0) { prio= virParseNumber(&cur); if ((prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR)) - return(-1); + goto cleanup; if (*cur != ':') - return(-1); + goto cleanup; cur++; str = cur; while ((*cur != 0) && (!IS_SPACE(cur))) cur++; if (str == cur) - return(-1); + goto cleanup; name = strndup(str, cur - str); if (name == NULL) - return(-1); + goto cleanup; if (virLogDefineFilter(name, prio, 0) >= 0) - ret++; + count++; VIR_FREE(name); virSkipSpaces(&cur); } + ret = count; +cleanup: + if (ret == -1) + VIR_WARN0(_("Ignoring invalid log filter setting.")); return(ret); }
On Fri, Jul 31, 2009 at 05:57:25PM -0400, Amy Griffis wrote:
Don't convert high priority levels to the debug level. Don't parse LIBVIRT_LOG_FILTERS and LIBVIRT_LOG_OUTPUTS when they're set to the empty string. Warn when the user specifies an invalid value (empty string remains a noop). ---
src/libvirt.c | 13 ++++++++----- src/logging.c | 52 ++++++++++++++++++++++++++++++++-------------------- 2 files changed, 40 insertions(+), 25 deletions(-)
ACK Daniel
diff --git a/src/libvirt.c b/src/libvirt.c index 61a9b7c..9e96410 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -274,21 +274,24 @@ virInitialize(void) return -1;
debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && *debugEnv != '0') { - if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) + if (debugEnv && *debugEnv) { + if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) + virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) virLogSetDefaultPriority(VIR_LOG_INFO); else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) virLogSetDefaultPriority(VIR_LOG_WARN); else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) virLogSetDefaultPriority(VIR_LOG_ERROR); else - virLogSetDefaultPriority(VIR_LOG_DEBUG); + VIR_WARN0(_("Ignoring invalid log level setting.")); } debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv) + if (debugEnv && *debugEnv) virLogParseFilters(debugEnv); + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv) + if (debugEnv && *debugEnv) virLogParseOutputs(debugEnv);
DEBUG0("register drivers"); diff --git a/src/logging.c b/src/logging.c index 46a4f9f..27d6e4b 100644 --- a/src/logging.c +++ b/src/logging.c @@ -314,8 +314,10 @@ error: * Returns 0 if successful, -1 in case of error. */ int virLogSetDefaultPriority(int priority) { - if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) + if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) { + VIR_WARN0(_("Ignoring invalid log level setting.")); return(-1); + } if (!virLogInitialized) virLogStartup(); virLogDefaultPriority = priority; @@ -681,7 +683,8 @@ int virLogParseOutputs(const char *outputs) { const char *cur = outputs, *str; char *name; int prio; - int ret = 0; + int ret = -1; + int count = 0;
if (cur == NULL) return(-1); @@ -690,53 +693,57 @@ int virLogParseOutputs(const char *outputs) { while (*cur != 0) { prio= virParseNumber(&cur); if ((prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR)) - return(-1); + goto cleanup; if (*cur != ':') - return(-1); + goto cleanup; cur++; if (STREQLEN(cur, "stderr", 6)) { cur += 6; if (virLogAddOutputToStderr(prio) == 0) - ret++; + count++; } else if (STREQLEN(cur, "syslog", 6)) { cur += 6; if (*cur != ':') - return(-1); + goto cleanup; cur++; str = cur; while ((*cur != 0) && (!IS_SPACE(cur))) cur++; if (str == cur) - return(-1); + goto cleanup; #if HAVE_SYSLOG_H name = strndup(str, cur - str); if (name == NULL) - return(-1); + goto cleanup; if (virLogAddOutputToSyslog(prio, name) == 0) - ret++; + count++; VIR_FREE(name); #endif /* HAVE_SYSLOG_H */ } else if (STREQLEN(cur, "file", 4)) { cur += 4; if (*cur != ':') - return(-1); + goto cleanup; cur++; str = cur; while ((*cur != 0) && (!IS_SPACE(cur))) cur++; if (str == cur) - return(-1); + goto cleanup; name = strndup(str, cur - str); if (name == NULL) - return(-1); + goto cleanup; if (virLogAddOutputToFile(prio, name) == 0) - ret++; + count++; VIR_FREE(name); } else { - return(-1); + goto cleanup; } virSkipSpaces(&cur); } + ret = count; +cleanup: + if (ret == -1) + VIR_WARN0(_("Ignoring invalid log output setting.")); return(ret); }
@@ -762,7 +769,8 @@ int virLogParseFilters(const char *filters) { const char *cur = filters, *str; char *name; int prio; - int ret = 0; + int ret = -1; + int count = 0;
if (cur == NULL) return(-1); @@ -771,22 +779,26 @@ int virLogParseFilters(const char *filters) { while (*cur != 0) { prio= virParseNumber(&cur); if ((prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR)) - return(-1); + goto cleanup; if (*cur != ':') - return(-1); + goto cleanup; cur++; str = cur; while ((*cur != 0) && (!IS_SPACE(cur))) cur++; if (str == cur) - return(-1); + goto cleanup; name = strndup(str, cur - str); if (name == NULL) - return(-1); + goto cleanup; if (virLogDefineFilter(name, prio, 0) >= 0) - ret++; + count++; VIR_FREE(name); virSkipSpaces(&cur); } + ret = count; +cleanup: + if (ret == -1) + VIR_WARN0(_("Ignoring invalid log filter setting.")); return(ret); }
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Jul 31, 2009 at 05:57:25PM -0400, Amy Griffis wrote:
Don't convert high priority levels to the debug level. Don't parse LIBVIRT_LOG_FILTERS and LIBVIRT_LOG_OUTPUTS when they're set to the empty string. Warn when the user specifies an invalid value (empty string remains a noop). [...] +++ b/src/logging.c @@ -314,8 +314,10 @@ error: * Returns 0 if successful, -1 in case of error. */ int virLogSetDefaultPriority(int priority) { - if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) + if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) { + VIR_WARN0(_("Ignoring invalid log level setting."));
Okay, applied, I just had to add src/logging.c to po/POTFILES.in as it now includes translatable strings, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Similar as for general libvirt, don't convert high priority levels to debug level. Ignore LIBVIRT_LOG_FILTERS and LIBVIRT_LOG_OUTPUTS when they're set to the empty string, otherwise they can override a valid setting from the config file. Send all settings through the parser functions for validation, so that the existence of a bad setting doesn't nullify a good setting that should have applied -- particularly the default output. Keep the order of precedence consistent for all variables between the environment and the config file. Warn when an invalid log level, filter, or output is ignored. --- qemud/qemud.c | 82 +++++++++++++++++++++++++++------------------- src/libvirt_private.syms | 3 ++ src/logging.c | 27 +++++++++++++++ src/logging.h | 3 ++ 4 files changed, 81 insertions(+), 34 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index 3e551ca..65c07d9 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -132,11 +132,6 @@ static int timeout = -1; /* -t: Shutdown timeout */ static int sigwrite = -1; /* Signal handler pipe */ static int ipsock = 0; /* -l Listen for TCP/IP */ -/* Defaults for logging */ -static int log_level = VIR_LOG_DEFAULT; -static char *log_filters = NULL; -static char *log_outputs = NULL; - /* Defaults for configuration file elements */ static int listen_tls = 1; static int listen_tcp = 0; @@ -2494,6 +2489,9 @@ remoteReadSaslAllowedUsernameList (virConfPtr conf ATTRIBUTE_UNUSED, static int qemudSetLogging(virConfPtr conf, const char *filename) { char *debugEnv; + int log_level; + char *log_filters = NULL; + char *log_outputs = NULL; int ret = -1; virLogReset(); @@ -2503,54 +2501,70 @@ qemudSetLogging(virConfPtr conf, const char *filename) { * then from environment variable and finally from command * line options */ + /* + * GET_CONF_INT returns 0 when there is no log_level setting in + * the config file. The conditional below eliminates a false + * warning in that case, but also has the side effect of missing + * a warning if the user actually does say log_level=0. + */ GET_CONF_INT (conf, filename, log_level); + if (log_level != 0) + virLogSetDefaultPriority(log_level); + debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && *debugEnv != '0') { - if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - log_level = VIR_LOG_INFO; + if (debugEnv && *debugEnv) { + if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) + virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) + virLogSetDefaultPriority(VIR_LOG_INFO); else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - log_level = VIR_LOG_WARN; + virLogSetDefaultPriority(VIR_LOG_WARN); else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - log_level = VIR_LOG_ERROR; + virLogSetDefaultPriority(VIR_LOG_ERROR); else - log_level = VIR_LOG_DEBUG; + VIR_WARN0(_("Ignoring invalid log level setting.")); + } + + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + + debugEnv = getenv("LIBVIRT_LOG_FILTERS"); + if (debugEnv && *debugEnv) + virLogParseFilters(strdup(debugEnv)); + + if (virLogGetNbFilters() == 0) { + GET_CONF_STR (conf, filename, log_filters); + virLogParseFilters(log_filters); } - if ((verbose) && (log_level >= VIR_LOG_WARN)) - log_level = VIR_LOG_INFO; - virLogSetDefaultPriority(log_level); /* there is no default filters */ - GET_CONF_STR (conf, filename, log_filters); - if (!log_filters) { - debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv) - log_filters = strdup(debugEnv); + + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); + if (debugEnv && *debugEnv) + virLogParseOutputs(strdup(debugEnv)); + + if (virLogGetNbOutputs() == 0) { + GET_CONF_STR (conf, filename, log_outputs); + virLogParseOutputs(log_outputs); } - virLogParseFilters(log_filters); - /* - * by default save all warning and errors to syslog or - * all logs to stderr if not running as daemon + /* + * If no defined outputs, then direct to syslog when running + * as daemon. Otherwise the default output is stderr. */ - GET_CONF_STR (conf, filename, log_outputs); - if (!log_outputs) { - debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv) - log_outputs = strdup(debugEnv); - } - if (!log_outputs) { + if (virLogGetNbOutputs() == 0) { char *tmp = NULL; if (godaemon) { - if (virAsprintf (&tmp, "%d:syslog:libvirtd", log_level) < 0) + if (virAsprintf (&tmp, "%d:syslog:libvirtd", + virLogGetDefaultPriority()) < 0) goto free_and_fail; } else { - if (virAsprintf(&tmp, "%d:stderr", log_level) < 0) + if (virAsprintf (&tmp, "%d:stderr", + virLogGetDefaultPriority()) < 0) goto free_and_fail; } virLogParseOutputs(tmp); VIR_FREE(tmp); - } else { - virLogParseOutputs(log_outputs); } ret = 0; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd63692..487585c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -213,6 +213,9 @@ virRegisterDeviceMonitor; # logging.h virLogMessage; +virLogGetNbFilters; +virLogGetNbOutputs; +virLogGetDefaultPriority; virLogSetDefaultPriority; virLogDefineFilter; virLogDefineOutput; diff --git a/src/logging.c b/src/logging.c index 27d6e4b..aad4b50 100644 --- a/src/logging.c +++ b/src/logging.c @@ -802,3 +802,30 @@ cleanup: VIR_WARN0(_("Ignoring invalid log filter setting.")); return(ret); } + +/** + * virLogGetDefaultPriority: + * + * Returns the current logging priority level. + */ +int virLogGetDefaultPriority(void) { + return (virLogDefaultPriority); +} + +/** + * virLogGetNbFilters: + * + * Returns the current number of defined log filters. + */ +int virLogGetNbFilters(void) { + return (virLogNbFilters); +} + +/** + * virLogGetNbOutputs: + * + * Returns the current number of defined log outputs. + */ +int virLogGetNbOutputs(void) { + return (virLogNbOutputs); +} diff --git a/src/logging.h b/src/logging.h index f1e2525..c8698e5 100644 --- a/src/logging.h +++ b/src/logging.h @@ -105,6 +105,9 @@ typedef int (*virLogOutputFunc) (const char *category, int priority, */ typedef void (*virLogCloseFunc) (void *data); +extern int virLogGetNbFilters(void); +extern int virLogGetNbOutputs(void); +extern int virLogGetDefaultPriority(void); extern int virLogSetDefaultPriority(int priority); extern int virLogDefineFilter(const char *match, int priority, int flags); extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c,
On Fri, Jul 31, 2009 at 05:57:31PM -0400, Amy Griffis wrote:
Similar as for general libvirt, don't convert high priority levels to debug level. Ignore LIBVIRT_LOG_FILTERS and LIBVIRT_LOG_OUTPUTS when they're set to the empty string, otherwise they can override a valid setting from the config file. Send all settings through the parser functions for validation, so that the existence of a bad setting doesn't nullify a good setting that should have applied -- particularly the default output. Keep the order of precedence consistent for all variables between the environment and the config file. Warn when an invalid log level, filter, or output is ignored. ---
qemud/qemud.c | 82 +++++++++++++++++++++++++++------------------- src/libvirt_private.syms | 3 ++ src/logging.c | 27 +++++++++++++++ src/logging.h | 3 ++ 4 files changed, 81 insertions(+), 34 deletions(-)
ACK Daniel
diff --git a/qemud/qemud.c b/qemud/qemud.c index 3e551ca..65c07d9 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -132,11 +132,6 @@ static int timeout = -1; /* -t: Shutdown timeout */ static int sigwrite = -1; /* Signal handler pipe */ static int ipsock = 0; /* -l Listen for TCP/IP */
-/* Defaults for logging */ -static int log_level = VIR_LOG_DEFAULT; -static char *log_filters = NULL; -static char *log_outputs = NULL; - /* Defaults for configuration file elements */ static int listen_tls = 1; static int listen_tcp = 0; @@ -2494,6 +2489,9 @@ remoteReadSaslAllowedUsernameList (virConfPtr conf ATTRIBUTE_UNUSED, static int qemudSetLogging(virConfPtr conf, const char *filename) { char *debugEnv; + int log_level; + char *log_filters = NULL; + char *log_outputs = NULL; int ret = -1;
virLogReset(); @@ -2503,54 +2501,70 @@ qemudSetLogging(virConfPtr conf, const char *filename) { * then from environment variable and finally from command * line options */ + /* + * GET_CONF_INT returns 0 when there is no log_level setting in + * the config file. The conditional below eliminates a false + * warning in that case, but also has the side effect of missing + * a warning if the user actually does say log_level=0. + */ GET_CONF_INT (conf, filename, log_level); + if (log_level != 0) + virLogSetDefaultPriority(log_level); + debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && *debugEnv != '0') { - if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - log_level = VIR_LOG_INFO; + if (debugEnv && *debugEnv) { + if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) + virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) + virLogSetDefaultPriority(VIR_LOG_INFO); else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - log_level = VIR_LOG_WARN; + virLogSetDefaultPriority(VIR_LOG_WARN); else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - log_level = VIR_LOG_ERROR; + virLogSetDefaultPriority(VIR_LOG_ERROR); else - log_level = VIR_LOG_DEBUG; + VIR_WARN0(_("Ignoring invalid log level setting.")); + } + + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + + debugEnv = getenv("LIBVIRT_LOG_FILTERS"); + if (debugEnv && *debugEnv) + virLogParseFilters(strdup(debugEnv)); + + if (virLogGetNbFilters() == 0) { + GET_CONF_STR (conf, filename, log_filters); + virLogParseFilters(log_filters); } - if ((verbose) && (log_level >= VIR_LOG_WARN)) - log_level = VIR_LOG_INFO; - virLogSetDefaultPriority(log_level);
/* there is no default filters */ - GET_CONF_STR (conf, filename, log_filters); - if (!log_filters) { - debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv) - log_filters = strdup(debugEnv); + + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); + if (debugEnv && *debugEnv) + virLogParseOutputs(strdup(debugEnv)); + + if (virLogGetNbOutputs() == 0) { + GET_CONF_STR (conf, filename, log_outputs); + virLogParseOutputs(log_outputs); } - virLogParseFilters(log_filters);
- /* - * by default save all warning and errors to syslog or - * all logs to stderr if not running as daemon + /* + * If no defined outputs, then direct to syslog when running + * as daemon. Otherwise the default output is stderr. */ - GET_CONF_STR (conf, filename, log_outputs); - if (!log_outputs) { - debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv) - log_outputs = strdup(debugEnv); - } - if (!log_outputs) { + if (virLogGetNbOutputs() == 0) { char *tmp = NULL; if (godaemon) { - if (virAsprintf (&tmp, "%d:syslog:libvirtd", log_level) < 0) + if (virAsprintf (&tmp, "%d:syslog:libvirtd", + virLogGetDefaultPriority()) < 0) goto free_and_fail; } else { - if (virAsprintf(&tmp, "%d:stderr", log_level) < 0) + if (virAsprintf (&tmp, "%d:stderr", + virLogGetDefaultPriority()) < 0) goto free_and_fail; } virLogParseOutputs(tmp); VIR_FREE(tmp); - } else { - virLogParseOutputs(log_outputs); } ret = 0;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd63692..487585c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -213,6 +213,9 @@ virRegisterDeviceMonitor;
# logging.h virLogMessage; +virLogGetNbFilters; +virLogGetNbOutputs; +virLogGetDefaultPriority; virLogSetDefaultPriority; virLogDefineFilter; virLogDefineOutput; diff --git a/src/logging.c b/src/logging.c index 27d6e4b..aad4b50 100644 --- a/src/logging.c +++ b/src/logging.c @@ -802,3 +802,30 @@ cleanup: VIR_WARN0(_("Ignoring invalid log filter setting.")); return(ret); } + +/** + * virLogGetDefaultPriority: + * + * Returns the current logging priority level. + */ +int virLogGetDefaultPriority(void) { + return (virLogDefaultPriority); +} + +/** + * virLogGetNbFilters: + * + * Returns the current number of defined log filters. + */ +int virLogGetNbFilters(void) { + return (virLogNbFilters); +} + +/** + * virLogGetNbOutputs: + * + * Returns the current number of defined log outputs. + */ +int virLogGetNbOutputs(void) { + return (virLogNbOutputs); +} diff --git a/src/logging.h b/src/logging.h index f1e2525..c8698e5 100644 --- a/src/logging.h +++ b/src/logging.h @@ -105,6 +105,9 @@ typedef int (*virLogOutputFunc) (const char *category, int priority, */ typedef void (*virLogCloseFunc) (void *data);
+extern int virLogGetNbFilters(void); +extern int virLogGetNbOutputs(void); +extern int virLogGetDefaultPriority(void); extern int virLogSetDefaultPriority(int priority); extern int virLogDefineFilter(const char *match, int priority, int flags); extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c,
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Jul 31, 2009 at 05:57:31PM -0400, Amy Griffis wrote:
Similar as for general libvirt, don't convert high priority levels to debug level. Ignore LIBVIRT_LOG_FILTERS and LIBVIRT_LOG_OUTPUTS when they're set to the empty string, otherwise they can override a valid setting from the config file. Send all settings through the parser functions for validation, so that the existence of a bad setting doesn't nullify a good setting that should have applied -- particularly the default output. Keep the order of precedence consistent for all variables between the environment and the config file. Warn when an invalid log level, filter, or output is ignored.
Okay, nice ! I just had to clear up an end of line space in a comment, commited, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Don't covert high priority levels to debug level. Consider an invalid priority level setting a setup failure. --- tests/eventtest.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/eventtest.c b/tests/eventtest.c index ff74b04..05fe3f3 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -272,15 +272,19 @@ mymain(int argc, char **argv) if (virThreadInitialize() < 0) return EXIT_FAILURE; char *debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && *debugEnv != '0') { - if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) + if (debugEnv && *debugEnv) { + if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) + virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) virLogSetDefaultPriority(VIR_LOG_INFO); else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) virLogSetDefaultPriority(VIR_LOG_WARN); else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) virLogSetDefaultPriority(VIR_LOG_ERROR); - else - virLogSetDefaultPriority(VIR_LOG_DEBUG); + else { + fprintf(stderr, "Invalid log level setting.\n"); + return EXIT_FAILURE; + } } virEventInit();
On Fri, Jul 31, 2009 at 05:57:37PM -0400, Amy Griffis wrote:
Don't covert high priority levels to debug level. Consider an invalid priority level setting a setup failure. ---
tests/eventtest.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
ACK Daniel
diff --git a/tests/eventtest.c b/tests/eventtest.c index ff74b04..05fe3f3 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -272,15 +272,19 @@ mymain(int argc, char **argv) if (virThreadInitialize() < 0) return EXIT_FAILURE; char *debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && *debugEnv != '0') { - if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) + if (debugEnv && *debugEnv) { + if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) + virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) virLogSetDefaultPriority(VIR_LOG_INFO); else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) virLogSetDefaultPriority(VIR_LOG_WARN); else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) virLogSetDefaultPriority(VIR_LOG_ERROR); - else - virLogSetDefaultPriority(VIR_LOG_DEBUG); + else { + fprintf(stderr, "Invalid log level setting.\n"); + return EXIT_FAILURE; + } }
virEventInit();
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Jul 31, 2009 at 05:57:37PM -0400, Amy Griffis wrote:
Don't covert high priority levels to debug level. Consider an invalid priority level setting a setup failure.
Okay, good to unify it there too, applied, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Add two new functions to the internal API, virLogParseDefaultPriority() and virLogSetFromEnv(), as was suggested earlier by Cole Robinson. --- qemud/qemud.c | 50 ++++++++++++++++++---------------------------- src/libvirt.c | 22 +------------------- src/libvirt_private.syms | 2 ++ src/logging.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ src/logging.h | 2 ++ tests/eventtest.c | 16 +++------------ 6 files changed, 78 insertions(+), 64 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index 65c07d9..e6662e4 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -2488,7 +2488,6 @@ remoteReadSaslAllowedUsernameList (virConfPtr conf ATTRIBUTE_UNUSED, */ static int qemudSetLogging(virConfPtr conf, const char *filename) { - char *debugEnv; int log_level; char *log_filters = NULL; char *log_outputs = NULL; @@ -2497,9 +2496,18 @@ qemudSetLogging(virConfPtr conf, const char *filename) { virLogReset(); /* - * look for default logging level first from config file, - * then from environment variable and finally from command - * line options + * Libvirtd's order of precedence is: + * cmdline > environment > config + * + * In order to achieve this, we must process configuration in + * different order for the log level versus the filters and + * outputs. Because filters and outputs append, we have to look at + * the environment first and then only check the config file if + * there was no result from the environment. The default output is + * then applied only if there was no setting from either of the + * first two. Because we don't have a way to determine if the log + * level has been set, we must process variables in the opposite + * order, each one overriding the previous. */ /* * GET_CONF_INT returns 0 when there is no log_level setting in @@ -2511,38 +2519,13 @@ qemudSetLogging(virConfPtr conf, const char *filename) { if (log_level != 0) virLogSetDefaultPriority(log_level); - debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) { - if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) - virLogSetDefaultPriority(VIR_LOG_DEBUG); - else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - virLogSetDefaultPriority(VIR_LOG_INFO); - else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - virLogSetDefaultPriority(VIR_LOG_WARN); - else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - virLogSetDefaultPriority(VIR_LOG_ERROR); - else - VIR_WARN0(_("Ignoring invalid log level setting.")); - } - - if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) - virLogSetDefaultPriority(VIR_LOG_INFO); - - debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv && *debugEnv) - virLogParseFilters(strdup(debugEnv)); + virLogSetFromEnv(); if (virLogGetNbFilters() == 0) { GET_CONF_STR (conf, filename, log_filters); virLogParseFilters(log_filters); } - /* there is no default filters */ - - debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv && *debugEnv) - virLogParseOutputs(strdup(debugEnv)); - if (virLogGetNbOutputs() == 0) { GET_CONF_STR (conf, filename, log_outputs); virLogParseOutputs(log_outputs); @@ -2566,6 +2549,13 @@ qemudSetLogging(virConfPtr conf, const char *filename) { virLogParseOutputs(tmp); VIR_FREE(tmp); } + + /* + * Command line override for --verbose + */ + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + ret = 0; free_and_fail: diff --git a/src/libvirt.c b/src/libvirt.c index 9e96410..19bffb1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -262,7 +262,6 @@ winsock_init (void) int virInitialize(void) { - char *debugEnv; if (initialized) return(0); @@ -273,26 +272,7 @@ virInitialize(void) virRandomInitialize(time(NULL) ^ getpid())) return -1; - debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) { - if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) - virLogSetDefaultPriority(VIR_LOG_DEBUG); - else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - virLogSetDefaultPriority(VIR_LOG_INFO); - else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - virLogSetDefaultPriority(VIR_LOG_WARN); - else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - virLogSetDefaultPriority(VIR_LOG_ERROR); - else - VIR_WARN0(_("Ignoring invalid log level setting.")); - } - debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv && *debugEnv) - virLogParseFilters(debugEnv); - - debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv && *debugEnv) - virLogParseOutputs(debugEnv); + virLogSetFromEnv(); DEBUG0("register drivers"); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 487585c..f20e5ce 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,8 +217,10 @@ virLogGetNbFilters; virLogGetNbOutputs; virLogGetDefaultPriority; virLogSetDefaultPriority; +virLogSetFromEnv; virLogDefineFilter; virLogDefineOutput; +virLogParseDefaultPriority; virLogParseFilters; virLogParseOutputs; virLogStartup; diff --git a/src/logging.c b/src/logging.c index aad4b50..b2cf0bf 100644 --- a/src/logging.c +++ b/src/logging.c @@ -829,3 +829,53 @@ int virLogGetNbFilters(void) { int virLogGetNbOutputs(void) { return (virLogNbOutputs); } + +/** + * virLogParseDefaultPriority: + * @priority: string defining the desired logging level + * + * Parses and sets the default log priority level. It can take a string or + * number corresponding to the following levels: + * 1: DEBUG + * 2: INFO + * 3: WARNING + * 4: ERROR + * + * Returns the parsed log level or -1 on error. + */ +int virLogParseDefaultPriority(const char *priority) { + int ret = -1; + + if (STREQ(priority, "1") || STREQ(priority, "debug")) + ret = virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(priority, "2") || STREQ(priority, "info")) + ret = virLogSetDefaultPriority(VIR_LOG_INFO); + else if (STREQ(priority, "3") || STREQ(priority, "warning")) + ret = virLogSetDefaultPriority(VIR_LOG_WARN); + else if (STREQ(priority, "4") || STREQ(priority, "error")) + ret = virLogSetDefaultPriority(VIR_LOG_ERROR); + else + VIR_WARN0(_("Ignoring invalid log level setting")); + + return ret; +} + +/** + * virLogSetFromEnv: + * + * Sets virLogDefaultPriority, virLogFilters and virLogOutputs based on + * environment variables. + */ +void virLogSetFromEnv(void) { + char *debugEnv; + + debugEnv = getenv("LIBVIRT_DEBUG"); + if (debugEnv && *debugEnv) + virLogParseDefaultPriority(debugEnv); + debugEnv = getenv("LIBVIRT_LOG_FILTERS"); + if (debugEnv && *debugEnv) + virLogParseFilters(strdup(debugEnv)); + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); + if (debugEnv && *debugEnv) + virLogParseOutputs(strdup(debugEnv)); +} diff --git a/src/logging.h b/src/logging.h index c8698e5..8b2b84c 100644 --- a/src/logging.h +++ b/src/logging.h @@ -109,6 +109,7 @@ extern int virLogGetNbFilters(void); extern int virLogGetNbOutputs(void); extern int virLogGetDefaultPriority(void); extern int virLogSetDefaultPriority(int priority); +extern void virLogSetFromEnv(void); extern int virLogDefineFilter(const char *match, int priority, int flags); extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, void *data, int priority, int flags); @@ -120,6 +121,7 @@ extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, extern int virLogStartup(void); extern int virLogReset(void); extern void virLogShutdown(void); +extern int virLogParseDefaultPriority(const char *priority); extern int virLogParseFilters(const char *filters); extern int virLogParseOutputs(const char *output); extern void virLogMessage(const char *category, int priority, diff --git a/tests/eventtest.c b/tests/eventtest.c index 05fe3f3..d25381d 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -272,19 +272,9 @@ mymain(int argc, char **argv) if (virThreadInitialize() < 0) return EXIT_FAILURE; char *debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) { - if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) - virLogSetDefaultPriority(VIR_LOG_DEBUG); - else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - virLogSetDefaultPriority(VIR_LOG_INFO); - else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - virLogSetDefaultPriority(VIR_LOG_WARN); - else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - virLogSetDefaultPriority(VIR_LOG_ERROR); - else { - fprintf(stderr, "Invalid log level setting.\n"); - return EXIT_FAILURE; - } + if (debugEnv && *debugEnv && (virLogParseDefaultPriority(debugEnv) == -1)) { + fprintf(stderr, "Invalid log level setting.\n"); + return EXIT_FAILURE; } virEventInit();
On Fri, Jul 31, 2009 at 05:57:42PM -0400, Amy Griffis wrote:
Add two new functions to the internal API, virLogParseDefaultPriority() and virLogSetFromEnv(), as was suggested earlier by Cole Robinson. ---
qemud/qemud.c | 50 ++++++++++++++++++---------------------------- src/libvirt.c | 22 +------------------- src/libvirt_private.syms | 2 ++ src/logging.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ src/logging.h | 2 ++ tests/eventtest.c | 16 +++------------ 6 files changed, 78 insertions(+), 64 deletions(-)
ACK, good to remove this duplicated code Daniel
diff --git a/qemud/qemud.c b/qemud/qemud.c index 65c07d9..e6662e4 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -2488,7 +2488,6 @@ remoteReadSaslAllowedUsernameList (virConfPtr conf ATTRIBUTE_UNUSED, */ static int qemudSetLogging(virConfPtr conf, const char *filename) { - char *debugEnv; int log_level; char *log_filters = NULL; char *log_outputs = NULL; @@ -2497,9 +2496,18 @@ qemudSetLogging(virConfPtr conf, const char *filename) { virLogReset();
/* - * look for default logging level first from config file, - * then from environment variable and finally from command - * line options + * Libvirtd's order of precedence is: + * cmdline > environment > config + * + * In order to achieve this, we must process configuration in + * different order for the log level versus the filters and + * outputs. Because filters and outputs append, we have to look at + * the environment first and then only check the config file if + * there was no result from the environment. The default output is + * then applied only if there was no setting from either of the + * first two. Because we don't have a way to determine if the log + * level has been set, we must process variables in the opposite + * order, each one overriding the previous. */ /* * GET_CONF_INT returns 0 when there is no log_level setting in @@ -2511,38 +2519,13 @@ qemudSetLogging(virConfPtr conf, const char *filename) { if (log_level != 0) virLogSetDefaultPriority(log_level);
- debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) { - if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) - virLogSetDefaultPriority(VIR_LOG_DEBUG); - else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - virLogSetDefaultPriority(VIR_LOG_INFO); - else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - virLogSetDefaultPriority(VIR_LOG_WARN); - else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - virLogSetDefaultPriority(VIR_LOG_ERROR); - else - VIR_WARN0(_("Ignoring invalid log level setting.")); - } - - if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) - virLogSetDefaultPriority(VIR_LOG_INFO); - - debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv && *debugEnv) - virLogParseFilters(strdup(debugEnv)); + virLogSetFromEnv();
if (virLogGetNbFilters() == 0) { GET_CONF_STR (conf, filename, log_filters); virLogParseFilters(log_filters); }
- /* there is no default filters */ - - debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv && *debugEnv) - virLogParseOutputs(strdup(debugEnv)); - if (virLogGetNbOutputs() == 0) { GET_CONF_STR (conf, filename, log_outputs); virLogParseOutputs(log_outputs); @@ -2566,6 +2549,13 @@ qemudSetLogging(virConfPtr conf, const char *filename) { virLogParseOutputs(tmp); VIR_FREE(tmp); } + + /* + * Command line override for --verbose + */ + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + ret = 0;
free_and_fail: diff --git a/src/libvirt.c b/src/libvirt.c index 9e96410..19bffb1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -262,7 +262,6 @@ winsock_init (void) int virInitialize(void) { - char *debugEnv; if (initialized) return(0);
@@ -273,26 +272,7 @@ virInitialize(void) virRandomInitialize(time(NULL) ^ getpid())) return -1;
- debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) { - if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) - virLogSetDefaultPriority(VIR_LOG_DEBUG); - else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - virLogSetDefaultPriority(VIR_LOG_INFO); - else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - virLogSetDefaultPriority(VIR_LOG_WARN); - else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - virLogSetDefaultPriority(VIR_LOG_ERROR); - else - VIR_WARN0(_("Ignoring invalid log level setting.")); - } - debugEnv = getenv("LIBVIRT_LOG_FILTERS"); - if (debugEnv && *debugEnv) - virLogParseFilters(debugEnv); - - debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); - if (debugEnv && *debugEnv) - virLogParseOutputs(debugEnv); + virLogSetFromEnv();
DEBUG0("register drivers");
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 487585c..f20e5ce 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,8 +217,10 @@ virLogGetNbFilters; virLogGetNbOutputs; virLogGetDefaultPriority; virLogSetDefaultPriority; +virLogSetFromEnv; virLogDefineFilter; virLogDefineOutput; +virLogParseDefaultPriority; virLogParseFilters; virLogParseOutputs; virLogStartup; diff --git a/src/logging.c b/src/logging.c index aad4b50..b2cf0bf 100644 --- a/src/logging.c +++ b/src/logging.c @@ -829,3 +829,53 @@ int virLogGetNbFilters(void) { int virLogGetNbOutputs(void) { return (virLogNbOutputs); } + +/** + * virLogParseDefaultPriority: + * @priority: string defining the desired logging level + * + * Parses and sets the default log priority level. It can take a string or + * number corresponding to the following levels: + * 1: DEBUG + * 2: INFO + * 3: WARNING + * 4: ERROR + * + * Returns the parsed log level or -1 on error. + */ +int virLogParseDefaultPriority(const char *priority) { + int ret = -1; + + if (STREQ(priority, "1") || STREQ(priority, "debug")) + ret = virLogSetDefaultPriority(VIR_LOG_DEBUG); + else if (STREQ(priority, "2") || STREQ(priority, "info")) + ret = virLogSetDefaultPriority(VIR_LOG_INFO); + else if (STREQ(priority, "3") || STREQ(priority, "warning")) + ret = virLogSetDefaultPriority(VIR_LOG_WARN); + else if (STREQ(priority, "4") || STREQ(priority, "error")) + ret = virLogSetDefaultPriority(VIR_LOG_ERROR); + else + VIR_WARN0(_("Ignoring invalid log level setting")); + + return ret; +} + +/** + * virLogSetFromEnv: + * + * Sets virLogDefaultPriority, virLogFilters and virLogOutputs based on + * environment variables. + */ +void virLogSetFromEnv(void) { + char *debugEnv; + + debugEnv = getenv("LIBVIRT_DEBUG"); + if (debugEnv && *debugEnv) + virLogParseDefaultPriority(debugEnv); + debugEnv = getenv("LIBVIRT_LOG_FILTERS"); + if (debugEnv && *debugEnv) + virLogParseFilters(strdup(debugEnv)); + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); + if (debugEnv && *debugEnv) + virLogParseOutputs(strdup(debugEnv)); +} diff --git a/src/logging.h b/src/logging.h index c8698e5..8b2b84c 100644 --- a/src/logging.h +++ b/src/logging.h @@ -109,6 +109,7 @@ extern int virLogGetNbFilters(void); extern int virLogGetNbOutputs(void); extern int virLogGetDefaultPriority(void); extern int virLogSetDefaultPriority(int priority); +extern void virLogSetFromEnv(void); extern int virLogDefineFilter(const char *match, int priority, int flags); extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, void *data, int priority, int flags); @@ -120,6 +121,7 @@ extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, extern int virLogStartup(void); extern int virLogReset(void); extern void virLogShutdown(void); +extern int virLogParseDefaultPriority(const char *priority); extern int virLogParseFilters(const char *filters); extern int virLogParseOutputs(const char *output); extern void virLogMessage(const char *category, int priority, diff --git a/tests/eventtest.c b/tests/eventtest.c index 05fe3f3..d25381d 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -272,19 +272,9 @@ mymain(int argc, char **argv) if (virThreadInitialize() < 0) return EXIT_FAILURE; char *debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv) { - if (STREQ(debugEnv, "1") || STREQ(debugEnv, "debug")) - virLogSetDefaultPriority(VIR_LOG_DEBUG); - else if (STREQ(debugEnv, "2") || STREQ(debugEnv, "info")) - virLogSetDefaultPriority(VIR_LOG_INFO); - else if (STREQ(debugEnv, "3") || STREQ(debugEnv, "warning")) - virLogSetDefaultPriority(VIR_LOG_WARN); - else if (STREQ(debugEnv, "4") || STREQ(debugEnv, "error")) - virLogSetDefaultPriority(VIR_LOG_ERROR); - else { - fprintf(stderr, "Invalid log level setting.\n"); - return EXIT_FAILURE; - } + if (debugEnv && *debugEnv && (virLogParseDefaultPriority(debugEnv) == -1)) { + fprintf(stderr, "Invalid log level setting.\n"); + return EXIT_FAILURE; }
virEventInit();
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Jul 31, 2009 at 05:57:42PM -0400, Amy Griffis wrote:
Add two new functions to the internal API, virLogParseDefaultPriority() and virLogSetFromEnv(), as was suggested earlier by Cole Robinson.
Nice cleanup and good comment about the order ! Applied, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Try to include a little more description about the corner cases, things someone might get hung up on on. --- docs/logging.html.in | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/logging.html.in b/docs/logging.html.in index fcd100f..1f3085d 100644 --- a/docs/logging.html.in +++ b/docs/logging.html.in @@ -44,6 +44,10 @@ <li>LIBVIRT_LOG_FILTERS: allow to define logging filters</li> <li>LIBVIRT_LOG_OUTPUTS: allow to define logging outputs</li> </ul> + <p>Note that, for example, setting LIBVIRT_DEBUG= is the same as unset. If + you specify an invalid value, it will be ignored with a warning. If you + have an error in a filter or output string, some of the settings may be + applied up to the point at which libvirt encountered the error.</p> <p>Similary the daemon logging behaviour can be tuned using 3 config variables, stored in the configuration file: <ul> @@ -57,7 +61,17 @@ <li>log_filters: allow to define logging filters</li> <li>log_outputs: allow to define logging outputs</li> </ul> - <p>In both case the syntax for filters and outputs is similar.</p> + <p>When starting the libvirt daemon, any logging environment variable + settings will override settings in the config file. Command line options + take precedence over all. If no outputs are defined for libvirtd, it + defaults to logging to syslog when it is running as a daemon, or to + stderr when it is running in the foreground.</p> + <p>Libvirtd does not reload its logging configuration when issued a SIGHUP. + If you want to reload the configuration, you must do a <code>service + libvirtd reload</code> or manually stop and restart the daemon + yourself.</p> + <p>The syntax for filters and outputs is the same for both types of + variables.</p> <p>The format for a filter is:</p> <pre>x:name</pre> <p>where <code>name</code> is a match string e.g. <code>remote</code> or @@ -69,10 +83,14 @@ <li>3: WARNING</li> <li>4: ERROR</li> </ul> - <p>Multiple filter can be defined in a single string, they just need to be + <p>Multiple filters can be defined in a single string, they just need to be separated by spaces, e.g: <code>"3:remote 4:event"</code> to only get warning or errors from the remote layer and only errors from the event layer.<p> + <p>If you specify a log priority in a filter that is below the default log + priority level, messages that match that filter will still be logged, + while others will not. In order to see those messages, you must also have + an output defined that includes the priority level of your filter.</p> <p>The format for an output can be one of those 3 forms:</p> <ul> <li><code>x:stderr</code> output goes to stderr</li>
On Fri, Jul 31, 2009 at 05:57:48PM -0400, Amy Griffis wrote:
Try to include a little more description about the corner cases, things someone might get hung up on on. ---
docs/logging.html.in | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/docs/logging.html.in b/docs/logging.html.in index fcd100f..1f3085d 100644 --- a/docs/logging.html.in +++ b/docs/logging.html.in @@ -44,6 +44,10 @@ <li>LIBVIRT_LOG_FILTERS: allow to define logging filters</li> <li>LIBVIRT_LOG_OUTPUTS: allow to define logging outputs</li> </ul> + <p>Note that, for example, setting LIBVIRT_DEBUG= is the same as unset. If + you specify an invalid value, it will be ignored with a warning. If you + have an error in a filter or output string, some of the settings may be + applied up to the point at which libvirt encountered the error.</p> <p>Similary the daemon logging behaviour can be tuned using 3 config variables, stored in the configuration file: <ul> @@ -57,7 +61,17 @@ <li>log_filters: allow to define logging filters</li> <li>log_outputs: allow to define logging outputs</li> </ul> - <p>In both case the syntax for filters and outputs is similar.</p> + <p>When starting the libvirt daemon, any logging environment variable + settings will override settings in the config file. Command line options + take precedence over all. If no outputs are defined for libvirtd, it + defaults to logging to syslog when it is running as a daemon, or to + stderr when it is running in the foreground.</p> + <p>Libvirtd does not reload its logging configuration when issued a SIGHUP. + If you want to reload the configuration, you must do a <code>service + libvirtd reload</code> or manually stop and restart the daemon + yourself.</p> + <p>The syntax for filters and outputs is the same for both types of + variables.</p> <p>The format for a filter is:</p> <pre>x:name</pre> <p>where <code>name</code> is a match string e.g. <code>remote</code> or @@ -69,10 +83,14 @@ <li>3: WARNING</li> <li>4: ERROR</li> </ul> - <p>Multiple filter can be defined in a single string, they just need to be + <p>Multiple filters can be defined in a single string, they just need to be separated by spaces, e.g: <code>"3:remote 4:event"</code> to only get warning or errors from the remote layer and only errors from the event layer.<p> + <p>If you specify a log priority in a filter that is below the default log + priority level, messages that match that filter will still be logged, + while others will not. In order to see those messages, you must also have + an output defined that includes the priority level of your filter.</p> <p>The format for an output can be one of those 3 forms:</p> <ul> <li><code>x:stderr</code> output goes to stderr</li>
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, 2009-07-31 at 17:57 -0400, Amy Griffis wrote:
+ <p>Libvirtd does not reload its logging configuration when issued a SIGHUP. + If you want to reload the configuration, you must do a <code>service + libvirtd reload</code> or manually stop and restart the daemon + yourself.</p>
I guess that should be "service libvirtd restart" ? (i.e. all reload does is issue a SIGHUP) Cheers, Mark.
Mark McLoughlin wrote: [Thu Aug 06 2009, 08:25:06AM EDT]
On Fri, 2009-07-31 at 17:57 -0400, Amy Griffis wrote:
+ <p>Libvirtd does not reload its logging configuration when issued a SIGHUP. + If you want to reload the configuration, you must do a <code>service + libvirtd reload</code> or manually stop and restart the daemon + yourself.</p>
I guess that should be "service libvirtd restart" ?
(i.e. all reload does is issue a SIGHUP)
Oh, I forgot I was looking at debian. :-) Their start script for libvirtd actually does a restart under the reload. Generalizing on "restart" for the docs would certainly cover all cases... Amy
On Thu, Aug 06, 2009 at 09:59:56AM -0400, Amy Griffis wrote:
Mark McLoughlin wrote: [Thu Aug 06 2009, 08:25:06AM EDT]
On Fri, 2009-07-31 at 17:57 -0400, Amy Griffis wrote:
+ <p>Libvirtd does not reload its logging configuration when issued a SIGHUP. + If you want to reload the configuration, you must do a <code>service + libvirtd reload</code> or manually stop and restart the daemon + yourself.</p>
I guess that should be "service libvirtd restart" ?
(i.e. all reload does is issue a SIGHUP)
Oh, I forgot I was looking at debian. :-) Their start script for libvirtd actually does a restart under the reload. Generalizing on "restart" for the docs would certainly cover all cases...
Ewww! They are separate actions for a reason ! Reload (via SIGHUP) is a much less dangerous operation, than a full stop/start based restart. At this point in time, a full restart should only be required for changes to the daemon/driver config files libvirtd.conf, or qemu.conf. Everything else (guest configs, etc) should just work with a mere SIGHUP. Someone might like to fix 'reload' in Debian's initscripts... Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote: [Thu Aug 06 2009, 10:07:26AM EDT]
I guess that should be "service libvirtd restart" ?
(i.e. all reload does is issue a SIGHUP)
Oh, I forgot I was looking at debian. :-) Their start script for libvirtd actually does a restart under the reload. Generalizing on "restart" for the docs would certainly cover all cases...
Ewww! They are separate actions for a reason ! Reload (via SIGHUP) is a much less dangerous operation, than a full stop/start based restart. At this point in time, a full restart should only be required for changes to the daemon/driver config files libvirtd.conf, or qemu.conf. Everything else (guest configs, etc) should just work with a mere SIGHUP. Someone might like to fix 'reload' in Debian's initscripts...
Scratch that, I must have made a typo when I wrote down what I had tried. And then looked at the script this morning when I was not quite awake. I just looked again, and it doesn't do anything funky. Amy
On Fri, Jul 31, 2009 at 05:57:48PM -0400, Amy Griffis wrote:
Try to include a little more description about the corner cases, things someone might get hung up on on.
Excellent, but as Mark pointed out, it's 'service libvirtd restart', modified accordingly, applied. thanks a lot ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Amy Griffis -
Daniel P. Berrange -
Daniel Veillard -
Mark McLoughlin