Right now, we define outputs one after another. However, the correct flow should be to define a set of outputs as a whole unit. Therefore each output should be first created, placed into an array/list and the list will be defined. Output creation should be a separate operation, so an output will be returned by a reference. From that perspective, it makes perfect sense to only store pointers to actual outputs. --- src/util/virlog.c | 66 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 6d11328..2519ce2 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -109,8 +109,8 @@ struct _virLogOutput { typedef struct _virLogOutput virLogOutput; typedef virLogOutput *virLogOutputPtr; -static virLogOutputPtr virLogOutputs; -static int virLogNbOutputs; +static virLogOutputPtr *virLogOutputs; +static size_t virLogNbOutputs; /* * Default priorities @@ -323,9 +323,9 @@ virLogResetOutputs(void) size_t i; for (i = 0; i < virLogNbOutputs; i++) { - if (virLogOutputs[i].c != NULL) - virLogOutputs[i].c(virLogOutputs[i].data); - VIR_FREE(virLogOutputs[i].name); + if (virLogOutputs[i]->c != NULL) + virLogOutputs[i]->c(virLogOutputs[i]->data); + VIR_FREE(virLogOutputs[i]->name); } VIR_FREE(virLogOutputs); virLogNbOutputs = 0; @@ -356,8 +356,8 @@ virLogDefineOutput(virLogOutputFunc f, const char *name, unsigned int flags) { - int ret = -1; char *ndup = NULL; + virLogOutputPtr output = NULL; virCheckFlags(0, -1); @@ -376,22 +376,26 @@ virLogDefineOutput(virLogOutputFunc f, return -1; } - virLogLock(); - if (VIR_REALLOC_N_QUIET(virLogOutputs, virLogNbOutputs + 1)) { + if (VIR_ALLOC_QUIET(output) < 0) { VIR_FREE(ndup); - goto cleanup; + return -1; } - ret = virLogNbOutputs++; - virLogOutputs[ret].logInitMessage = true; - virLogOutputs[ret].f = f; - virLogOutputs[ret].c = c; - virLogOutputs[ret].data = data; - virLogOutputs[ret].priority = priority; - virLogOutputs[ret].dest = dest; - virLogOutputs[ret].name = ndup; + + output->logInitMessage = true; + output->f = f; + output->c = c; + output->data = data; + output->priority = priority; + output->dest = dest; + output->name = ndup; + + virLogLock(); + if (VIR_APPEND_ELEMENT_QUIET(virLogOutputs, virLogNbOutputs, output)) + goto cleanup; + cleanup: virLogUnlock(); - return ret; + return virLogNbOutputs; } @@ -589,30 +593,30 @@ virLogVMessage(virLogSourcePtr source, * use stderr. */ for (i = 0; i < virLogNbOutputs; i++) { - if (priority >= virLogOutputs[i].priority) { - if (virLogOutputs[i].logInitMessage) { + if (priority >= virLogOutputs[i]->priority) { + if (virLogOutputs[i]->logInitMessage) { const char *rawinitmsg; char *hoststr = NULL; char *initmsg = NULL; if (virLogVersionString(&rawinitmsg, &initmsg) >= 0) - virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO, + virLogOutputs[i]->f(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, timestamp, NULL, 0, rawinitmsg, initmsg, - virLogOutputs[i].data); + virLogOutputs[i]->data); VIR_FREE(initmsg); if (virLogHostnameString(&hoststr, &initmsg) >= 0) - virLogOutputs[i].f(&virLogSelf, VIR_LOG_INFO, + virLogOutputs[i]->f(&virLogSelf, VIR_LOG_INFO, __FILE__, __LINE__, __func__, timestamp, NULL, 0, hoststr, initmsg, - virLogOutputs[i].data); + virLogOutputs[i]->data); VIR_FREE(hoststr); VIR_FREE(initmsg); - virLogOutputs[i].logInitMessage = false; + virLogOutputs[i]->logInitMessage = false; } - virLogOutputs[i].f(source, priority, + virLogOutputs[i]->f(source, priority, filename, linenr, funcname, timestamp, metadata, filterflags, - str, msg, virLogOutputs[i].data); + str, msg, virLogOutputs[i]->data); } } if (virLogNbOutputs == 0) { @@ -1363,20 +1367,20 @@ virLogGetOutputs(void) virLogLock(); for (i = 0; i < virLogNbOutputs; i++) { - virLogDestination dest = virLogOutputs[i].dest; + virLogDestination dest = virLogOutputs[i]->dest; if (i) virBufferAddChar(&outputbuf, ' '); switch (dest) { case VIR_LOG_TO_SYSLOG: case VIR_LOG_TO_FILE: virBufferAsprintf(&outputbuf, "%d:%s:%s", - virLogOutputs[i].priority, + virLogOutputs[i]->priority, virLogDestinationTypeToString(dest), - virLogOutputs[i].name); + virLogOutputs[i]->name); break; default: virBufferAsprintf(&outputbuf, "%d:%s", - virLogOutputs[i].priority, + virLogOutputs[i]->priority, virLogDestinationTypeToString(dest)); } } -- 2.4.11

Same as with outputs; since the operations will be further divided into smaller tasks, creating a filter will become a separate operation that will return a reference to a newly created filter. --- src/util/virlog.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 2519ce2..812e2cd 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -90,8 +90,8 @@ typedef struct _virLogFilter virLogFilter; typedef virLogFilter *virLogFilterPtr; static int virLogFiltersSerial = 1; -static virLogFilterPtr virLogFilters; -static int virLogNbFilters; +static virLogFilterPtr *virLogFilters; +static size_t virLogNbFilters; /* * Outputs are used to emit the messages retained @@ -246,7 +246,7 @@ virLogResetFilters(void) size_t i; for (i = 0; i < virLogNbFilters; i++) - VIR_FREE(virLogFilters[i].match); + VIR_FREE(virLogFilters[i]->match); VIR_FREE(virLogFilters); virLogNbFilters = 0; virLogFiltersSerial++; @@ -274,6 +274,7 @@ virLogDefineFilter(const char *match, size_t i; int ret = -1; char *mdup = NULL; + virLogFilterPtr filter = NULL; virCheckFlags(VIR_LOG_STACK_TRACE, -1); @@ -286,8 +287,8 @@ virLogDefineFilter(const char *match, virLogLock(); for (i = 0; i < virLogNbFilters; i++) { - if (STREQ(virLogFilters[i].match, match)) { - virLogFilters[i].priority = priority; + if (STREQ(virLogFilters[i]->match, match)) { + virLogFilters[i]->priority = priority; ret = i; goto cleanup; } @@ -295,21 +296,25 @@ virLogDefineFilter(const char *match, if (VIR_STRDUP_QUIET(mdup, match) < 0) goto cleanup; - if (VIR_REALLOC_N_QUIET(virLogFilters, virLogNbFilters + 1)) { + + if (VIR_ALLOC_QUIET(filter) < 0) { VIR_FREE(mdup); goto cleanup; } - ret = virLogNbFilters; - virLogFilters[i].match = mdup; - virLogFilters[i].priority = priority; - virLogFilters[i].flags = flags; - virLogNbFilters++; + + filter->match = mdup; + filter->priority = priority; + filter->flags = flags; + + if (VIR_APPEND_ELEMENT_QUIET(virLogFilters, virLogNbFilters, filter) < 0) + goto cleanup; + virLogFiltersSerial++; cleanup: virLogUnlock(); if (ret < 0) virReportOOMError(); - return ret; + return virLogNbFilters; } /** @@ -474,9 +479,9 @@ virLogSourceUpdate(virLogSourcePtr source) size_t i; for (i = 0; i < virLogNbFilters; i++) { - if (strstr(source->name, virLogFilters[i].match)) { - priority = virLogFilters[i].priority; - flags = virLogFilters[i].flags; + if (strstr(source->name, virLogFilters[i]->match)) { + priority = virLogFilters[i]->priority; + flags = virLogFilters[i]->flags; break; } } @@ -1334,12 +1339,12 @@ virLogGetFilters(void) virLogLock(); for (i = 0; i < virLogNbFilters; i++) { const char *sep = ":"; - if (virLogFilters[i].flags & VIR_LOG_STACK_TRACE) + if (virLogFilters[i]->flags & VIR_LOG_STACK_TRACE) sep = ":+"; virBufferAsprintf(&filterbuf, "%d%s%s ", - virLogFilters[i].priority, + virLogFilters[i]->priority, sep, - virLogFilters[i].match); + virLogFilters[i]->match); } virLogUnlock(); -- 2.4.11

It needs to be exported, since some caller might (for some reason) want to create a logging output without calling the parser which does this. Also, some methods will use virLogOutputPtr as data type for one of its arguments. --- src/util/virlog.c | 2 -- src/util/virlog.h | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 812e2cd..0be1701 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -106,8 +106,6 @@ struct _virLogOutput { virLogDestination dest; char *name; }; -typedef struct _virLogOutput virLogOutput; -typedef virLogOutput *virLogOutputPtr; static virLogOutputPtr *virLogOutputs; static size_t virLogNbOutputs; diff --git a/src/util/virlog.h b/src/util/virlog.h index b5056f5..7706d22 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -130,6 +130,9 @@ struct _virLogMetadata { typedef struct _virLogMetadata virLogMetadata; typedef struct _virLogMetadata *virLogMetadataPtr; +typedef struct _virLogOutput virLogOutput; +typedef virLogOutput *virLogOutputPtr; + /** * virLogOutputFunc: * @src: the source of the log message -- 2.4.11

As with outputs, some methods declared through a header file need to know the datatype because of their arguments. --- src/util/virlog.c | 2 -- src/util/virlog.h | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 0be1701..b893365 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -86,8 +86,6 @@ struct _virLogFilter { virLogPriority priority; unsigned int flags; }; -typedef struct _virLogFilter virLogFilter; -typedef virLogFilter *virLogFilterPtr; static int virLogFiltersSerial = 1; static virLogFilterPtr *virLogFilters; diff --git a/src/util/virlog.h b/src/util/virlog.h index 7706d22..36c610b 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -133,6 +133,9 @@ typedef struct _virLogMetadata *virLogMetadataPtr; typedef struct _virLogOutput virLogOutput; typedef virLogOutput *virLogOutputPtr; +typedef struct _virLogFilter virLogFilter; +typedef virLogFilter *virLogFilterPtr; + /** * virLogOutputFunc: * @src: the source of the log message -- 2.4.11

This method will eventually replace the existing virLogParseFilters which currently does both parsing and defining. However, each method should do one thing only, so will the current virLogParseFilters method will be split into parsing and defining, only to be swallowed by the entry-point API virLogSetFilters. --- src/libvirt_private.syms | 1 + src/util/virlog.c | 23 +++++++++++++++++++++++ src/util/virlog.h | 1 + 3 files changed, 25 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c445d9f..65fafeb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1800,6 +1800,7 @@ virLogPriorityFromSyslog; virLogProbablyLogMessage; virLogReset; virLogSetDefaultPriority; +virLogSetFilters; virLogSetFromEnv; virLogUnlock; virLogVMessage; diff --git a/src/util/virlog.c b/src/util/virlog.c index b893365..0aa722d 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1493,3 +1493,26 @@ bool virLogProbablyLogMessage(const char *str) ret = true; return ret; } + + +/** + * virLogSetFilters: + * @filters: string defining a (set of) filter(s) + * + * Replaces the current set of defined filters with a new set of filters. + * + * Returns the number of filters successfully defined or -1 in case of an + * error. + */ +int +virLogSetFilters(const char *filters) +{ + int ret = -1; + + if (virLogInitialize() < 0) + return -1; + + ret = virLogParseFilters(filters); + + return ret; +} diff --git a/src/util/virlog.h b/src/util/virlog.h index 36c610b..4d7cc98 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -183,6 +183,7 @@ extern char *virLogGetOutputs(void); extern virLogPriority virLogGetDefaultPriority(void); extern int virLogSetDefaultPriority(virLogPriority priority); extern void virLogSetFromEnv(void); +extern int virLogSetFilters(const char *filters); extern int virLogDefineFilter(const char *match, virLogPriority priority, unsigned int flags); -- 2.4.11

Since virLogParseOutputs is going to be stripped from 'output defining' logic, replace all relevant occurrences with virLogSetOutputs to make the change transparent to all original callers (daemons mostly). --- daemon/libvirtd.c | 6 +++--- src/locking/lock_daemon.c | 6 +++--- src/logging/log_daemon.c | 6 +++--- src/util/virlog.c | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index cc5190f..d0019b0 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -693,7 +693,7 @@ daemonSetupLogging(struct daemonConfig *config, virLogParseFilters(config->log_filters); if (virLogGetNbOutputs() == 0) - virLogParseOutputs(config->log_outputs); + virLogSetOutputs(config->log_outputs); /* * Command line override for --verbose @@ -720,7 +720,7 @@ daemonSetupLogging(struct daemonConfig *config, if (virAsprintf(&tmp, "%d:journald", priority) < 0) goto error; - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } } @@ -763,7 +763,7 @@ daemonSetupLogging(struct daemonConfig *config, if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) goto error; } - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f889a34..8d760b2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -479,7 +479,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, virLogParseFilters(config->log_filters); if (virLogGetNbOutputs() == 0) - virLogParseOutputs(config->log_outputs); + virLogSetOutputs(config->log_outputs); /* * If no defined outputs, and either running @@ -493,7 +493,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, if (access("/run/systemd/journal/socket", W_OK) >= 0) { if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) goto error; - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } } @@ -537,7 +537,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) goto error; } - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 90f8427..f8a96e8 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -402,7 +402,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, virLogParseFilters(config->log_filters); if (virLogGetNbOutputs() == 0) - virLogParseOutputs(config->log_outputs); + virLogSetOutputs(config->log_outputs); /* * If no defined outputs, and either running @@ -416,7 +416,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, if (access("/run/systemd/journal/socket", W_OK) >= 0) { if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) goto error; - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } } @@ -460,7 +460,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) goto error; } - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } diff --git a/src/util/virlog.c b/src/util/virlog.c index 2cd3773..538a5b8 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1475,7 +1475,7 @@ virLogSetFromEnv(void) virLogParseFilters(debugEnv); debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_OUTPUTS"); if (debugEnv && *debugEnv) - virLogParseOutputs(debugEnv); + virLogSetOutputs(debugEnv); } -- 2.4.11

Prepare a method that only defines a set of outputs. It takes a list of outputs, preferably created by virLogParseOutputs. The original set of outputs is reset and replaced by the new user-provided set of outputs. --- src/libvirt_private.syms | 1 + src/util/virlog.c | 25 +++++++++++++++++++++++++ src/util/virlog.h | 1 + 3 files changed, 27 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dafcb6c..565fc7c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1786,6 +1786,7 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h virLogDefineFilter; virLogDefineOutput; +virLogDefineOutputs; virLogGetDefaultPriority; virLogGetFilters; virLogGetNbFilters; diff --git a/src/util/virlog.c b/src/util/virlog.c index 9059722..e5923ce 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1539,3 +1539,28 @@ virLogSetOutputs(const char *outputs) return ret; } + + +/** + * virLogDefineOutputs: + * @outputs: new set of outputs to be defined + * @noutputs: number of outputs in @outputs + * + * Resets any existing set of outputs and defines a completely new one. + * + * Returns number of outputs successfully defined or -1 in case of error; + */ +int +virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs) +{ + if (virLogInitialize() < 0) + return -1; + + virLogLock(); + virLogResetOutputs(); + virLogOutputs = outputs; + virLogNbOutputs = noutputs; + virLogUnlock(); + + return virLogNbOutputs; +} diff --git a/src/util/virlog.h b/src/util/virlog.h index 4141ecd..166db5a 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -224,5 +224,6 @@ extern void virLogVMessage(virLogSourcePtr source, va_list vargs) ATTRIBUTE_FMT_PRINTF(7, 0); bool virLogProbablyLogMessage(const char *str); +int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs); #endif -- 2.4.11

This is merely a cosmetic change to clearly state that we won't be adding a new output to the existing global set of outputs, instead we're always creating a completely new output that needs to be later placed into a list. --- src/util/virlog.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 60573a9..387e08a 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -714,7 +714,7 @@ virLogCloseFd(void *data) static int -virLogAddOutputToStderr(virLogPriority priority) +virLogNewOutputToStderr(virLogPriority priority) { if (virLogDefineOutput(virLogOutputToFd, NULL, (void *)2L, priority, VIR_LOG_TO_STDERR, NULL, 0) < 0) @@ -724,7 +724,7 @@ virLogAddOutputToStderr(virLogPriority priority) static int -virLogAddOutputToFile(virLogPriority priority, +virLogNewOutputToFile(virLogPriority priority, const char *file) { int fd; @@ -808,7 +808,7 @@ virLogCloseSyslog(void *data ATTRIBUTE_UNUSED) static int -virLogAddOutputToSyslog(virLogPriority priority, +virLogNewOutputToSyslog(virLogPriority priority, const char *ident) { /* @@ -1031,7 +1031,7 @@ static void virLogCloseJournald(void *data ATTRIBUTE_UNUSED) } -static int virLogAddOutputToJournald(int priority) +static int virLogNewOutputToJournald(int priority) { if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) return -1; @@ -1119,22 +1119,22 @@ virLogParseOutput(const char *src) switch ((virLogDestination) dest) { case VIR_LOG_TO_STDERR: - ret = virLogAddOutputToStderr(prio); + ret = virLogNewOutputToStderr(prio); break; case VIR_LOG_TO_SYSLOG: #if HAVE_SYSLOG_H - ret = virLogAddOutputToSyslog(prio, tokens[2]); + ret = virLogNewOutputToSyslog(prio, tokens[2]); #endif break; case VIR_LOG_TO_FILE: if (virFileAbsPath(tokens[2], &abspath) < 0) goto cleanup; - ret = virLogAddOutputToFile(prio, abspath); + ret = virLogNewOutputToFile(prio, abspath); VIR_FREE(abspath); break; case VIR_LOG_TO_JOURNALD: #if USE_JOURNALD - ret = virLogAddOutputToJournald(prio); + ret = virLogNewOutputToJournald(prio); #endif break; case VIR_LOG_TO_OUTPUT_LAST: -- 2.4.11

Same as for outputs, prepare for the fact, that defining is going to be done for the whole set of filters, rather than defining each filter individually. --- src/libvirt_private.syms | 2 +- src/util/virlog.c | 10 +++++----- src/util/virlog.h | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1f601b1..56dfcf4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1784,9 +1784,9 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h -virLogDefineFilter; virLogDefineFilters; virLogDefineOutputs; +virLogFilterNew; virLogGetDefaultPriority; virLogGetFilters; virLogGetNbFilters; diff --git a/src/util/virlog.c b/src/util/virlog.c index ec942a3..e101484 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -250,7 +250,7 @@ virLogResetFilters(void) /** - * virLogDefineFilter: + * virLogFilterNew: * @match: the pattern to match * @priority: the priority to give to messages matching the pattern * @flags: extra flags, see virLogFilterFlags enum @@ -263,9 +263,9 @@ virLogResetFilters(void) * Returns -1 in case of failure or the filter number if successful */ int -virLogDefineFilter(const char *match, - virLogPriority priority, - unsigned int flags) +virLogFilterNew(const char *match, + virLogPriority priority, + unsigned int flags) { size_t i; int ret = -1; @@ -1243,7 +1243,7 @@ virLogParseFilter(const char *filter) if (!*ref) goto cleanup; - if (virLogDefineFilter(ref, prio, flags) < 0) + if (virLogFilterNew(ref, prio, flags) < 0) goto cleanup; ret = 0; diff --git a/src/util/virlog.h b/src/util/virlog.h index 27037a4..93456db 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -185,9 +185,9 @@ extern int virLogSetDefaultPriority(virLogPriority priority); extern void virLogSetFromEnv(void); extern int virLogSetFilters(const char *filters); extern int virLogSetOutputs(const char *outputs); -extern int virLogDefineFilter(const char *match, - virLogPriority priority, - unsigned int flags); +extern int virLogFilterNew(const char *match, + virLogPriority priority, + unsigned int flags); extern int virLogOutputNew(virLogOutputFunc f, virLogCloseFunc c, void *data, -- 2.4.11

Complementary method to virLogFilterNew. --- src/util/virlog.c | 10 ++++++++++ src/util/virlog.h | 1 + 2 files changed, 11 insertions(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 5da1af7..36fecda 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1620,3 +1620,13 @@ virLogOutputListFree(virLogOutputPtr *list, int count) virLogOutputFree(list[i]); VIR_FREE(list); } + +void +virLogFilterFree(virLogFilterPtr filter) +{ + if (!filter) + return; + + VIR_FREE(filter->match); + VIR_FREE(filter); +} diff --git a/src/util/virlog.h b/src/util/virlog.h index 4f0eea7..eec3a5d 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -196,6 +196,7 @@ extern int virLogOutputNew(virLogOutputFunc f, const char *name, unsigned int flags); extern void virLogOutputFree(virLogOutputPtr output); +extern void virLogFilterFree(virLogFilterPtr filter); extern void virLogOutputListFree(virLogOutputPtr *list, int count); /* -- 2.4.11

Now that methods to free logging related lists were introduced, put them to a use. --- src/util/virlog.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index a1f5872..7e0936c 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -239,11 +239,8 @@ virLogSetDefaultPriority(virLogPriority priority) static void virLogResetFilters(void) { - size_t i; - - for (i = 0; i < virLogNbFilters; i++) - VIR_FREE(virLogFilters[i]->match); - VIR_FREE(virLogFilters); + virLogFilterListFree(virLogFilters, virLogNbFilters); + virLogFilters = NULL; virLogNbFilters = 0; virLogFiltersSerial++; } @@ -321,12 +318,8 @@ virLogFilterNew(const char *match, static void virLogResetOutputs(void) { - size_t i; - - for (i = 0; i < virLogNbOutputs; i++) - virLogOutputFree(virLogOutputs[i]); - - VIR_FREE(virLogOutputs); + virLogOutputListFree(virLogOutputs, virLogNbOutputs); + virLogOutputs = NULL; virLogNbOutputs = 0; } -- 2.4.11

Everything has been prepared to successfully split parsing and defining logic to separate operations. --- src/libvirt_private.syms | 1 + src/util/virlog.c | 100 +++++++++++++++++++++++------------------------ src/util/virlog.h | 8 ++-- tests/virlogtest.c | 7 ++-- 4 files changed, 58 insertions(+), 58 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 608d959..5d6224e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1786,6 +1786,7 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h virLogDefineFilters; virLogDefineOutputs; +virLogFilterListFree; virLogFilterNew; virLogGetDefaultPriority; virLogGetFilters; diff --git a/src/util/virlog.c b/src/util/virlog.c index 369d7dd..0116f56 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -257,57 +257,36 @@ virLogResetFilters(void) * The filter defines a rules that will apply only to messages matching * the pattern (currently if @match is a substring of the message category) * - * Returns -1 in case of failure or the filter number if successful + * Returns a reference to a newly created filter that needs to be defined using + * virLogDefineFiltersm, or NULL in case of an error. */ -int +virLogFilterPtr virLogFilterNew(const char *match, virLogPriority priority, unsigned int flags) { - size_t i; - int ret = -1; + virLogFilterPtr ret = NULL; char *mdup = NULL; - virLogFilterPtr filter = NULL; - virCheckFlags(VIR_LOG_STACK_TRACE, -1); - - if (virLogInitialize() < 0) - return -1; + virCheckFlags(VIR_LOG_STACK_TRACE, NULL); if ((match == NULL) || (priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) - return -1; - - virLogLock(); - for (i = 0; i < virLogNbFilters; i++) { - if (STREQ(virLogFilters[i]->match, match)) { - virLogFilters[i]->priority = priority; - ret = i; - goto cleanup; - } - } + return NULL; if (VIR_STRDUP_QUIET(mdup, match) < 0) - goto cleanup; + return NULL; - if (VIR_ALLOC_QUIET(filter) < 0) { + if (VIR_ALLOC_QUIET(ret) < 0) { VIR_FREE(mdup); - goto cleanup; + return NULL; } - filter->match = mdup; - filter->priority = priority; - filter->flags = flags; - - if (VIR_APPEND_ELEMENT_QUIET(virLogFilters, virLogNbFilters, filter) < 0) - goto cleanup; + ret->match = mdup; + ret->priority = priority; + ret->flags = flags; - virLogFiltersSerial++; - cleanup: - virLogUnlock(); - if (ret < 0) - virReportOOMError(); - return virLogNbFilters; + return ret; } /** @@ -1217,10 +1196,10 @@ virLogParseOutputs(const char *src, virLogOutputPtr **outputs) } -static int +static virLogFilterPtr virLogParseFilter(const char *filter) { - int ret = -1; + virLogFilterPtr ret = NULL; size_t count = 0; virLogPriority prio; char **tokens = NULL; @@ -1228,12 +1207,12 @@ virLogParseFilter(const char *filter) char *ref = NULL; if (!filter) - return -1; + return NULL; VIR_DEBUG("filter=%s", filter); if (!(tokens = virStringSplitCount(filter, ":", 0, &count))) - return -1; + return NULL; if (count != 2) goto cleanup; @@ -1251,12 +1230,11 @@ virLogParseFilter(const char *filter) if (!*ref) goto cleanup; - if (virLogFilterNew(ref, prio, flags) < 0) + if (!(ret = virLogFilterNew(ref, prio, flags))) goto cleanup; - ret = 0; cleanup: - if (ret < 0) + if (!ret) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse and define log filter %s"), filter); virStringFreeList(tokens); @@ -1282,19 +1260,21 @@ virLogParseFilter(const char *filter) * Returns the number of filter parsed or -1 in case of error. */ int -virLogParseFilters(const char *filters) +virLogParseFilters(const char *src, virLogFilterPtr **filters) { int ret = -1; - int count = 0; + size_t count = 0; size_t i; char **strings = NULL; + virLogFilterPtr filter = NULL; + virLogFilterPtr *list = NULL; - if (!filters) + if (!src) return -1; - VIR_DEBUG("filters=%s", filters); + VIR_DEBUG("filters=%s", src); - if (!(strings = virStringSplit(filters, " ", 0))) + if (!(strings = virStringSplit(src, " ", 0))) goto cleanup; for (i = 0; strings[i]; i++) { @@ -1302,14 +1282,22 @@ virLogParseFilters(const char *filters) if (STREQ(strings[i], "")) continue; - if (virLogParseFilter(strings[i]) < 0) + if (!(filter = virLogParseFilter(strings[i]))) goto cleanup; - count++; + if (VIR_APPEND_ELEMENT(list, count, filter)) { + virLogFilterFree(filter); + goto cleanup; + } + + virLogFilterFree(filter); } ret = count; + *filters = list; cleanup: + if (ret < 0) + virLogFilterListFree(list, count); virStringFreeList(strings); return ret; } @@ -1516,12 +1504,22 @@ int virLogSetFilters(const char *filters) { int ret = -1; + int count = 0; + virLogFilterPtr *list = NULL; if (virLogInitialize() < 0) return -1; - ret = virLogParseFilters(filters); + if ((count = virLogParseFilters(filters, &list)) < 0) + goto cleanup; + if (virLogDefineFilters(list, count) < 0) + goto cleanup; + + ret = count; + cleanup: + if (ret < 0) + virLogFilterListFree(list, count); return ret; } @@ -1599,9 +1597,9 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters) return -1; virLogLock(); - virLogResetOutputs(); + virLogResetFilters(); virLogFilters = filters; - virLogNbOutputs = nfilters; + virLogNbFilters = nfilters; virLogFiltersSerial++; virLogUnlock(); diff --git a/src/util/virlog.h b/src/util/virlog.h index 9b9f643..0102489 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -185,9 +185,9 @@ extern int virLogSetDefaultPriority(virLogPriority priority); extern void virLogSetFromEnv(void); extern int virLogSetFilters(const char *filters); extern int virLogSetOutputs(const char *outputs); -extern int virLogFilterNew(const char *match, - virLogPriority priority, - unsigned int flags); +extern virLogFilterPtr virLogFilterNew(const char *match, + virLogPriority priority, + unsigned int flags); extern virLogOutputPtr virLogOutputNew(virLogOutputFunc f, virLogCloseFunc c, void *data, @@ -207,7 +207,7 @@ extern void virLogLock(void); extern void virLogUnlock(void); extern int virLogReset(void); extern int virLogParseDefaultPriority(const char *priority); -extern int virLogParseFilters(const char *filters); +extern int virLogParseFilters(const char *src, virLogFilterPtr **filters); extern int virLogParseOutputs(const char *src, virLogOutputPtr **outputs); extern int virLogPriorityFromSyslog(int priority); extern void virLogMessage(virLogSourcePtr source, diff --git a/tests/virlogtest.c b/tests/virlogtest.c index 02613f5..047013f 100644 --- a/tests/virlogtest.c +++ b/tests/virlogtest.c @@ -79,10 +79,11 @@ static int testLogParseFilters(const void *opaque) { int ret = -1; - int nfilters; + int nfilters = -1; + virLogFilterPtr *list = NULL; const struct testLogData *data = opaque; - nfilters = virLogParseFilters(data->str); + nfilters = virLogParseFilters(data->str, &list); if (nfilters < 0) { if (!data->pass) { VIR_TEST_DEBUG("Got expected error: %s\n", @@ -102,7 +103,7 @@ testLogParseFilters(const void *opaque) ret = 0; cleanup: - virLogReset(); + virLogFilterListFree(list, nfilters); return ret; } -- 2.4.11

Start using the method that was introduced and described by commit 29d0068b. --- src/util/virlog.c | 83 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 204372f..62533b1 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -344,10 +344,17 @@ virLogResetOutputs(void) void virLogOutputFree(virLogOutputPtr output) { + virLogOutputPtr tmp = NULL; + if (!output) return; - if (output->c) + /* @output shall be closed only if such output does not already exist + * in the global list of outputs or @output itself is part of the global + * list of outputs + */ + tmp = virLogOutputExists(output->dest, output->name); + if (output->c && (!tmp || tmp == output)) output->c(output->data); VIR_FREE(output->name); VIR_FREE(output); @@ -733,16 +740,21 @@ virLogNewOutputToFile(virLogPriority priority, const char *file) { int fd; - virLogOutputPtr ret = NULL; + virLogOutputPtr gref, ret = NULL; - fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR); - if (fd < 0) - return NULL; + if ((gref = virLogOutputExists(VIR_LOG_TO_FILE, file))) { + fd = (intptr_t) gref->data; + } else { + fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR); + if (fd < 0) + return NULL; + } if (!(ret = virLogOutputNew(virLogOutputToFd, virLogCloseFd, (void *)(intptr_t)fd, priority, VIR_LOG_TO_FILE, file))) { - VIR_LOG_CLOSE(fd); + if (!gref) + VIR_LOG_CLOSE(fd); return NULL; } return ret; @@ -818,21 +830,39 @@ static virLogOutputPtr virLogNewOutputToSyslog(virLogPriority priority, const char *ident) { - virLogOutputPtr ret = NULL; - - /* - * ident needs to be kept around on Solaris + virLogOutputPtr gref, ret = NULL; + + /* syslog suffers from several issues: + * 1) Every other call to openlog would be a NOOP, but we can't close the + * existing connection just yet, because we're not holding the lock and + * the output cannot be changed until we're sure nothing can go wrong and + * we just swap our new list of outputs with the old global one. + * + * 2) Syslog keeps the open file descriptor private, so we can't just copy + * it like we do it with files if an output to syslog already exists and + * even if it did, there is no other way than to issue openlog if user + * wants to change ident. + * + * Therefore, dealing with syslog has to be special-cased and postponed + * until the very last moment. */ - VIR_FREE(current_ident); - if (VIR_STRDUP(current_ident, ident) < 0) - return NULL; + if (!(gref = virLogOutputExists(VIR_LOG_TO_SYSLOG, NULL))) { + /* + * rather than copying @ident, syslog uses caller's reference instead + */ + VIR_FREE(current_ident); + if (VIR_STRDUP(current_ident, ident) < 0) + return NULL; + + openlog(current_ident, 0, 0); + } - openlog(current_ident, 0, 0); if (!(ret = virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog, - NULL, priority, VIR_LOG_TO_SYSLOG, - ident))) { - closelog(); - VIR_FREE(current_ident); + NULL, priority, VIR_LOG_TO_SYSLOG, ident))) { + if (!gref) { + closelog(); + VIR_FREE(current_ident); + } return NULL; } return ret; @@ -1043,19 +1073,22 @@ static void virLogCloseJournald(void *data ATTRIBUTE_UNUSED) static virLogOutputPtr virLogNewOutputToJournald(int priority) { - virLogOutputPtr ret = NULL; + virLogOutputPtr gref, ret = NULL; - if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) - return NULL; - if (virSetInherit(journalfd, false) < 0) { - VIR_LOG_CLOSE(journalfd); - return NULL; + if (!(gref = virLogOutputExists(VIR_LOG_TO_JOURNALD, NULL))) { + if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) + return NULL; + if (virSetInherit(journalfd, false) < 0) { + VIR_LOG_CLOSE(journalfd); + return NULL; + } } if (!(ret = virLogOutputNew(virLogOutputToJournald, virLogCloseJournald, NULL, priority, VIR_LOG_TO_JOURNALD, NULL))) { - VIR_LOG_CLOSE(journalfd); + if (!gref) + VIR_LOG_CLOSE(journalfd); return NULL; } -- 2.4.11

Actually perform the swap between the active set of outputs and the user-provided copy. Deallocation ensures that no file descriptors that should not have been closed are not closed and all descriptors that should be closed are closed properly. --- src/util/virlog.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index a20cde4..8e14a9e 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1641,22 +1641,29 @@ virLogSetOutputs(const char *outputs) * @outputs: new set of outputs to be defined * @noutputs: number of outputs in @outputs * - * Resets any existing set of outputs and defines a completely new one. + * Swaps any existing set of outputs with a completely new one. * * Returns number of outputs successfully defined or -1 in case of error; */ int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs) { + size_t count; + virLogOutputPtr *tmp = NULL; + if (virLogInitialize() < 0) return -1; virLogLock(); - virLogResetOutputs(); + /* swap the currently defined list with a new copy and */ + tmp = virLogOutputs; + count = virLogNbOutputs; virLogOutputs = outputs; virLogNbOutputs = noutputs; virLogUnlock(); + virLogOutputListFree(tmp, count); + return virLogNbOutputs; } -- 2.4.11

The source update is only triggered when a logging filter gets changed (by incrementing virLogSerial counter), but if we also want to enable remote global priority tuning, any change to priority needs to trigger the source update as well. --- src/util/virlog.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index a837e17..769dcec 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -265,6 +265,7 @@ virLogSetDefaultPriority(virLogPriority priority) return -1; virLogDefaultPriority = priority; + virLogSerial++; return 0; } -- 2.4.11

Now that a lock to serialize setters has been introduced, make use of it and lock every log setting API. --- src/util/virlog.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 6aa9c91..240d6e3 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -279,8 +279,10 @@ virLogSetDefaultPriority(virLogPriority priority) if (virLogInitialize() < 0) return -1; + virLogAPILock(); virLogDefaultPriority = priority; virLogSerial++; + virLogAPIUnlock(); return 0; } @@ -1592,6 +1594,8 @@ virLogSetFilters(const char *filters) if (virLogInitialize() < 0) return -1; + virLogAPILock(); + if ((count = virLogParseFilters(filters, &list)) < 0) goto cleanup; @@ -1600,6 +1604,7 @@ virLogSetFilters(const char *filters) ret = count; cleanup: + virLogAPIUnlock(); if (ret < 0) virLogFilterListFree(list, count); return ret; @@ -1627,6 +1632,8 @@ virLogSetOutputs(const char *outputs) if (virLogInitialize() < 0) return -1; + virLogAPILock(); + if ((count = virLogParseOutputs(outputs, &list)) < 0) goto cleanup; @@ -1646,6 +1653,7 @@ virLogSetOutputs(const char *outputs) ret = count; cleanup: + virLogAPIUnlock(); if (ret < 0) virLogOutputListFree(list, count); return ret; -- 2.4.11

Enable libvirt users to query logging filter settings. --- daemon/admin.c | 45 +++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 ++++++++++++++- src/admin/admin_remote.c | 43 +++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 40 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 158 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 5129e2d..ac81b82 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -304,4 +304,49 @@ adminConnectGetLoggingLevel(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return virLogGetDefaultPriority(); } + +static int +adminConnectGetLoggingFilters(char **filters, unsigned int flags) +{ + char *tmp = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(tmp = virLogGetFilters())) + return -1; + + ret = virLogGetNbFilters(); + + *filters = tmp; + return ret; +} + +static int +adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + admin_connect_get_logging_filters_args *args, + admin_connect_get_logging_filters_ret *ret) +{ + char *filters = NULL; + int nfilters = 0; + int rv = -1; + + if ((nfilters = adminConnectGetLoggingFilters(&filters, args->flags)) < 0) + goto cleanup; + + if (VIR_STRDUP(ret->filters, filters) < 0) + goto cleanup; + + ret->nfilters = nfilters; + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + VIR_FREE(filters); + return rv; +} #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 2878c78..87616d0 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -351,6 +351,10 @@ int virAdmClientGetInfo(virAdmClientPtr client, int virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, unsigned int flags); +int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, + char **filters, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 6a95665..05eecec 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -169,6 +169,15 @@ struct admin_connect_get_logging_level_ret { int level; }; +struct admin_connect_get_logging_filters_args { + unsigned int flags; +}; + +struct admin_connect_get_logging_filters_ret { + admin_nonnull_string filters; + unsigned int nfilters; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -244,5 +253,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 11 + ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 11, + + /** + * @generate: none + */ + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 12 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 40fcddb..86deaec 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -355,3 +355,46 @@ remoteAdminClientGetInfo(virAdmClientPtr client, virObjectUnlock(priv); return rv; } + +static int +remoteAdminConnectGetLoggingFilters(virAdmConnectPtr conn, + char **filters, + unsigned int flags) +{ + int rv = -1; + char *tmp_filters = NULL; + remoteAdminPrivPtr priv = conn->privateData; + admin_connect_get_logging_filters_args args; + admin_connect_get_logging_filters_ret ret; + + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + virObjectLock(priv); + + if (call(conn, + 0, + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS, + (xdrproc_t) xdr_admin_connect_get_logging_filters_args, + (char *) &args, + (xdrproc_t) xdr_admin_connect_get_logging_filters_ret, + (char *) &ret) == -1) + goto done; + + if (filters) { + if (VIR_STRDUP(tmp_filters, ret.filters) < 0) + goto cleanup; + + *filters = tmp_filters; + tmp_filters = NULL; + } + + rv = ret.nfilters; + + cleanup: + xdr_free((xdrproc_t) xdr_admin_connect_get_logging_filters_ret, (char *) &ret); + + done: + virObjectUnlock(priv); + return rv; +} diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 72d93ec..bbe4a5a 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -112,6 +112,13 @@ struct admin_connect_get_logging_level_args { struct admin_connect_get_logging_level_ret { int level; }; +struct admin_connect_get_logging_filters_args { + u_int flags; +}; +struct admin_connect_get_logging_filters_ret { + admin_nonnull_string filters; + u_int nfilters; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -124,4 +131,5 @@ enum admin_procedure { ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9, ADMIN_PROC_CLIENT_GET_INFO = 10, ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 11, + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 12, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 898b9c1..e61dc10 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -1009,3 +1009,43 @@ virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectGetLoggingFilters: + * @conn: pointer to an active admin connection + * @filters: pointer to a variable to store a string containing all currently + * defined logging filters on daemon (allocated automatically) + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Retrieves a list of currently installed logging filters. Filters returned + * are contained within an automatically allocated string and delimited by + * spaces. The format of each filter conforms to the format described in + * daemon's configuration file (e.g. libvirtd.conf). + * To retrieve individual filters, additional parsing needs to be done by the + * caller. Caller is also responsible for freeing @filters correctly. + * + * Returns the number of filters returned in @filters, or -1 in case of + * an error. + */ +int +virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, + char **filters, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, flags=%x", conn, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + virCheckNonNullArgGoto(filters, error); + + if ((ret = remoteAdminConnectGetLoggingFilters(conn, filters, + flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 8534ab1..6029575 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,8 @@ xdr_admin_client_get_info_args; xdr_admin_client_get_info_ret; xdr_admin_connect_get_lib_version_ret; +xdr_admin_connect_get_logging_filters_args; +xdr_admin_connect_get_logging_filters_ret; xdr_admin_connect_get_logging_level_args; xdr_admin_connect_get_logging_level_ret; xdr_admin_connect_list_servers_args; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 4cc0a5b..d7648a3 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -27,6 +27,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectUnregisterCloseCallback; virAdmConnectListServers; virAdmConnectGetLoggingLevel; + virAdmConnectGetLoggingFilters; virAdmServerGetName; virAdmServerGetThreadPoolParameters; virAdmServerFree; -- 2.4.11

Enable libvirt users to set logging level of a daemon from outside. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin.c | 10 ++++++++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 12 +++++++++++- src/admin_protocol-structs | 5 +++++ src/libvirt-admin.c | 35 +++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + src/util/virlog.c | 2 -- 8 files changed, 67 insertions(+), 3 deletions(-) diff --git a/daemon/admin.c b/daemon/admin.c index 5f7a957..a2d35df 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -323,6 +323,16 @@ adminConnectGetLoggingFilters(char **filters, unsigned int flags) } static int +adminConnectSetLoggingLevel(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + unsigned int priority, + unsigned int flags) +{ + virCheckFlags(0, -1); + + return virLogSetDefaultPriority(priority); +} + +static int adminConnectGetLoggingOutputs(char **outputs, unsigned int flags) { char *tmp = NULL; diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 755e6a6..41a9e2b 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -359,6 +359,10 @@ int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, char **outputs, unsigned int flags); +int virAdmConnectSetLoggingLevel(virAdmConnectPtr conn, + unsigned int level, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 5465efe..d20c9d5 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -187,6 +187,11 @@ struct admin_connect_get_logging_outputs_ret { unsigned int noutputs; }; +struct admin_connect_set_logging_level_args { + unsigned int level; + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -272,5 +277,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 13 + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 13, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 14 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 7cf2421..6620fb3 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -126,6 +126,10 @@ struct admin_connect_get_logging_outputs_ret { admin_nonnull_string outputs; u_int noutputs; }; +struct admin_connect_set_logging_level_args { + u_int level; + u_int flags; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -140,4 +144,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 11, ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 12, ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 13, + ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 14, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 95b47c0..7ac72d1 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -1088,3 +1088,38 @@ virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectSetLoggingLevel: + * @conn: pointer to an active admin connection + * @level: desired logging level, valid values are (see virLogPriority): + * 1) VIR_LOG_DEBUG + * 2) VIR_LOG_INFO + * 3) VIR_LOG_WARNING + * 4) VIR_LOG_ERROR + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Set the current global logging level to @level. + * + * Returns 0 on success, -1 on error. + */ +int +virAdmConnectSetLoggingLevel(virAdmConnectPtr conn, + unsigned int level, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, level=%u, flags=%x", conn, level, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + + if ((ret = remoteAdminConnectSetLoggingLevel(conn, level, flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index ac183c6..28ff81c 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -20,6 +20,7 @@ xdr_admin_connect_list_servers_ret; xdr_admin_connect_lookup_server_args; xdr_admin_connect_lookup_server_ret; xdr_admin_connect_open_args; +xdr_admin_connect_set_logging_level_args; xdr_admin_server_get_threadpool_parameters_args; xdr_admin_server_get_threadpool_parameters_ret; xdr_admin_server_list_clients_args; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 68475b1..a8b51c3 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -37,4 +37,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerSetThreadPoolParameters; virAdmServerListClients; virAdmClientGetInfo; + virAdmConnectSetLoggingLevel; }; diff --git a/src/util/virlog.c b/src/util/virlog.c index 240d6e3..857a326 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -276,8 +276,6 @@ virLogSetDefaultPriority(virLogPriority priority) "invalid"), priority); return -1; } - if (virLogInitialize() < 0) - return -1; virLogAPILock(); virLogDefaultPriority = priority; -- 2.4.11

Enable libvirt users to modify daemon's logging output settings from outside. --- daemon/admin.c | 10 ++++++++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 12 +++++++++++- src/admin_protocol-structs | 5 +++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + 7 files changed, 68 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 2a0a9ab..acffb44 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -360,6 +360,16 @@ adminConnectSetLoggingFilters(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, } static int +adminConnectSetLoggingOutputs(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *outputs, + unsigned int flags) +{ + virCheckFlags(0, -1); + + return virLogSetOutputs(outputs); +} + +static int adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 6be649a..6fe5732 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -367,6 +367,10 @@ int virAdmConnectSetLoggingFilters(virAdmConnectPtr conn, const char *filters, unsigned int flags); +int virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn, + const char *outputs, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 80de75a..f64b2ed 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -197,6 +197,11 @@ struct admin_connect_set_logging_filters_args { unsigned int flags; }; +struct admin_connect_set_logging_outputs_args { + admin_nonnull_string outputs; + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -292,5 +297,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 15 + ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 15, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 000cef9..65439a0 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -134,6 +134,10 @@ struct admin_connect_set_logging_filters_args { admin_nonnull_string filters; u_int flags; }; +struct admin_connect_set_logging_outputs_args { + admin_nonnull_string outputs; + u_int flags; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -150,4 +154,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 13, ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 14, ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 15, + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index fedd4a6..6afbe45 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -1159,3 +1159,39 @@ virAdmConnectSetLoggingFilters(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectSetLoggingOutputs: + * @conn: pointer to an active admin connection + * @outputs: pointer to a string containing a list of outputs to be defined + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Redefine the existing (set of) outputs(s) with a new one specified in + * @outputs. If multiple outputs are specified, they need to be delimited by + * spaces. The format of each output must conform to the format described in + * daemon's configuration file (e.g. libvirtd.conf). + * + * Returns 0 if the new output or the set of outputs has been defined + * successfully, or -1 in case of an error. + */ +int +virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn, + const char *outputs, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, outputs=%s, flags=%x", conn, outputs, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + virCheckNonNullArgGoto(outputs, error); + + if ((ret = remoteAdminConnectSetLoggingOutputs(conn, outputs, flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 2d72a86..8208358 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -22,6 +22,7 @@ xdr_admin_connect_lookup_server_ret; xdr_admin_connect_open_args; xdr_admin_connect_set_logging_filters_args; xdr_admin_connect_set_logging_level_args; +xdr_admin_connect_set_logging_outputs_args; xdr_admin_server_get_threadpool_parameters_args; xdr_admin_server_get_threadpool_parameters_ret; xdr_admin_server_list_clients_args; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index ffc8606..b82bda8 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -39,4 +39,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmClientGetInfo; virAdmConnectSetLoggingLevel; virAdmConnectSetLoggingFilters; + virAdmConnectSetLoggingOutputs; }; -- 2.4.11

Finally, now that all APIs have been introduced, wire them up to virt-admin and introduce dmn-log-info and dmn-log-define commands. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 204 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 204 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index d6f7084..1ec139a 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -742,6 +742,198 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) VIR_FREE(timestr); return ret; } + +/* ------------------- + * Command dmn-log-info + * ------------------- + */ +static const vshCmdInfo info_dmn_log_info[] = { + {.name = "help", + .data = N_("view daemon's current logging settings") + }, + {.name = "desc", + .data = N_("Returns all currently active logging settings on daemon. " + "These include global logging level, logging filters and " + "logging outputs.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_dmn_log_info[] = { + {.name = "daemon", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("name of the daemon to query information from") + }, + {.name = "outputs", + .type = VSH_OT_BOOL, + .help = N_("query logging outputs") + }, + {.name = "filters", + .type = VSH_OT_BOOL, + .help = N_("query logging filters") + }, + {.name = "level", + .type = VSH_OT_BOOL, + .help = N_("query logging level") + }, + {.name = NULL} +}; + +static bool +cmdDmnLogInfo(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + bool optOutputs = vshCommandOptBool(cmd, "outputs"); + bool optFilters = vshCommandOptBool(cmd, "filters"); + bool optLevel = vshCommandOptBool(cmd, "level"); + bool all = optOutputs + optFilters + optLevel == 0; + int level, nfilters, noutputs; + char *filters, *outputs; + const char *levelstr = NULL; + vshAdmControlPtr priv = ctl->privData; + + if (optLevel || all) { + if ((level = virAdmConnectGetLoggingLevel(priv->conn, 0)) < 0) + goto cleanup; + + switch ((virLogPriority) level) { + case VIR_LOG_DEBUG: + levelstr = "debug"; + break; + case VIR_LOG_INFO: + levelstr = "info"; + break; + case VIR_LOG_WARN: + levelstr = "warning"; + break; + case VIR_LOG_ERROR: + levelstr = "error"; + break; + default: + vshError(ctl, _("Remote side returned a logging level this " + "version of library does not support")); + goto cleanup; + } + } + + if (optFilters || all) { + if ((nfilters = virAdmConnectGetLoggingFilters(priv->conn, + &filters, 0)) < 0) { + vshError(ctl, _("Unable to get daemon logging filters information")); + goto cleanup; + } + } + + if (optOutputs || all) { + if ((noutputs = virAdmConnectGetLoggingOutputs(priv->conn, + &outputs, 0)) < 0) { + vshError(ctl, _("Unable to get daemon logging outputs information")); + goto cleanup; + } + } + + if (optLevel || all) { + vshPrintExtra(ctl, " %-15s", _("Logging level: ")); + vshPrint(ctl, "%s\n", levelstr); + } + + if (optFilters || all) { + vshPrintExtra(ctl, " %-15s", _("Logging filters: ")); + vshPrint(ctl, "%s\n", filters); + } + + if (optOutputs || all) { + vshPrintExtra(ctl, " %-15s", _("Logging outputs: ")); + vshPrint(ctl, "%s\n", outputs); + } + + ret = true; + cleanup: + return ret; +} + +/* ---------------------- + * Command dmn-log-define + * ---------------------- + */ +static const vshCmdInfo info_dmn_log_define[] = { + {.name = "help", + .data = N_("change daemon's logging settings") + }, + {.name = "desc", + .data = N_("Defines and installs a new set of logging settings on a daemon. " + "These include global logging level, logging filters and " + "logging outputs.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_dmn_log_define[] = { + {.name = "daemon", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("name of the daemon the logging settings of which should be changed ") + }, + {.name = "outputs", + .type = VSH_OT_STRING, + .help = N_("comma separated list of logging outputs") + }, + {.name = "filters", + .type = VSH_OT_STRING, + .help = N_("comma separated list of logging filters") + }, + {.name = "level", + .type = VSH_OT_INT, + .help = N_("logging level") + }, + {.name = NULL} +}; + +static bool +cmdDmnLogDefine(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + const char *filters = NULL; + const char *outputs = NULL; + unsigned int level = 0; + bool optOutputs = vshCommandOptBool(cmd, "outputs"); + bool optFilters = vshCommandOptBool(cmd, "filters"); + bool optLevel = vshCommandOptBool(cmd, "level"); + vshAdmControlPtr priv = ctl->privData; + + if (!(optOutputs + optFilters + optLevel)) { + vshError(ctl, _("At least one of options --outputs, --filters, " + "--level is mandatory")); + goto cleanup; + } + + if (optLevel && + (vshCommandOptUInt(ctl, cmd, "level", &level) < 0 || + virAdmConnectSetLoggingLevel(priv->conn, level, 0) < 0)) { + vshError(ctl, _("Unable to change daemon logging settings")); + goto cleanup; + } + + if (optFilters && + (vshCommandOptStringReq(ctl, cmd, "filters", &filters) < 0 || + virAdmConnectSetLoggingFilters(priv->conn, filters, 0) < 0)) { + vshError(ctl, _("Unable to change daemon logging settings")); + goto cleanup; + } + + if (optOutputs && + (vshCommandOptStringReq(ctl, cmd, "outputs", &outputs) < 0 || + virAdmConnectSetLoggingOutputs(priv->conn, outputs, 0) < 0)) { + vshError(ctl, _("Unable to change daemon logging settings")); + goto cleanup; + } + + ret = true; + cleanup: + return ret; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -1059,6 +1251,12 @@ static const vshCmdDef monitoringCmds[] = { .info = info_client_info, .flags = 0 }, + {.name = "dmn-log-info", + .handler = cmdDmnLogInfo, + .opts = opts_dmn_log_info, + .info = info_dmn_log_info, + .flags = 0 + }, {.name = NULL} }; @@ -1069,6 +1267,12 @@ static const vshCmdDef managementCmds[] = { .info = info_srv_threadpool_set, .flags = 0 }, + {.name = "dmn-log-define", + .handler = cmdDmnLogDefine, + .opts = opts_dmn_log_define, + .info = info_dmn_log_define, + .flags = 0 + }, {.name = NULL} }; -- 2.4.11