[libvirt] [PATCH 00/38] Introduce APIs to change daemon's logging settings
This series implements administration APIs to modify daemon's logging settings which include priority, filters, and outputs. It also performs all necessary changes to enable this feature. Patches also involve some slight refactors to the existing code as well as wiring up the APIs to virt-admin client. The major change in internal code is how definition of filters and outputs is achieved. Until now each filter/output has been defined and appended to the global set of filters/outputs individually whilst holding a lock that was repeatedly released after each filter/output. If modification to filters and outputs should be exported through public APIs, all these setter operations must be atomic, i.e. accomplished as a unit, not gradually. For filters and priority, these are replaced normally, i.e. reset the original and replace it with the new copy while having the lock. For outputs, there was a slight issue, since resetting the global set of outputs would result in closing all the file descriptors, which might not be exactly what we want, since the new set of outputs that is meant to replace the original set might include outputs that are also contained within the original set. The logic for such outputs is following: Each file-based output that already exists in the original set and should continue existing in the new set will copy the existing FD from the global set. For each syslog-based output, closelog and openlog must be called anyway (because syslog keeps its FD private), but will be called at the very last moment if such an output already exists to prevent from changing before the lock for unit replaced is acquired, and thus breaking the atomicity that we want. For journald-based outputs, the FD is global, we just need to prevent it from being closed if such an output shall continue to exist. All outputs that need to continue to exist are prevented from closing when being destroyed-deallocated. On the other hand, all outputs that do not exist in the new set of outputs shall be closed correctly. Erik Skultety (38): virlog: Return void instead of int in virLogReset<Foo> methods virlog: Convert virLogOutputs to a list of pointers to outputs virlog: Convert virLogFilters to a list of pointers to filters virlog: Export virLogOutputPtr through header virlog: Export virLogFilterPtr through header virlog: Introduce virLogSetFilters virlog: Introduce virLogSetOutputs daemon: Replace virLogParseOutputs with virLogSetOutputs in callers daemon: Replace virLogParseFilters with virLogSetFilters in callers virlog: Introduce virLogDefineOutputs virlog: Introduce virLogDefineFilters virlog: Rename virLogAddOutputTo to virLogNewOutput virlog: Rename virLogDefineOutput to virLogOutputNew virlog: Rename virLogDefineFilter to virLogFilterNew virlog: Introduce virLogOutputFree virlog: Introduce virLogOutputListFree virlog: Introduce virLogFilterFree virlog: Introduce virLogFilterListFree virlog: Make virLogReset methods use of virLog(Output|Filter)ListFree virlog: Split output parsing and output defining to separate operations virlog: Split filter parsing and filter defining to separate operations virlog: Split parsing and setting priority virlog: Introduce virLogOutputExists virlog: Make use of virLogOutputExists virlog: Take a special care of syslog when setting new set of log outputs virlog: Swap the new copy of outputs with the global existing one virlog: Rename virLogFiltersSerial to virLogSerial virlog: Make virLogSetDefaultPriority trigger source update as well virlog: Introduce an API mutex that serializes all setters virlog: Acquire virLogAPILock in each setter API admin: Introduce virAdmConnectGetLoggingLevel admin: Introduce virAdmConnectGetLoggingFilters admin: Introduce virAdmConnectGetLoggingOutputs admin: Introduce virAdmConnectSetLoggingLevel admin: Introduce virAdmConnectSetLoggingFilters admin: Introduce virAdmConnectSetLoggingOutputs admin: Export logging level constants via libvirt-admin.h virt-admin: Wire-up the logging APIs daemon/admin.c | 125 +++++++ daemon/libvirtd.c | 8 +- include/libvirt/libvirt-admin.h | 37 +++ src/admin/admin_protocol.x | 73 ++++- src/admin/admin_remote.c | 86 +++++ src/admin_protocol-structs | 38 +++ src/libvirt-admin.c | 219 +++++++++++++ src/libvirt_admin_private.syms | 9 + src/libvirt_admin_public.syms | 6 + src/libvirt_private.syms | 14 +- src/locking/lock_daemon.c | 8 +- src/logging/log_daemon.c | 8 +- src/util/virlog.c | 698 +++++++++++++++++++++++++++------------- src/util/virlog.h | 50 +-- tests/eventtest.c | 3 +- tests/testutils.c | 19 +- tests/virlogtest.c | 12 +- tools/virt-admin.c | 208 ++++++++++++ 18 files changed, 1361 insertions(+), 260 deletions(-) -- 2.4.3
In this particular case, reset is meant as clearing the whole list of outputs/filters, not resetting it to a predefined default setting. Looking at it from that perspective, returning the number of records removed doesn't help the caller in any way (not that any of the callers would actually check for it). Well, callers could detect an error from the number of successfully removed records, but the only thing that can fail in virLogReset is force closing a file descriptor in which case the error isn't propagated back to virLogReset anyway. Conclusion: there is no practical use for having a return type of 'int' rather than 'void' in this case. --- 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 738eaac..6d11328 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -117,8 +117,8 @@ static int virLogNbOutputs; */ static virLogPriority virLogDefaultPriority = VIR_LOG_DEFAULT; -static int virLogResetFilters(void); -static int virLogResetOutputs(void); +static void virLogResetFilters(void); +static void virLogResetOutputs(void); static void virLogOutputToFd(virLogSourcePtr src, virLogPriority priority, const char *filename, @@ -239,10 +239,8 @@ virLogSetDefaultPriority(virLogPriority priority) * virLogResetFilters: * * Removes the set of logging filters defined. - * - * Returns the number of filters removed */ -static int +static void virLogResetFilters(void) { size_t i; @@ -252,7 +250,6 @@ virLogResetFilters(void) VIR_FREE(virLogFilters); virLogNbFilters = 0; virLogFiltersSerial++; - return i; } @@ -319,10 +316,8 @@ virLogDefineFilter(const char *match, * virLogResetOutputs: * * Removes the set of logging output defined. - * - * Returns the number of output removed */ -static int +static void virLogResetOutputs(void) { size_t i; @@ -333,9 +328,7 @@ virLogResetOutputs(void) VIR_FREE(virLogOutputs[i].name); } VIR_FREE(virLogOutputs); - i = virLogNbOutputs; virLogNbOutputs = 0; - return i; } -- 2.4.3
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.3
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.3
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.3
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.3
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 684f06c..bd3f72d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1757,6 +1757,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.3
This API is the entry point to output modification of the logger. Currently, everything is done by virLogParseOutputs which should do parsing only and a separate method which will do the definition part should be added. --- 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 bd3f72d..99b3b55 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1759,6 +1759,7 @@ virLogReset; virLogSetDefaultPriority; virLogSetFilters; virLogSetFromEnv; +virLogSetOutputs; virLogUnlock; virLogVMessage; diff --git a/src/util/virlog.c b/src/util/virlog.c index 0aa722d..2cd3773 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1516,3 +1516,26 @@ virLogSetFilters(const char *filters) return ret; } + + +/** + * virLogSetOutputs: + * @outputs: string defining a (set of) output(s) + * + * Replaces the current set of defined outputs with a new set of outputs. + * + * Returns the number of outputs successfully defined or -1 in case of an + * error. + */ +int +virLogSetOutputs(const char *outputs) +{ + int ret = -1; + + if (virLogInitialize() < 0) + return -1; + + ret = virLogParseOutputs(outputs); + + return ret; +} diff --git a/src/util/virlog.h b/src/util/virlog.h index 4d7cc98..4141ecd 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -184,6 +184,7 @@ extern virLogPriority virLogGetDefaultPriority(void); 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); -- 2.4.3
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 3d38a46..477d42d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -682,7 +682,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 @@ -709,7 +709,7 @@ daemonSetupLogging(struct daemonConfig *config, if (virAsprintf(&tmp, "%d:journald", priority) < 0) goto error; - virLogParseOutputs(tmp); + virLogSetOutputs(tmp); VIR_FREE(tmp); } } @@ -752,7 +752,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 973e691..1826fc6 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 68f0647..f43c57b 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.3
Similar to outputs, parser should do parsing only, thus the 'define' logic is going to be stripped from virLogParseFilters. Make this change transparent to all callers by using virLogSetFilters which has already been introduced earlier in this series. --- daemon/libvirtd.c | 2 +- src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/util/virlog.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 477d42d..672057f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -679,7 +679,7 @@ daemonSetupLogging(struct daemonConfig *config, virLogSetFromEnv(); if (virLogGetNbFilters() == 0) - virLogParseFilters(config->log_filters); + virLogSetFilters(config->log_filters); if (virLogGetNbOutputs() == 0) virLogSetOutputs(config->log_outputs); diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 1826fc6..e83bd40 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -476,7 +476,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, virLogSetFromEnv(); if (virLogGetNbFilters() == 0) - virLogParseFilters(config->log_filters); + virLogSetFilters(config->log_filters); if (virLogGetNbOutputs() == 0) virLogSetOutputs(config->log_outputs); diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index f43c57b..b031bb1 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -399,7 +399,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, virLogSetFromEnv(); if (virLogGetNbFilters() == 0) - virLogParseFilters(config->log_filters); + virLogSetFilters(config->log_filters); if (virLogGetNbOutputs() == 0) virLogSetOutputs(config->log_outputs); diff --git a/src/util/virlog.c b/src/util/virlog.c index 538a5b8..9059722 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1472,7 +1472,7 @@ virLogSetFromEnv(void) virLogParseDefaultPriority(debugEnv); debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_FILTERS"); if (debugEnv && *debugEnv) - virLogParseFilters(debugEnv); + virLogSetFilters(debugEnv); debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_OUTPUTS"); if (debugEnv && *debugEnv) virLogSetOutputs(debugEnv); -- 2.4.3
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 99b3b55..e97ec45 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1743,6 +1743,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.3
Prepare a method that only defines a set of filters. It takes a list of filters, preferably created by virLogParseFilters. The original set of filters is reset and replaced by the new user-provided set of filters. --- 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 e97ec45..8773270 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1742,6 +1742,7 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h virLogDefineFilter; +virLogDefineFilters; virLogDefineOutput; virLogDefineOutputs; virLogGetDefaultPriority; diff --git a/src/util/virlog.c b/src/util/virlog.c index e5923ce..60573a9 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1564,3 +1564,28 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs) return virLogNbOutputs; } + +/** + * virLogDefineFilters: + * @filters: new set of filters to be defined + * @nfilters: number of filters in @filters + * + * Resets any existing set of filters and defines a completely new one. + * + * Returns number of filters successfully defined or -1 in case of error; + */ +int +virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters) +{ + if (virLogInitialize() < 0) + return -1; + + virLogLock(); + virLogResetOutputs(); + virLogFilters = filters; + virLogNbOutputs = nfilters; + virLogFiltersSerial++; + virLogUnlock(); + + return virLogNbFilters; +} diff --git a/src/util/virlog.h b/src/util/virlog.h index 166db5a..ff16034 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -225,5 +225,6 @@ extern void virLogVMessage(virLogSourcePtr source, bool virLogProbablyLogMessage(const char *str); int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs); +int virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters); #endif -- 2.4.3
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.3
Preparation for the fact, that opposite to the original define method that took an already existing global set of outputs and appended/defined a new output, virLogOutputNew only creates a new output, but caller is responsible for appending the output to a list and managing the list itself. --- src/libvirt_private.syms | 2 +- src/util/virlog.c | 34 +++++++++++++++++----------------- src/util/virlog.h | 14 +++++++------- tests/testutils.c | 4 ++-- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8773270..62641ab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1743,7 +1743,6 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h virLogDefineFilter; virLogDefineFilters; -virLogDefineOutput; virLogDefineOutputs; virLogGetDefaultPriority; virLogGetFilters; @@ -1752,6 +1751,7 @@ virLogGetNbOutputs; virLogGetOutputs; virLogLock; virLogMessage; +virLogOutputNew; virLogParseDefaultPriority; virLogParseFilters; virLogParseOutputs; diff --git a/src/util/virlog.c b/src/util/virlog.c index 387e08a..ec942a3 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -334,7 +334,7 @@ virLogResetOutputs(void) /** - * virLogDefineOutput: + * virLogOutputNew: * @f: the function to call to output a message * @c: the function to call to close the output (or NULL) * @data: extra data passed as first arg to the function @@ -349,13 +349,13 @@ virLogResetOutputs(void) * Returns -1 in case of failure or the output number if successful */ int -virLogDefineOutput(virLogOutputFunc f, - virLogCloseFunc c, - void *data, - virLogPriority priority, - virLogDestination dest, - const char *name, - unsigned int flags) +virLogOutputNew(virLogOutputFunc f, + virLogCloseFunc c, + void *data, + virLogPriority priority, + virLogDestination dest, + const char *name, + unsigned int flags) { char *ndup = NULL; virLogOutputPtr output = NULL; @@ -716,8 +716,8 @@ virLogCloseFd(void *data) static int virLogNewOutputToStderr(virLogPriority priority) { - if (virLogDefineOutput(virLogOutputToFd, NULL, (void *)2L, priority, - VIR_LOG_TO_STDERR, NULL, 0) < 0) + if (virLogOutputNew(virLogOutputToFd, NULL, (void *)2L, priority, + VIR_LOG_TO_STDERR, NULL, 0) < 0) return -1; return 0; } @@ -732,9 +732,9 @@ virLogNewOutputToFile(virLogPriority priority, fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR); if (fd < 0) return -1; - if (virLogDefineOutput(virLogOutputToFd, virLogCloseFd, - (void *)(intptr_t)fd, - priority, VIR_LOG_TO_FILE, file, 0) < 0) { + if (virLogOutputNew(virLogOutputToFd, virLogCloseFd, + (void *)(intptr_t)fd, + priority, VIR_LOG_TO_FILE, file, 0) < 0) { VIR_FORCE_CLOSE(fd); return -1; } @@ -819,8 +819,8 @@ virLogNewOutputToSyslog(virLogPriority priority, return -1; openlog(current_ident, 0, 0); - if (virLogDefineOutput(virLogOutputToSyslog, virLogCloseSyslog, NULL, - priority, VIR_LOG_TO_SYSLOG, ident, 0) < 0) { + if (virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog, NULL, + priority, VIR_LOG_TO_SYSLOG, ident, 0) < 0) { closelog(); VIR_FREE(current_ident); return -1; @@ -1039,8 +1039,8 @@ static int virLogNewOutputToJournald(int priority) VIR_LOG_CLOSE(journalfd); return -1; } - if (virLogDefineOutput(virLogOutputToJournald, virLogCloseJournald, NULL, - priority, VIR_LOG_TO_JOURNALD, NULL, 0) < 0) { + if (virLogOutputNew(virLogOutputToJournald, virLogCloseJournald, NULL, + priority, VIR_LOG_TO_JOURNALD, NULL, 0) < 0) { return -1; } return 0; diff --git a/src/util/virlog.h b/src/util/virlog.h index ff16034..27037a4 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -188,13 +188,13 @@ extern int virLogSetOutputs(const char *outputs); extern int virLogDefineFilter(const char *match, virLogPriority priority, unsigned int flags); -extern int virLogDefineOutput(virLogOutputFunc f, - virLogCloseFunc c, - void *data, - virLogPriority priority, - virLogDestination dest, - const char *name, - unsigned int flags); +extern int virLogOutputNew(virLogOutputFunc f, + virLogCloseFunc c, + void *data, + virLogPriority priority, + virLogDestination dest, + const char *name, + unsigned int flags); /* * Internal logging API diff --git a/tests/testutils.c b/tests/testutils.c index fc4c339..4998931 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -869,8 +869,8 @@ int virtTestMain(int argc, virLogSetFromEnv(); if (!getenv("LIBVIRT_DEBUG") && !virLogGetNbOutputs()) { - if (virLogDefineOutput(virtTestLogOutput, virtTestLogClose, &testLog, - VIR_LOG_DEBUG, VIR_LOG_TO_STDERR, NULL, 0) < 0) + if (virLogOutputNew(virtTestLogOutput, virtTestLogClose, &testLog, + VIR_LOG_DEBUG, VIR_LOG_TO_STDERR, NULL, 0) < 0) return EXIT_FAILURE; } -- 2.4.3
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 62641ab..fac361b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,9 +1741,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.3
Add a complementary method to virLogOutputNew. --- src/libvirt_private.syms | 1 + src/util/virlog.c | 21 ++++++++++++++++----- src/util/virlog.h | 1 + 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fac361b..433cf60 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1751,6 +1751,7 @@ virLogGetNbOutputs; virLogGetOutputs; virLogLock; virLogMessage; +virLogOutputFree; virLogOutputNew; virLogParseDefaultPriority; virLogParseFilters; diff --git a/src/util/virlog.c b/src/util/virlog.c index e101484..e36ff73 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -323,16 +323,27 @@ 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); - } + for (i = 0; i < virLogNbOutputs; i++) + virLogOutputFree(virLogOutputs[i]); + VIR_FREE(virLogOutputs); virLogNbOutputs = 0; } +void +virLogOutputFree(virLogOutputPtr output) +{ + if (!output) + return; + + if (output->c) + output->c(output->data); + VIR_FREE(output->name); + VIR_FREE(output); + +} + /** * virLogOutputNew: * @f: the function to call to output a message diff --git a/src/util/virlog.h b/src/util/virlog.h index 93456db..7573984 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -195,6 +195,7 @@ extern int virLogOutputNew(virLogOutputFunc f, virLogDestination dest, const char *name, unsigned int flags); +extern void virLogOutputFree(virLogOutputPtr output); /* * Internal logging API -- 2.4.3
This is just a convenience method for discarding a list of outputs instead of using a 'for' loop everywhere. It is safe to pass -1 as the number of elements in the list as well as passing NULL as list reference. --- src/libvirt_private.syms | 1 + src/util/virlog.c | 20 ++++++++++++++++++++ src/util/virlog.h | 1 + 3 files changed, 22 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 433cf60..e594eaa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1752,6 +1752,7 @@ virLogGetOutputs; virLogLock; virLogMessage; virLogOutputFree; +virLogOutputListFree; virLogOutputNew; virLogParseDefaultPriority; virLogParseFilters; diff --git a/src/util/virlog.c b/src/util/virlog.c index e36ff73..5da1af7 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1600,3 +1600,23 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters) return virLogNbFilters; } + +/** + * virLogOutputsFreeList: + * @list: list of outputs to be freed + * @count: number of elements in the list + * + * Frees a list of outputs. + */ +void +virLogOutputListFree(virLogOutputPtr *list, int count) +{ + size_t i; + + if (!list || count < 0) + return; + + for (i = 0; i < count; i++) + virLogOutputFree(list[i]); + VIR_FREE(list); +} diff --git a/src/util/virlog.h b/src/util/virlog.h index 7573984..4f0eea7 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 virLogOutputListFree(virLogOutputPtr *list, int count); /* * Internal logging API -- 2.4.3
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.3
This is just a convenience method for discarding a list of filters instead of using a 'for' loop everywhere. It is safe to pass -1 as the number of elements in the list as well as passing NULL as list reference. --- src/util/virlog.c | 20 ++++++++++++++++++++ src/util/virlog.h | 1 + 2 files changed, 21 insertions(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 36fecda..a1f5872 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1630,3 +1630,23 @@ virLogFilterFree(virLogFilterPtr filter) VIR_FREE(filter->match); VIR_FREE(filter); } + +/** + * virLogFilterFreeList: + * @list: list of filters to be freed + * @count: number of elements in the list + * + * Frees a list of filters. + */ +void +virLogFilterListFree(virLogFilterPtr *list, int count) +{ + size_t i; + + if (!list || count < 0) + return; + + for (i = 0; i < count; i++) + virLogFilterFree(list[i]); + VIR_FREE(list); +} diff --git a/src/util/virlog.h b/src/util/virlog.h index eec3a5d..eca7894 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -198,6 +198,7 @@ extern int virLogOutputNew(virLogOutputFunc f, extern void virLogOutputFree(virLogOutputPtr output); extern void virLogFilterFree(virLogFilterPtr filter); extern void virLogOutputListFree(virLogOutputPtr *list, int count); +extern void virLogFilterListFree(virLogFilterPtr *list, int count); /* * Internal logging API -- 2.4.3
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.3
Everything has been prepared to successfully split parsing and defining logic to separate operations. --- src/util/virlog.c | 160 +++++++++++++++++++++++++++++------------------------ src/util/virlog.h | 15 +++-- tests/testutils.c | 19 ++++++- tests/virlogtest.c | 5 +- 4 files changed, 114 insertions(+), 85 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 7e0936c..369d7dd 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -343,64 +343,50 @@ virLogOutputFree(virLogOutputPtr output) * @c: the function to call to close the output (or NULL) * @data: extra data passed as first arg to the function * @priority: minimal priority for this filter, use 0 for none - * @dest: where to send output of this priority + * @dest: where to send output of this priority (see virLogDestination) * @name: optional name data associated with an output - * @flags: extra flag, currently unused * * Defines an output function for log messages. Each message once - * gone though filtering is emitted through each registered output. + * gone through filtering is emitted through each registered output. * - * Returns -1 in case of failure or the output number if successful + * Returns reference to a newly created object or NULL in case of failure. */ -int +virLogOutputPtr virLogOutputNew(virLogOutputFunc f, virLogCloseFunc c, void *data, virLogPriority priority, virLogDestination dest, - const char *name, - unsigned int flags) + const char *name) { + virLogOutputPtr ret = NULL; char *ndup = NULL; - virLogOutputPtr output = NULL; - - virCheckFlags(0, -1); - - if (virLogInitialize() < 0) - return -1; - if (f == NULL) - return -1; + if (!f) + return NULL; if (dest == VIR_LOG_TO_SYSLOG || dest == VIR_LOG_TO_FILE) { - if (!name) { - virReportOOMError(); - return -1; - } + if (!name) + return NULL; + if (VIR_STRDUP(ndup, name) < 0) - return -1; + return NULL; } - if (VIR_ALLOC_QUIET(output) < 0) { + if (VIR_ALLOC_QUIET(ret) < 0) { VIR_FREE(ndup); - return -1; + return NULL; } - output->logInitMessage = true; - output->f = f; - output->c = c; - output->data = data; - output->priority = priority; - output->dest = dest; - output->name = ndup; + ret->logInitMessage = true; + ret->f = f; + ret->c = c; + ret->data = data; + ret->priority = priority; + ret->dest = dest; + ret->name = ndup; - virLogLock(); - if (VIR_APPEND_ELEMENT_QUIET(virLogOutputs, virLogNbOutputs, output)) - goto cleanup; - - cleanup: - virLogUnlock(); - return virLogNbOutputs; + return ret; } @@ -717,32 +703,32 @@ virLogCloseFd(void *data) } -static int +static virLogOutputPtr virLogNewOutputToStderr(virLogPriority priority) { - if (virLogOutputNew(virLogOutputToFd, NULL, (void *)2L, priority, - VIR_LOG_TO_STDERR, NULL, 0) < 0) - return -1; - return 0; + return virLogOutputNew(virLogOutputToFd, NULL, (void *)2L, priority, + VIR_LOG_TO_STDERR, NULL); } -static int +static virLogOutputPtr virLogNewOutputToFile(virLogPriority priority, const char *file) { int fd; + virLogOutputPtr ret = NULL; fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR); if (fd < 0) - return -1; - if (virLogOutputNew(virLogOutputToFd, virLogCloseFd, - (void *)(intptr_t)fd, - priority, VIR_LOG_TO_FILE, file, 0) < 0) { - VIR_FORCE_CLOSE(fd); - return -1; + return NULL; + + if (!(ret = virLogOutputNew(virLogOutputToFd, virLogCloseFd, + (void *)(intptr_t)fd, + priority, VIR_LOG_TO_FILE, file))) { + VIR_LOG_CLOSE(fd); + return NULL; } - return 0; + return ret; } @@ -811,25 +797,28 @@ virLogCloseSyslog(void *data ATTRIBUTE_UNUSED) } -static int +static virLogOutputPtr virLogNewOutputToSyslog(virLogPriority priority, const char *ident) { + virLogOutputPtr ret = NULL; + /* * ident needs to be kept around on Solaris */ VIR_FREE(current_ident); if (VIR_STRDUP(current_ident, ident) < 0) - return -1; + return NULL; openlog(current_ident, 0, 0); - if (virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog, NULL, - priority, VIR_LOG_TO_SYSLOG, ident, 0) < 0) { + if (!(ret = virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog, + NULL, priority, VIR_LOG_TO_SYSLOG, + ident))) { closelog(); VIR_FREE(current_ident); - return -1; + return NULL; } - return 0; + return ret; } @@ -1035,19 +1024,25 @@ static void virLogCloseJournald(void *data ATTRIBUTE_UNUSED) } -static int virLogNewOutputToJournald(int priority) +static virLogOutputPtr virLogNewOutputToJournald(int priority) { + virLogOutputPtr ret = NULL; + if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) - return -1; + return NULL; if (virSetInherit(journalfd, false) < 0) { VIR_LOG_CLOSE(journalfd); - return -1; + return NULL; } - if (virLogOutputNew(virLogOutputToJournald, virLogCloseJournald, NULL, - priority, VIR_LOG_TO_JOURNALD, NULL, 0) < 0) { - return -1; + + if (!(ret = virLogOutputNew(virLogOutputToJournald, + virLogCloseJournald, NULL, + priority, VIR_LOG_TO_JOURNALD, NULL))) { + VIR_LOG_CLOSE(journalfd); + return NULL; } - return 0; + + return ret; } # endif /* USE_JOURNALD */ @@ -1081,11 +1076,10 @@ int virLogPriorityFromSyslog(int priority ATTRIBUTE_UNUSED) ((*cur == ' ') || (*cur == '\t') || (*cur == '\n') || \ (*cur == '\r') || (*cur == '\\')) - -static int +static virLogOutputPtr virLogParseOutput(const char *src) { - int ret = -1; + virLogOutputPtr ret = NULL; char **tokens = NULL; char *abspath = NULL; size_t count = 0; @@ -1094,7 +1088,7 @@ virLogParseOutput(const char *src) bool isSUID = virIsSUID(); if (!src) - return -1; + return NULL; VIR_DEBUG("output=%s", src); @@ -1102,7 +1096,7 @@ virLogParseOutput(const char *src) * them individually */ if (!(tokens = virStringSplitCount(src, ":", 0, &count))) - return -1; + return NULL; if (virStrToLong_uip(tokens[0], NULL, 10, &prio) < 0 || (prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR)) @@ -1146,7 +1140,7 @@ virLogParseOutput(const char *src) } cleanup: - if (ret < 0) + if (!ret) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to parse and define log output %s"), src); virStringFreeList(tokens); @@ -1180,12 +1174,14 @@ virLogParseOutput(const char *src) * Returns the number of output parsed or -1 in case of error. */ int -virLogParseOutputs(const char *src) +virLogParseOutputs(const char *src, virLogOutputPtr **outputs) { int ret = -1; - int count = 0; + size_t count = 0; size_t i; char **strings = NULL; + virLogOutputPtr output = NULL; + virLogOutputPtr *list = NULL; if (!src) return -1; @@ -1200,14 +1196,22 @@ virLogParseOutputs(const char *src) if (STREQ(strings[i], "")) continue; - if (virLogParseOutput(strings[i]) < 0) + if (!(output = virLogParseOutput(strings[i]))) goto cleanup; - count++; + if (VIR_APPEND_ELEMENT(list, count, output) < 0) { + virLogOutputFree(output); + goto cleanup; + } + + virLogOutputFree(output); } ret = count; + *outputs = list; cleanup: + if (ret < 0) + virLogOutputListFree(list, count); virStringFreeList(strings); return ret; } @@ -1535,12 +1539,22 @@ int virLogSetOutputs(const char *outputs) { int ret = -1; + int count = 0; + virLogOutputPtr *list = NULL; if (virLogInitialize() < 0) return -1; - ret = virLogParseOutputs(outputs); + if ((count = virLogParseOutputs(outputs, &list)) < 0) + goto cleanup; + if (virLogDefineOutputs(list, count) < 0) + goto cleanup; + + ret = count; + cleanup: + if (ret < 0) + virLogOutputListFree(list, count); return ret; } diff --git a/src/util/virlog.h b/src/util/virlog.h index eca7894..9b9f643 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -188,13 +188,12 @@ extern int virLogSetOutputs(const char *outputs); extern int virLogFilterNew(const char *match, virLogPriority priority, unsigned int flags); -extern int virLogOutputNew(virLogOutputFunc f, - virLogCloseFunc c, - void *data, - virLogPriority priority, - virLogDestination dest, - const char *name, - unsigned int flags); +extern virLogOutputPtr virLogOutputNew(virLogOutputFunc f, + virLogCloseFunc c, + void *data, + virLogPriority priority, + virLogDestination dest, + const char *name); extern void virLogOutputFree(virLogOutputPtr output); extern void virLogFilterFree(virLogFilterPtr filter); extern void virLogOutputListFree(virLogOutputPtr *list, int count); @@ -209,7 +208,7 @@ extern void virLogUnlock(void); extern int virLogReset(void); extern int virLogParseDefaultPriority(const char *priority); extern int virLogParseFilters(const char *filters); -extern int virLogParseOutputs(const char *output); +extern int virLogParseOutputs(const char *src, virLogOutputPtr **outputs); extern int virLogPriorityFromSyslog(int priority); extern void virLogMessage(virLogSourcePtr source, virLogPriority priority, diff --git a/tests/testutils.c b/tests/testutils.c index 4998931..e8da060 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -838,6 +838,9 @@ int virtTestMain(int argc, { int ret; char *testRange = NULL; + size_t count = 0; + virLogOutputPtr output = NULL; + virLogOutputPtr *list = NULL; #ifdef TEST_OOM char *oomstr; #endif @@ -869,9 +872,21 @@ int virtTestMain(int argc, virLogSetFromEnv(); if (!getenv("LIBVIRT_DEBUG") && !virLogGetNbOutputs()) { - if (virLogOutputNew(virtTestLogOutput, virtTestLogClose, &testLog, - VIR_LOG_DEBUG, VIR_LOG_TO_STDERR, NULL, 0) < 0) + if (!(output = virLogOutputNew(virtTestLogOutput, virtTestLogClose, + &testLog, VIR_LOG_DEBUG, + VIR_LOG_TO_STDERR, NULL))) return EXIT_FAILURE; + + if (VIR_APPEND_ELEMENT(list, count, output) < 0) { + virLogOutputFree(output); + return EXIT_FAILURE; + } + + if (virLogDefineOutputs(list, count) < 0) { + virLogOutputListFree(list, count); + return EXIT_FAILURE; + } + } if ((testRange = getenv("VIR_TEST_RANGE")) != NULL) { diff --git a/tests/virlogtest.c b/tests/virlogtest.c index 6abd4ae..02613f5 100644 --- a/tests/virlogtest.c +++ b/tests/virlogtest.c @@ -48,9 +48,10 @@ testLogParseOutputs(const void *opaque) { int ret = -1; int noutputs; + virLogOutputPtr *list = NULL; const struct testLogData *data = opaque; - noutputs = virLogParseOutputs(data->str); + noutputs = virLogParseOutputs(data->str, &list); if (noutputs < 0) { if (!data->pass) { VIR_TEST_DEBUG("Got expected error: %s\n", @@ -70,7 +71,7 @@ testLogParseOutputs(const void *opaque) ret = 0; cleanup: - virLogReset(); + virLogOutputListFree(list, noutputs); return ret; } -- 2.4.3
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 e594eaa..319ef18 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1743,6 +1743,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.3
Handling of outputs and filters has been changed in a way that splits parsing and defining. Do the same thing for logging priority as well, this however, doesn't need much of a preparation. --- src/util/virlog.c | 21 +++++++++------------ tests/eventtest.c | 3 ++- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 0116f56..9fe47fb 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -220,7 +220,9 @@ int virLogSetDefaultPriority(virLogPriority priority) { if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) { - VIR_WARN("Ignoring invalid log level setting."); + virReportError(VIR_ERR_INVALID_ARG, + _("Failed to set logging priority, argument '%u' is " + "invalid"), priority); return -1; } if (virLogInitialize() < 0) @@ -1432,20 +1434,15 @@ virLogGetNbOutputs(void) int virLogParseDefaultPriority(const char *priority) { - int ret = -1; - if (STREQ(priority, "1") || STREQ(priority, "debug")) - ret = virLogSetDefaultPriority(VIR_LOG_DEBUG); + return VIR_LOG_DEBUG; else if (STREQ(priority, "2") || STREQ(priority, "info")) - ret = virLogSetDefaultPriority(VIR_LOG_INFO); + return VIR_LOG_INFO; else if (STREQ(priority, "3") || STREQ(priority, "warning")) - ret = virLogSetDefaultPriority(VIR_LOG_WARN); + return VIR_LOG_WARN; else if (STREQ(priority, "4") || STREQ(priority, "error")) - ret = virLogSetDefaultPriority(VIR_LOG_ERROR); - else - VIR_WARN("Ignoring invalid log level setting"); - - return ret; + return VIR_LOG_ERROR; + return -1; } @@ -1465,7 +1462,7 @@ virLogSetFromEnv(void) debugEnv = virGetEnvAllowSUID("LIBVIRT_DEBUG"); if (debugEnv && *debugEnv) - virLogParseDefaultPriority(debugEnv); + virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv)); debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_FILTERS"); if (debugEnv && *debugEnv) virLogSetFilters(debugEnv); diff --git a/tests/eventtest.c b/tests/eventtest.c index c4be606..b6d12a5 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -311,7 +311,8 @@ mymain(void) if (virThreadInitialize() < 0) return EXIT_FAILURE; char *debugEnv = getenv("LIBVIRT_DEBUG"); - if (debugEnv && *debugEnv && (virLogParseDefaultPriority(debugEnv) == -1)) { + if (debugEnv && *debugEnv && + (virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv)) < 0)) { fprintf(stderr, "Invalid log level setting.\n"); return EXIT_FAILURE; } -- 2.4.3
Outputs are a little bit trickier than filters, because if user wants to change the set of outputs that is currently defined for a different set that may contain outputs that do already exist in the current set, opening a file descriptor for that output would only work for files, but not for syslog or journald, because that would overwrite the existing ones which might (and probably will be in use by workers). So, we need a mechanism to find out, if such an output is already defined and only copy its data. For files, the data also include the already opened file descriptor. For journald, the file descriptor is global, so nothing to copy there. Syslog is the most trickiest one, since it keeps the open file descriptor to system daemon private and cannot be reopened (to change the message prefix) until the very last moment when the whole output set is ready to be swapped with the defined one. This method is also used when deallocating a list of outputs (either the new copy that is just being prepared or the global active set of outputs). According to the paragraph above, the problem of changing data when it is being used has been solved. It still needs to be ensured that when deallocating the global active set of outputs after it was swap with the user-provided copy does not close all file descriptors, since that would cause invalid references in the copy that we just swapped. By issuing virLogOutputExist during deallocation, the described scenario can be safely prevented. --- src/libvirt_private.syms | 1 + src/util/virlog.c | 36 ++++++++++++++++++++++++++++++++++++ src/util/virlog.h | 1 + 3 files changed, 38 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 319ef18..cc40b46 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1752,6 +1752,7 @@ virLogGetNbOutputs; virLogGetOutputs; virLogLock; virLogMessage; +virLogOutputExists; virLogOutputFree; virLogOutputListFree; virLogOutputNew; diff --git a/src/util/virlog.c b/src/util/virlog.c index 9fe47fb..204372f 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -186,6 +186,42 @@ VIR_ONCE_GLOBAL_INIT(virLog) /** + * virLogOutputExists: + * @dest: destination type + * @opaque: opaque data to the method + * + * Looks for an output of destination type @dest. If such output exists, + * a reference to the output is returned, unless the destination is of type + * file in which case a comparison of the output's data with @opaque needs to + * be done first. + * + * Returns a reference to an output if one was found or NULL if such output + * does not exist. + */ +virLogOutputPtr +virLogOutputExists(virLogDestination dest, const void *opaque) +{ + size_t i; + const char *name = opaque; + + for (i = 0; i < virLogNbOutputs; i++) { + if (!virLogOutputs[i]) + continue; + + if (dest == virLogOutputs[i]->dest) { + if (dest != VIR_LOG_TO_FILE) + return virLogOutputs[i]; + + if (STREQ(virLogOutputs[i]->name, name)) + return virLogOutputs[i]; + } + } + + return NULL; +} + + +/** * virLogReset: * * Reset the logging module to its default initial state diff --git a/src/util/virlog.h b/src/util/virlog.h index 0102489..1c55a48 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -229,5 +229,6 @@ extern void virLogVMessage(virLogSourcePtr source, bool virLogProbablyLogMessage(const char *str); int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs); int virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters); +virLogOutputPtr virLogOutputExists(virLogDestination dest, const void *opaque); #endif -- 2.4.3
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.3
Now that we're in the critical section, closelog and openlog need to be issued if the user setting changed, which is something that cannot be done beforehand, since syslog keeps its file descriptor private. --- src/util/virlog.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 62533b1..a20cde4 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1605,6 +1605,8 @@ virLogSetOutputs(const char *outputs) int ret = -1; int count = 0; virLogOutputPtr *list = NULL; + size_t i; + char *tmp = NULL; if (virLogInitialize() < 0) return -1; @@ -1612,6 +1614,17 @@ virLogSetOutputs(const char *outputs) if ((count = virLogParseOutputs(outputs, &list)) < 0) goto cleanup; + /* syslog needs a special care */ + for (i = 0; i < count; i++) { + if (list[i]->dest == VIR_LOG_TO_SYSLOG) { + if (VIR_STRDUP(tmp, list[i]->name) < 0) + goto cleanup; + VIR_FREE(current_ident); + current_ident = tmp; + openlog(current_ident, 0, 0); + } + } + if (virLogDefineOutputs(list, count) < 0) goto cleanup; -- 2.4.3
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.3
Global priority can be changed from outside as well. So far, updating priority is filter driven, meaning that source priority can only be changed when filters change. To assure, users can change the global priority from outside, make the serial more generic, so that any change to priority would trigger source update. --- src/util/virlog.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 8e14a9e..a837e17 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -87,7 +87,7 @@ struct _virLogFilter { unsigned int flags; }; -static int virLogFiltersSerial = 1; +static int virLogSerial = 1; static virLogFilterPtr *virLogFilters; static size_t virLogNbFilters; @@ -280,7 +280,7 @@ virLogResetFilters(void) virLogFilterListFree(virLogFilters, virLogNbFilters); virLogFilters = NULL; virLogNbFilters = 0; - virLogFiltersSerial++; + virLogSerial++; } @@ -483,7 +483,7 @@ static void virLogSourceUpdate(virLogSourcePtr source) { virLogLock(); - if (source->serial < virLogFiltersSerial) { + if (source->serial < virLogSerial) { unsigned int priority = virLogDefaultPriority; unsigned int flags = 0; size_t i; @@ -498,7 +498,7 @@ virLogSourceUpdate(virLogSourcePtr source) source->priority = priority; source->flags = flags; - source->serial = virLogFiltersSerial; + source->serial = virLogSerial; } virLogUnlock(); } @@ -582,7 +582,7 @@ virLogVMessage(virLogSourcePtr source, * thread is updating log filter list concurrently * with a log message emission. */ - if (source->serial < virLogFiltersSerial) + if (source->serial < virLogSerial) virLogSourceUpdate(source); if (priority < source->priority) goto cleanup; @@ -1686,7 +1686,7 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters) virLogResetFilters(); virLogFilters = filters; virLogNbFilters = nfilters; - virLogFiltersSerial++; + virLogSerial++; virLogUnlock(); return virLogNbFilters; -- 2.4.3
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.3
If the API isn't closed, a situation with 2 setters where one is about to define a set of outputs and the other is already defining a new one, may occur. By resetting all outputs, all file descriptors are closed. However, the other setter may still have a dangling reference to a file descriptor which may have already been closed. --- src/libvirt_private.syms | 2 ++ src/util/virlog.c | 15 +++++++++++++++ src/util/virlog.h | 2 ++ 3 files changed, 19 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc40b46..14047f5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,6 +1741,8 @@ virLockSpaceReleaseResourcesForOwner; # util/virlog.h +virLogAPILock; +virLogAPIUnlock; virLogDefineFilters; virLogDefineOutputs; virLogFilterListFree; diff --git a/src/util/virlog.c b/src/util/virlog.c index 769dcec..6aa9c91 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -128,6 +128,21 @@ static void virLogOutputToFd(virLogSourcePtr src, void *data); +/* Setters need to be serialized on API entry point */ +static virMutex virLogAPIMutex; + +void +virLogAPILock(void) +{ + virMutexLock(&virLogAPIMutex); +} + +void +virLogAPIUnlock(void) +{ + virMutexUnlock(&virLogAPIMutex); +} + /* * Logs accesses must be serialized though a mutex */ diff --git a/src/util/virlog.h b/src/util/virlog.h index 1c55a48..f5c0a4f 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -203,6 +203,8 @@ extern void virLogFilterListFree(virLogFilterPtr *list, int count); * Internal logging API */ +extern void virLogAPILock(void); +extern void virLogAPIUnlock(void); extern void virLogLock(void); extern void virLogUnlock(void); extern int virLogReset(void); -- 2.4.3
On Thu, Mar 31, 2016 at 07:49:02PM +0200, Erik Skultety wrote:
If the API isn't closed, a situation with 2 setters where one is about to define a set of outputs and the other is already defining a new one, may occur. By resetting all outputs, all file descriptors are closed. However, the other setter may still have a dangling reference to a file descriptor which may have already been closed. --- src/libvirt_private.syms | 2 ++ src/util/virlog.c | 15 +++++++++++++++ src/util/virlog.h | 2 ++ 3 files changed, 19 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc40b46..14047f5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,6 +1741,8 @@ virLockSpaceReleaseResourcesForOwner;
# util/virlog.h +virLogAPILock; +virLogAPIUnlock; virLogDefineFilters; virLogDefineOutputs; virLogFilterListFree; diff --git a/src/util/virlog.c b/src/util/virlog.c index 769dcec..6aa9c91 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -128,6 +128,21 @@ static void virLogOutputToFd(virLogSourcePtr src, void *data);
+/* Setters need to be serialized on API entry point */ +static virMutex virLogAPIMutex; + +void +virLogAPILock(void) +{ + virMutexLock(&virLogAPIMutex); +} + +void +virLogAPIUnlock(void) +{ + virMutexUnlock(&virLogAPIMutex); +} + /* * Logs accesses must be serialized though a mutex */ diff --git a/src/util/virlog.h b/src/util/virlog.h index 1c55a48..f5c0a4f 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -203,6 +203,8 @@ extern void virLogFilterListFree(virLogFilterPtr *list, int count); * Internal logging API */
+extern void virLogAPILock(void); +extern void virLogAPIUnlock(void); extern void virLogLock(void); extern void virLogUnlock(void);
I'm not really seeing the reason why we need a second mutex instead of just ensuring we acquire the existing mutex in the right places. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 31/03/16 20:13, Daniel P. Berrange wrote:
On Thu, Mar 31, 2016 at 07:49:02PM +0200, Erik Skultety wrote:
If the API isn't closed, a situation with 2 setters where one is about to define a set of outputs and the other is already defining a new one, may occur. By resetting all outputs, all file descriptors are closed. However, the other setter may still have a dangling reference to a file descriptor which may have already been closed. --- src/libvirt_private.syms | 2 ++ src/util/virlog.c | 15 +++++++++++++++ src/util/virlog.h | 2 ++ 3 files changed, 19 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc40b46..14047f5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,6 +1741,8 @@ virLockSpaceReleaseResourcesForOwner;
# util/virlog.h +virLogAPILock; +virLogAPIUnlock; virLogDefineFilters; virLogDefineOutputs; virLogFilterListFree; diff --git a/src/util/virlog.c b/src/util/virlog.c index 769dcec..6aa9c91 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -128,6 +128,21 @@ static void virLogOutputToFd(virLogSourcePtr src, void *data);
+/* Setters need to be serialized on API entry point */ +static virMutex virLogAPIMutex; + +void +virLogAPILock(void) +{ + virMutexLock(&virLogAPIMutex); +} + +void +virLogAPIUnlock(void) +{ + virMutexUnlock(&virLogAPIMutex); +} + /* * Logs accesses must be serialized though a mutex */ diff --git a/src/util/virlog.h b/src/util/virlog.h index 1c55a48..f5c0a4f 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -203,6 +203,8 @@ extern void virLogFilterListFree(virLogFilterPtr *list, int count); * Internal logging API */
+extern void virLogAPILock(void); +extern void virLogAPIUnlock(void); extern void virLogLock(void); extern void virLogUnlock(void);
I'm not really seeing the reason why we need a second mutex instead of just ensuring we acquire the existing mutex in the right places.
Regards, Daniel
Could you provide more details on this, because what I had in mind (and I still see it that way), if we agree that operation of setting log outputs exposed via public API should be done atomically for the whole set rather than doing it gradually for each output, then there isn't much else to do. From my point of view, the critical section should be as small as possible, so moving the locks to the place where I proposed to use the API locks would mean that the critical section would include the parsing as well, not sure if that's the best approach. Let's say we do it that way, we put our existing log mutex to virLogSetOutputs entry point. The other thing is that from the point we start parsing to the point where the outputs are actually defined, a number of errors could occur and none of them can be reported, otherwise a deadlock occurs. We also cannot omit locking at the entry point completely, i.e. only having our original virLogMutex where I proposed to have it - before the actual definition takes place. Imagine 2 admin clients, both attempting to set their own set of outputs, sharing some of those outputs with the currently defined global set of log outputs. Now, both clients create their own private copies, opening all outputs that are not defined yet and copying all file descriptors from outputs that should continue to exist once the new copy replaces the original. The tricky part: client n.1 acquires the lock, swaps the original with its copy and forces the original to close all outputs that aren't shared between copy n.1 and the original. Now client n.2 acquires the lock and attempts to swap copy n.1 with its copy n.2. The problem is, that copy n2. wasn't created against copy n.1, instead, it was created against the original. So, it might happen that copy n.2 now might have some invalid file descriptors (that client n.1 forced the original to close, since these two didn't share them). If used with the new mutex I proposed, this scenario can't happen (similar to what I actually wrote into the cover-letter). Other solution that might (might as well not) work, rather than copying file descriptors and picking which output should or should not be closed, we could open a single file repeatedly (increasing the counter of references to an open file), with the exception of syslog and journald, that will both have to be special-cased inside the critical section. This way, syslog is ok, journald is ok, and all files can be safely closed because most of the time, only counter will be decremented, thus no dangling file descriptor can exist. Not sure however, if we want to force open each file multiple times instead of copying the existing file descriptors. Anyway, I'd like to hear your opinion and see your idea, how would you approach it. Erik
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.3
Enable libvirt users to query for the current logging level setting. --- daemon/admin.c | 8 ++++++++ include/libvirt/libvirt-admin.h | 1 + src/admin/admin_protocol.x | 15 ++++++++++++++- src/admin_protocol-structs | 7 +++++++ src/libvirt-admin.c | 33 +++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 7 files changed, 66 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 3169cdd..ed07988 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -133,4 +133,12 @@ adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return 0; } +static int +adminConnectGetLoggingLevel(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(0, -1); + + return virLogGetDefaultPriority(); +} #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 25bcbf4..c1aba01 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -110,6 +110,7 @@ virAdmServerPtr virAdmConnectLookupServer(virAdmConnectPtr conn, const char *name, unsigned int flags); +int virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, unsigned int flags); # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 6590980..268c81e 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -74,6 +74,14 @@ struct admin_connect_lookup_server_ret { admin_nonnull_server srv; }; +struct admin_connect_get_logging_level_args { + unsigned int flags; +}; + +struct admin_connect_get_logging_level_ret { + int level; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -119,5 +127,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5 + ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index d8aca06..9d0397c 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -26,10 +26,17 @@ struct admin_connect_lookup_server_args { struct admin_connect_lookup_server_ret { admin_nonnull_server srv; }; +struct admin_connect_get_logging_level_args { + u_int flags; +}; +struct admin_connect_get_logging_level_ret { + int level; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, ADMIN_PROC_CONNECT_LIST_SERVERS = 4, ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5, + ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 54af90c..f29c83d 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -674,3 +674,36 @@ virAdmConnectLookupServer(virAdmConnectPtr conn, virDispatchError(NULL); return ret; } + +/** + * virAdmConnectGetLoggingLevel: + * @conn: pointer to an active admin connection + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Retrieves the current global logging level (as per daemon configuration): + * 1: DEBUG + * 2: INFO + * 3: WARNING + * 4: ERROR + * + * Returns the numeric logging level representation or -1 in case of an error. + */ +int +virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, flags=%x", conn, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + + if ((ret = remoteAdminConnectGetLoggingLevel(conn, 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 268f1e6..d2e2292 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -7,6 +7,8 @@ # admin/admin_protocol.x xdr_admin_connect_get_lib_version_ret; +xdr_admin_connect_get_logging_level_args; +xdr_admin_connect_get_logging_level_ret; xdr_admin_connect_list_servers_args; xdr_admin_connect_list_servers_ret; xdr_admin_connect_lookup_server_args; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 58d085e..37d534e 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -22,6 +22,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectRegisterCloseCallback; virAdmConnectUnregisterCloseCallback; virAdmConnectListServers; + virAdmConnectGetLoggingLevel; virAdmServerGetName; virAdmServerFree; virAdmConnectLookupServer; -- 2.4.3
On Thu, Mar 31, 2016 at 07:49:04PM +0200, Erik Skultety wrote:
Enable libvirt users to query for the current logging level setting.
I'm not really convinced we need to expose this via the API, as the 'log_level' flag has really long ago ceased to be something practical to use. We generate soooooo much debug information these days that you just drown in noise if you set log_level. In fact, I'd suggest we should probably just deprecate it as a concept, with a view to removing it entirely in future. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 31/03/16 20:16, Daniel P. Berrange wrote:
On Thu, Mar 31, 2016 at 07:49:04PM +0200, Erik Skultety wrote:
Enable libvirt users to query for the current logging level setting.
I'm not really convinced we need to expose this via the API, as the 'log_level' flag has really long ago ceased to be something practical to use. We generate soooooo much debug information these days that you just drown in noise if you set log_level.
In fact, I'd suggest we should probably just deprecate it as a concept, with a view to removing it entirely in future.
Regards, Daniel
Indeed, I didn't see it this way, but now that you mentioned it, I agree, it's nothing that couldn't be accomplished via filters solely...will be dropped in v2 along with 37/38. Erik
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 ed07988..5098286 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -141,4 +141,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 c1aba01..27e1f0b 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -111,6 +111,10 @@ virAdmServerPtr virAdmConnectLookupServer(virAdmConnectPtr conn, unsigned int flags); int virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, unsigned int flags); + +int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, + char **logFilters, + unsigned int flags); # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 268c81e..60ebe03 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -82,6 +82,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; @@ -132,5 +141,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6 + ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6, + + /** + * @generate: none + */ + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 21e0dd3..17954e6 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -224,3 +224,46 @@ remoteAdminPrivNew(const char *sock_path) virObjectUnref(priv); return NULL; } + +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 9d0397c..b7d5fa1 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -32,6 +32,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, @@ -39,4 +46,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_LIST_SERVERS = 4, ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5, ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6, + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index f29c83d..2c7548c 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -707,3 +707,43 @@ virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectGetLoggingFilters: + * @conn: pointer to an active admin connection + * @filters: pointer to a variable to store a NULL-terminated list of + * currently defined logging filters; + * @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 a 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 d2e2292..341f896 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -7,6 +7,8 @@ # admin/admin_protocol.x 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 37d534e..41c858f 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -23,6 +23,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectUnregisterCloseCallback; virAdmConnectListServers; virAdmConnectGetLoggingLevel; + virAdmConnectGetLoggingFilters; virAdmServerGetName; virAdmServerFree; virAdmConnectLookupServer; -- 2.4.3
Enable libvirt users to query logging output settings. --- daemon/admin.c | 42 ++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 5 +++++ src/admin/admin_protocol.x | 16 ++++++++++++++- src/admin/admin_remote.c | 43 +++++++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 39 +++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 155 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 5098286..43ba2cb 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -160,6 +160,23 @@ adminConnectGetLoggingFilters(char **filters, unsigned int flags) } static int +adminConnectGetLoggingOutputs(char **outputs, unsigned int flags) +{ + char *tmp = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(tmp = virLogGetOutputs())) + return -1; + + ret = virLogGetNbOutputs(); + + *outputs = tmp; + return ret; +} + +static int adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, @@ -186,4 +203,29 @@ adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, VIR_FREE(filters); return rv; } + +static int +adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + admin_connect_get_logging_outputs_args *args, + admin_connect_get_logging_outputs_ret *ret) +{ + char *outputs = NULL; + int noutputs = 0; + int rv = -1; + + if ((noutputs = adminConnectGetLoggingOutputs(&outputs, args->flags) < 0)) + goto cleanup; + + ret->outputs = outputs; + ret->noutputs = noutputs; + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 27e1f0b..9608c42 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -115,6 +115,11 @@ int virAdmConnectGetLoggingLevel(virAdmConnectPtr conn, unsigned int flags); int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, char **logFilters, unsigned int flags); + +int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, + char **logOutputs, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 60ebe03..2decb1e 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -91,6 +91,15 @@ struct admin_connect_get_logging_filters_ret { unsigned int nfilters; }; +struct admin_connect_get_logging_outputs_args { + unsigned int flags; +}; + +struct admin_connect_get_logging_outputs_ret { + admin_nonnull_string outputs; + unsigned int noutputs; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -146,5 +155,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7 + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7, + + /** + * @generate: none + */ + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 17954e6..b533149 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -267,3 +267,46 @@ remoteAdminConnectGetLoggingFilters(virAdmConnectPtr conn, virObjectUnlock(priv); return rv; } + +static int +remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn, + char **outputs, + unsigned int flags) +{ + int rv = -1; + char *tmp_outputs = NULL; + remoteAdminPrivPtr priv = conn->privateData; + admin_connect_get_logging_outputs_args args; + admin_connect_get_logging_outputs_ret ret; + + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + virObjectLock(priv); + + if (call(conn, + 0, + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS, + (xdrproc_t) xdr_admin_connect_get_logging_outputs_args, + (char *) &args, + (xdrproc_t) xdr_admin_connect_get_logging_outputs_ret, + (char *) &ret) == -1) + goto done; + + if (outputs) { + if (VIR_STRDUP(tmp_outputs, ret.outputs) < 0) + goto cleanup; + + *outputs = tmp_outputs; + tmp_outputs = NULL; + } + + rv = ret.noutputs; + + cleanup: + xdr_free((xdrproc_t) xdr_admin_connect_get_logging_outputs_ret, (char *) &ret); + + done: + virObjectUnlock(priv); + return rv; +} diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index b7d5fa1..8482dee 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -39,6 +39,13 @@ struct admin_connect_get_logging_filters_ret { admin_nonnull_string filters; u_int nfilters; }; +struct admin_connect_get_logging_outputs_args { + u_int flags; +}; +struct admin_connect_get_logging_outputs_ret { + admin_nonnull_string outputs; + u_int noutputs; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -47,4 +54,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5, ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6, ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7, + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 2c7548c..39a7b02 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -747,3 +747,42 @@ virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectGetLoggingOutputs: + * @conn: pointer to an active admin connection + * @outputs: pointer to a variable to store a NULL-terminated list of + * currently defined logging outputs; + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Retrieves a list of currently installed logging outputs. Outputs returned + * are contained within a string and delimited by spaces. The format of each + * output conforms to the format described in daemon's configuration file + * (e.g. libvirtd.conf). + * To retrieve individual outputs, additional parsing needs to be done by the + * caller. Caller is also responsible for freeing @outputs correctly. + * + * Returns the count of outputs in @outputs, or -1 in case of an error. + */ +int +virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, + char **outputs, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, flags=%x", conn, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + virCheckNonNullArgGoto(outputs, error); + + if ((ret = remoteAdminConnectGetLoggingOutputs(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 341f896..2e34178 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -11,6 +11,8 @@ 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_get_logging_outputs_args; +xdr_admin_connect_get_logging_outputs_ret; xdr_admin_connect_list_servers_args; xdr_admin_connect_list_servers_ret; xdr_admin_connect_lookup_server_args; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 41c858f..56bb073 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -24,6 +24,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectListServers; virAdmConnectGetLoggingLevel; virAdmConnectGetLoggingFilters; + virAdmConnectGetLoggingOutputs; virAdmServerGetName; virAdmServerFree; virAdmConnectLookupServer; -- 2.4.3
Enable libvirt users to set logging level of a daemon 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 | 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 43ba2cb..866717a 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -160,6 +160,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 9608c42..8f26778 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -120,6 +120,10 @@ int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, char **logOutputs, 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 2decb1e..c62319c 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -100,6 +100,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; @@ -160,5 +165,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8 + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 9 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 8482dee..e6741c3 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -46,6 +46,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, @@ -55,4 +59,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LOGGING_LEVEL = 6, ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7, ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8, + ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 9, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 39a7b02..7b50dcb 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -786,3 +786,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 2e34178..f94e3f6 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -18,6 +18,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; # datatypes.h virAdmConnectClass; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 56bb073..0feadee 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -28,4 +28,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerGetName; virAdmServerFree; virAdmConnectLookupServer; + 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.3
Enable libvirt users to modify logging filters of a daemon 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 866717a..663e818 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -187,6 +187,16 @@ adminConnectGetLoggingOutputs(char **outputs, unsigned int flags) } static int +adminConnectSetLoggingFilters(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *filters, + unsigned int flags) +{ + virCheckFlags(0, -1); + + return virLogSetFilters(filters); +} + +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 8f26778..284a8f3 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -124,6 +124,10 @@ int virAdmConnectSetLoggingLevel(virAdmConnectPtr conn, unsigned int level, unsigned int flags); +int virAdmConnectSetLoggingFilters(virAdmConnectPtr conn, + const char *filters, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index c62319c..aefebd4 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -105,6 +105,11 @@ struct admin_connect_set_logging_level_args { unsigned int flags; }; +struct admin_connect_set_logging_filters_args { + admin_nonnull_string filters; + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -170,5 +175,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 9 + ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 9, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 10 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index e6741c3..70b1b0e 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -50,6 +50,10 @@ struct admin_connect_set_logging_level_args { u_int level; u_int flags; }; +struct admin_connect_set_logging_filters_args { + admin_nonnull_string filters; + u_int flags; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -60,4 +64,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 7, ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8, ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 9, + ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 10, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 7b50dcb..3888fb9 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -821,3 +821,39 @@ virAdmConnectSetLoggingLevel(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectSetLoggingFilters: + * @conn: pointer to an active admin connection + * @filters: list of filters, the format must conform to the format described in + * daemon's configuration file (e.g. libvirtd.conf) + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Redefines the existing (set of) filter(s) with a new one specified in + * @filters. If multiple filters are specified, they need to be delimited by + * spaces. + * + * Returns 0 if a new set of filters has been successfully defined, or -1 in + * case of an error. + */ +int +virAdmConnectSetLoggingFilters(virAdmConnectPtr conn, + const 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 = remoteAdminConnectSetLoggingFilters(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 f94e3f6..f78649e 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -18,6 +18,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_filters_args; xdr_admin_connect_set_logging_level_args; # datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 0feadee..0a46cb7 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -29,4 +29,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerFree; virAdmConnectLookupServer; virAdmConnectSetLoggingLevel; + virAdmConnectSetLoggingFilters; }; -- 2.4.3
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 663e818..4aa25ff 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -197,6 +197,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 284a8f3..be3af3a 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -128,6 +128,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 aefebd4..00d03a5 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -110,6 +110,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; @@ -180,5 +185,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 10 + ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 10, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 11 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 70b1b0e..cd16ee3 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -54,6 +54,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, @@ -65,4 +69,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 8, ADMIN_PROC_CONNECT_SET_LOGGING_LEVEL = 9, ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 10, + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 11, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 3888fb9..55105f4 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -857,3 +857,39 @@ virAdmConnectSetLoggingFilters(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectSetLoggingOutputs: + * @conn: pointer to an active admin connection + * @outputs: list of outputs, the format must conform to the format described in + * daemon's configuration file (e.g. libvirtd.conf) + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Redefines 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. + * + * Returns 0 if the list of outputs has been successfully defined 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 f78649e..2ca8cdc 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -20,6 +20,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; # datatypes.h virAdmConnectClass; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 0a46cb7..be4a0fe 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -30,4 +30,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectLookupServer; virAdmConnectSetLoggingLevel; virAdmConnectSetLoggingFilters; + virAdmConnectSetLoggingOutputs; }; -- 2.4.3
We should encourage libvirt users to use our own constants when modifying logging level, instead of their own values. --- include/libvirt/libvirt-admin.h | 15 +++++++++++++++ src/util/virlog.h | 10 ---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index be3af3a..dc7d252 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -35,6 +35,21 @@ extern "C" { # undef __VIR_ADMIN_H_INCLUDES__ /** + * virLogPriority: + * + * These levels are recognized by daemon's logger and can be statically + * configured in daemon's configuration file (e.g. libvirtd.conf). For runtime + * level retrieval and modification use virAdmConnectGetLoggingLevel and + * virAdmConnectSetLoggingLevel APIs. + */ +typedef enum { + VIR_LOG_DEBUG = 1, + VIR_LOG_INFO, + VIR_LOG_WARN, + VIR_LOG_ERROR, +} virLogPriority; + +/** * virAdmConnect: * * a virAdmConnect is a private structure representing a connection to diff --git a/src/util/virlog.h b/src/util/virlog.h index f5c0a4f..b631327 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -38,16 +38,6 @@ "libvirt version: " VERSION # endif -/* - * To be made public - */ -typedef enum { - VIR_LOG_DEBUG = 1, - VIR_LOG_INFO, - VIR_LOG_WARN, - VIR_LOG_ERROR, -} virLogPriority; - # define VIR_LOG_DEFAULT VIR_LOG_WARN typedef enum { -- 2.4.3
On Thu, Mar 31, 2016 at 07:49:10PM +0200, Erik Skultety wrote:
We should encourage libvirt users to use our own constants when modifying logging level, instead of their own values. --- include/libvirt/libvirt-admin.h | 15 +++++++++++++++ src/util/virlog.h | 10 ---------- 2 files changed, 15 insertions(+), 10 deletions(-)
If we don't expose API for changing the global log level, we can avoid exposing these constants. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Finally, now that all APIs have been introduced, wire them up to virt-admin and introduce dmn-log-info and dmn-log-define commands. --- tools/virt-admin.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index edb8690..9389ffe 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -353,6 +353,197 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return ret; } +/* ------------------- + * Command dmn-log-info + * ------------------- + */ +static const vshCmdInfo info_dmn_log_info[] = { + {.name = "help", + .data = N_("view daemon current logging information") + }, + {.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_("define 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) { @@ -652,12 +843,29 @@ static const vshCmdDef monitoringCmds[] = { .info = info_srv_list, .flags = 0 }, + {.name = "dmn-log-info", + .handler = cmdDmnLogInfo, + .opts = opts_dmn_log_info, + .info = info_dmn_log_info, + .flags = 0 + }, + {.name = NULL} +}; + +static const vshCmdDef managementCmds[] = { + {.name = "dmn-log-define", + .handler = cmdDmnLogDefine, + .opts = opts_dmn_log_define, + .info = info_dmn_log_define, + .flags = 0 + }, {.name = NULL} }; static const vshCmdGrp cmdGroups[] = { {"Virt-admin itself", "virt-admin", vshAdmCmds}, {"Monitoring commands", "monitor", monitoringCmds}, + {"Management commands", "management", managementCmds}, {NULL, NULL, NULL} }; -- 2.4.3
participants (2)
-
Daniel P. Berrange -
Erik Skultety