These helpers will manage the log destination defaults (fetch/set). The reason for this is to stay consistent with the current daemon's behaviour with respect to /etc/libvirt/<daemon>.conf file, since both assignment of an empty string or not setting the log output variable at all trigger the daemon's decision on the default log destination which depends on whether the daemon runs daemonized or not. This patch also changes the logic of the selection of the default logging output compared to how it is done now. The main difference though is that we should only really care if we're running daemonized or not, disregarding the fact of (not) having a TTY completely (introduced by commit eba36a3878) as that should be of the libvirtd's parent concern (what FD it will pass to it). Before: if (godaemon || !hasTTY): if (journald): use journald if (godaemon): if (privileged): use SYSCONFIG/libvirtd.log else: use XDG_CONFIG_HOME/libvirtd.log else: use stderr After: if (godaemon): if (journald): use journald else: if (privileged): use SYSCONFIG/libvirtd.log else: use XDG_CONFIG_HOME/libvirtd.log else: use stderr Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/libvirt_private.syms | 2 ++ src/util/virlog.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virlog.h | 2 ++ 3 files changed, 96 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a04ba23..803458f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1883,6 +1883,7 @@ virLogFilterFree; virLogFilterListFree; virLogFilterNew; virLogFindOutput; +virLogGetDefaultOutput; virLogGetDefaultPriority; virLogGetFilters; virLogGetNbFilters; @@ -1901,6 +1902,7 @@ virLogParseOutputs; virLogPriorityFromSyslog; virLogProbablyLogMessage; virLogReset; +virLogSetDefaultOutput; virLogSetDefaultPriority; virLogSetFilters; virLogSetFromEnv; diff --git a/src/util/virlog.c b/src/util/virlog.c index 8f831fc..9b52e66 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -50,6 +50,7 @@ #include "virtime.h" #include "intprops.h" #include "virstring.h" +#include "configmake.h" /* Journald output is only supported on Linux new enough to expose * htole64. */ @@ -105,6 +106,7 @@ struct _virLogOutput { char *name; }; +static char *virLogDefaultOutput; static virLogOutputPtr *virLogOutputs; static size_t virLogNbOutputs; @@ -147,6 +149,96 @@ virLogUnlock(void) } +static int +virLogSetDefaultOutputToStderr(void) +{ + return virAsprintf(&virLogDefaultOutput, "%d:stderr", + virLogDefaultPriority); +} + + +static int +virLogSetDefaultOutputToJournald(void) +{ + virLogPriority priority = virLogDefaultPriority; + + /* By default we don't want to log too much stuff into journald as + * it may employ rate limiting and thus block libvirt execution. */ + if (priority == VIR_LOG_DEBUG) + priority = VIR_LOG_INFO; + + return virAsprintf(&virLogDefaultOutput, "%d:journald", priority); +} + + +static int +virLogSetDefaultOutputToFile(const char *filename, bool privileged) +{ + int ret = -1; + char *logdir = NULL; + mode_t old_umask; + + if (privileged) { + if (virAsprintf(&virLogDefaultOutput, + "%d:file:%s/log/libvirt/%s", virLogDefaultPriority, + LOCALSTATEDIR, filename) < 0) + goto cleanup; + } else { + if (!(logdir = virGetUserCacheDirectory())) + goto cleanup; + + old_umask = umask(077); + if (virFileMakePath(logdir) < 0) { + umask(old_umask); + goto cleanup; + } + umask(old_umask); + + if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s", + virLogDefaultPriority, logdir, filename) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(logdir); + return ret; +} + + +/* + * virLogSetDefaultOutput: + * @filename: the file that the output should be redirected to (only needed + * when @godaemon equals true + * @godaemon: whether we're running daemonized + * @privileged: whether we're running with root privileges or not (session) + * + * Decides on what the default output (journald, file, stderr) should be + * according to @filename, @godaemon, @privileged. This function should be run + * exactly once at daemon startup, so no locks are used. + * + * Returns 0 on success, -1 in case of a failure. + */ +int +virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged) +{ + if (!godaemon) + return virLogSetDefaultOutputToStderr(); + + if (access("/run/systemd/journal/socket", W_OK) >= 0) + return virLogSetDefaultOutputToJournald(); + + return virLogSetDefaultOutputToFile(filename, privileged); +} + + +char * +virLogGetDefaultOutput(void) +{ + return virLogDefaultOutput; +} + + static const char * virLogPriorityString(virLogPriority lvl) { diff --git a/src/util/virlog.h b/src/util/virlog.h index 3f2d422..b4ffeca 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -189,6 +189,8 @@ void virLogFilterFree(virLogFilterPtr filter); void virLogFilterListFree(virLogFilterPtr *list, int count); int virLogSetOutputs(const char *outputs) ATTRIBUTE_NONNULL(1); int virLogSetFilters(const char *filters); +char *virLogGetDefaultOutput(void); +int virLogSetDefaultOutput(const char *fname, bool godaemon, bool privileged); /* * Internal logging API -- 2.5.5

Along with an empty string, it should also be possible for users to pass NULL to the public APIs which in turn would trigger a routine (future work) responsible for defining an appropriate default logging output given the current circumstances. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- daemon/libvirtd.c | 2 +- src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/util/virlog.c | 8 +++++++- src/util/virlog.h | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index cd25b50..3902a8b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -693,7 +693,7 @@ daemonSetupLogging(struct daemonConfig *config, if (virLogGetNbFilters() == 0) virLogSetFilters(config->log_filters); - if (config->log_outputs && virLogGetNbOutputs() == 0) + if (virLogGetNbOutputs() == 0) virLogSetOutputs(config->log_outputs); /* diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 02745be..9ee818e 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -478,7 +478,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, if (virLogGetNbFilters() == 0) virLogSetFilters(config->log_filters); - if (config->log_outputs && virLogGetNbOutputs() == 0) + if (virLogGetNbOutputs() == 0) virLogSetOutputs(config->log_outputs); /* diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 04bb836..a9aebdb 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -406,7 +406,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, if (virLogGetNbFilters() == 0) virLogSetFilters(config->log_filters); - if (config->log_outputs && virLogGetNbOutputs() == 0) + if (virLogGetNbOutputs() == 0) virLogSetOutputs(config->log_outputs); /* diff --git a/src/util/virlog.c b/src/util/virlog.c index 9b52e66..acd2285 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1818,6 +1818,8 @@ virLogParseFilters(const char *src, virLogFilterPtr **filters) * @outputs: string defining a (set of) output(s) * * Replaces the current set of defined outputs with a new set of outputs. + * Should the set be empty or NULL, a default output is used according to the + * daemon's runtime attributes. * * Returns 0 on success or -1 in case of an error. */ @@ -1826,12 +1828,16 @@ virLogSetOutputs(const char *src) { int ret = -1; int noutputs = 0; + const char *outputstr = virLogDefaultOutput; virLogOutputPtr *outputs = NULL; if (virLogInitialize() < 0) return -1; - if ((noutputs = virLogParseOutputs(src, &outputs)) < 0) + if (src && *src) + outputstr = src; + + if ((noutputs = virLogParseOutputs(outputstr, &outputs)) < 0) goto cleanup; if (virLogDefineOutputs(outputs, noutputs) < 0) diff --git a/src/util/virlog.h b/src/util/virlog.h index b4ffeca..cc09f48 100644 --- a/src/util/virlog.h +++ b/src/util/virlog.h @@ -187,7 +187,7 @@ void virLogOutputFree(virLogOutputPtr output); void virLogOutputListFree(virLogOutputPtr *list, int count); void virLogFilterFree(virLogFilterPtr filter); void virLogFilterListFree(virLogFilterPtr *list, int count); -int virLogSetOutputs(const char *outputs) ATTRIBUTE_NONNULL(1); +int virLogSetOutputs(const char *outputs); int virLogSetFilters(const char *filters); char *virLogGetDefaultOutput(void); int virLogSetDefaultOutput(const char *fname, bool godaemon, bool privileged); -- 2.5.5

Now that virLog{Get,Set}DefaultOutput routines are introduced we can wire them up to the daemon's logging initialization code. Also, change the order of operations a bit so that we still strictly honor our precedence of settings: cmdline > env > config now that outputs and filters are not appended anymore. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- daemon/libvirtd.c | 96 +++++++---------------------------------------- src/locking/lock_daemon.c | 90 +++++++------------------------------------- src/logging/log_daemon.c | 92 +++++++-------------------------------------- 3 files changed, 40 insertions(+), 238 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3902a8b..b2e89fd 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -675,103 +675,33 @@ daemonSetupLogging(struct daemonConfig *config, * Libvirtd's order of precedence is: * cmdline > environment > config * - * In order to achieve this, we must process configuration in - * different order for the log level versus the filters and - * outputs. Because filters and outputs append, we have to look at - * the environment first and then only check the config file if - * there was no result from the environment. The default output is - * then applied only if there was no setting from either of the - * first two. Because we don't have a way to determine if the log - * level has been set, we must process variables in the opposite + * The default output is applied only if there was no setting from either + * the config or the environment. Because we don't have a way to determine + * if the log level has been set, we must process variables in the opposite * order, each one overriding the previous. */ if (config->log_level != 0) virLogSetDefaultPriority(config->log_level); + if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0) + return -1; + + /* In case the config is empty, the filters become empty and outputs will + * be set to default + */ + virLogSetFilters(config->log_filters); + virLogSetOutputs(config->log_outputs); + + /* If there are some environment variables defined, use those instead */ virLogSetFromEnv(); - if (virLogGetNbFilters() == 0) - virLogSetFilters(config->log_filters); - - if (virLogGetNbOutputs() == 0) - virLogSetOutputs(config->log_outputs); - /* * Command line override for --verbose */ if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) virLogSetDefaultPriority(VIR_LOG_INFO); - /* - * If no defined outputs, and either running - * as daemon or not on a tty, then first try - * to direct it to the systemd journal - * (if it exists).... - */ - if (virLogGetNbOutputs() == 0 && - (godaemon || !isatty(STDIN_FILENO))) { - char *tmp; - if (access("/run/systemd/journal/socket", W_OK) >= 0) { - virLogPriority priority = virLogGetDefaultPriority(); - - /* By default we don't want to log too much stuff into journald as - * it may employ rate limiting and thus block libvirt execution. */ - if (priority == VIR_LOG_DEBUG) - priority = VIR_LOG_INFO; - - if (virAsprintf(&tmp, "%d:journald", priority) < 0) - goto error; - virLogSetOutputs(tmp); - VIR_FREE(tmp); - } - } - - /* - * otherwise direct to libvirtd.log when running - * as daemon. Otherwise the default output is stderr. - */ - if (virLogGetNbOutputs() == 0) { - char *tmp = NULL; - - if (godaemon) { - if (privileged) { - if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/libvirtd.log", - virLogGetDefaultPriority(), - LOCALSTATEDIR) == -1) - goto error; - } else { - char *logdir = virGetUserCacheDirectory(); - mode_t old_umask; - - if (!logdir) - goto error; - - old_umask = umask(077); - if (virFileMakePath(logdir) < 0) { - umask(old_umask); - goto error; - } - umask(old_umask); - - if (virAsprintf(&tmp, "%d:file:%s/libvirtd.log", - virLogGetDefaultPriority(), logdir) == -1) { - VIR_FREE(logdir); - goto error; - } - VIR_FREE(logdir); - } - } else { - if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) - goto error; - } - virLogSetOutputs(tmp); - VIR_FREE(tmp); - } - return 0; - - error: - return -1; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 9ee818e..881af11 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -460,97 +460,33 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, * Libvirtd's order of precedence is: * cmdline > environment > config * - * In order to achieve this, we must process configuration in - * different order for the log level versus the filters and - * outputs. Because filters and outputs append, we have to look at - * the environment first and then only check the config file if - * there was no result from the environment. The default output is - * then applied only if there was no setting from either of the - * first two. Because we don't have a way to determine if the log - * level has been set, we must process variables in the opposite + * The default output is applied only if there was no setting from either + * the config or the environment. Because we don't have a way to determine + * if the log level has been set, we must process variables in the opposite * order, each one overriding the previous. */ if (config->log_level != 0) virLogSetDefaultPriority(config->log_level); + if (virLogSetDefaultOutput("virtlockd.log", godaemon, privileged) < 0) + return -1; + + /* In case the config is empty, the filters become empty and outputs will + * be set to default + */ + virLogSetFilters(config->log_filters); + virLogSetOutputs(config->log_outputs); + + /* If there are some environment variables defined, use those instead */ virLogSetFromEnv(); - if (virLogGetNbFilters() == 0) - virLogSetFilters(config->log_filters); - - if (virLogGetNbOutputs() == 0) - virLogSetOutputs(config->log_outputs); - /* * Command line override for --verbose */ if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) virLogSetDefaultPriority(VIR_LOG_INFO); - /* - * If no defined outputs, and either running - * as daemon or not on a tty, then first try - * to direct it to the systemd journal - * (if it exists).... - */ - if (virLogGetNbOutputs() == 0 && - (godaemon || !isatty(STDIN_FILENO))) { - char *tmp; - if (access("/run/systemd/journal/socket", W_OK) >= 0) { - if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) - goto error; - virLogSetOutputs(tmp); - VIR_FREE(tmp); - } - } - - /* - * otherwise direct to libvirtd.log when running - * as daemon. Otherwise the default output is stderr. - */ - if (virLogGetNbOutputs() == 0) { - char *tmp = NULL; - - if (godaemon) { - if (privileged) { - if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/virtlockd.log", - virLogGetDefaultPriority(), - LOCALSTATEDIR) == -1) - goto error; - } else { - char *logdir = virGetUserCacheDirectory(); - mode_t old_umask; - - if (!logdir) - goto error; - - old_umask = umask(077); - if (virFileMakePath(logdir) < 0) { - VIR_FREE(logdir); - umask(old_umask); - goto error; - } - umask(old_umask); - - if (virAsprintf(&tmp, "%d:file:%s/virtlockd.log", - virLogGetDefaultPriority(), logdir) == -1) { - VIR_FREE(logdir); - goto error; - } - VIR_FREE(logdir); - } - } else { - if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) - goto error; - } - virLogSetOutputs(tmp); - VIR_FREE(tmp); - } - return 0; - - error: - return -1; } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index a9aebdb..793811e 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -385,100 +385,36 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, virLogReset(); /* - * virtlogd's order of precedence is: + * Libvirtd's order of precedence is: * cmdline > environment > config * - * In order to achieve this, we must process configuration in - * different order for the log level versus the filters and - * outputs. Because filters and outputs append, we have to look at - * the environment first and then only check the config file if - * there was no result from the environment. The default output is - * then applied only if there was no setting from either of the - * first two. Because we don't have a way to determine if the log - * level has been set, we must process variables in the opposite + * The default output is applied only if there was no setting from either + * the config or the environment. Because we don't have a way to determine + * if the log level has been set, we must process variables in the opposite * order, each one overriding the previous. */ if (config->log_level != 0) virLogSetDefaultPriority(config->log_level); + if (virLogSetDefaultOutput("virtlogd.log", godaemon, privileged) < 0) + return -1; + + /* In case the config is empty, the filters become empty and outputs will + * be set to default + */ + virLogSetFilters(config->log_filters); + virLogSetOutputs(config->log_outputs); + + /* If there are some environment variables defined, use those instead */ virLogSetFromEnv(); - if (virLogGetNbFilters() == 0) - virLogSetFilters(config->log_filters); - - if (virLogGetNbOutputs() == 0) - virLogSetOutputs(config->log_outputs); - /* * Command line override for --verbose */ if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) virLogSetDefaultPriority(VIR_LOG_INFO); - /* - * If no defined outputs, and either running - * as daemon or not on a tty, then first try - * to direct it to the systemd journal - * (if it exists).... - */ - if (virLogGetNbOutputs() == 0 && - (godaemon || !isatty(STDIN_FILENO))) { - char *tmp; - if (access("/run/systemd/journal/socket", W_OK) >= 0) { - if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) - goto error; - virLogSetOutputs(tmp); - VIR_FREE(tmp); - } - } - - /* - * otherwise direct to libvirtd.log when running - * as daemon. Otherwise the default output is stderr. - */ - if (virLogGetNbOutputs() == 0) { - char *tmp = NULL; - - if (godaemon) { - if (privileged) { - if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/virtlogd.log", - virLogGetDefaultPriority(), - LOCALSTATEDIR) == -1) - goto error; - } else { - char *logdir = virGetUserCacheDirectory(); - mode_t old_umask; - - if (!logdir) - goto error; - - old_umask = umask(077); - if (virFileMakePath(logdir) < 0) { - umask(old_umask); - VIR_FREE(logdir); - goto error; - } - umask(old_umask); - - if (virAsprintf(&tmp, "%d:file:%s/virtlogd.log", - virLogGetDefaultPriority(), logdir) == -1) { - VIR_FREE(logdir); - goto error; - } - VIR_FREE(logdir); - } - } else { - if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) - goto error; - } - virLogSetOutputs(tmp); - VIR_FREE(tmp); - } - return 0; - - error: - return -1; } -- 2.5.5

Enable libvirt users to query logging output settings. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- daemon/admin.c | 37 +++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++++- src/admin/admin_remote.c | 35 +++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 5 +++++ 8 files changed, 146 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index a3c8b89..4fa3851 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -383,4 +383,41 @@ adminDispatchServerSetClientLimits(virNetServerPtr server ATTRIBUTE_UNUSED, virObjectUnref(srv); return rv; } + +/* Returns the number of outputs stored in @outputs */ +static int +adminConnectGetLoggingOutputs(char **outputs, unsigned int flags) +{ + char *tmp = NULL; + + virCheckFlags(0, -1); + + if (!(tmp = virLogGetOutputs())) + return -1; + + *outputs = tmp; + return virLogGetNbOutputs(); +} + +static int +adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + admin_connect_get_logging_outputs_args *args, + admin_connect_get_logging_outputs_ret *ret) +{ + char *outputs = NULL; + int noutputs = 0; + + if ((noutputs = adminConnectGetLoggingOutputs(&outputs, args->flags) < 0)) { + virNetMessageSaveError(rerr); + return -1; + } + + VIR_STEAL_PTR(ret->outputs, outputs); + ret->noutputs = noutputs; + + return 0; +} #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index c810be3..46d2155 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -404,6 +404,10 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv, int nparams, unsigned int flags); +int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, + char **outputs, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 5963233..237632a 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -183,6 +183,15 @@ struct admin_server_set_client_limits_args { unsigned int flags; }; +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; @@ -268,5 +277,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13 + ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13, + + /** + * @generate: none + */ + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index f37ff5e..e472484 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -425,3 +425,38 @@ remoteAdminServerSetClientLimits(virAdmServerPtr srv, virObjectUnlock(priv); return rv; } + +static int +remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn, + char **outputs, + unsigned int flags) +{ + int rv = -1; + 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) + VIR_STEAL_PTR(*outputs, ret.outputs); + + rv = ret.noutputs; + 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 1437d9e..096bf5a 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -127,6 +127,13 @@ struct admin_server_set_client_limits_args { } params; u_int flags; }; +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, @@ -141,4 +148,5 @@ enum admin_procedure { ADMIN_PROC_CLIENT_CLOSE = 11, ADMIN_PROC_SERVER_GET_CLIENT_LIMITS = 12, ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13, + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 8877e49..13cba1d 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -1084,3 +1084,43 @@ virAdmServerSetClientLimits(virAdmServerPtr srv, virDispatchError(NULL); return ret; } + +/** + * virAdmConnectGetLoggingOutputs: + * @conn: pointer to an active admin connection + * @outputs: pointer to a variable to store a string containing all currently + * defined logging outputs on daemon (allocated automatically) or + * NULL if just the number of defined outputs is required + * @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 an automatically allocated 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); + + 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 191e3f2..7160af6 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -10,6 +10,8 @@ xdr_admin_client_close_args; xdr_admin_client_get_info_args; xdr_admin_client_get_info_ret; xdr_admin_connect_get_lib_version_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 2de28e9..9343a4f 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -39,3 +39,8 @@ LIBVIRT_ADMIN_2.0.0 { virAdmServerGetClientLimits; virAdmServerSetClientLimits; }; + +LIBVIRT_ADMIN_2.5.0 { + global: + virAdmConnectGetLoggingOutputs; +} LIBVIRT_ADMIN_2.0.0; -- 2.5.5

Enable libvirt users to query logging filter settings. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- daemon/admin.c | 47 +++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++- src/admin/admin_remote.c | 35 ++++++++++++++++++++++++++++++ src/admin_protocol-structs | 8 +++++++ src/libvirt-admin.c | 41 +++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 153 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 4fa3851..f3bc1de 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -399,6 +399,22 @@ adminConnectGetLoggingOutputs(char **outputs, unsigned int flags) return virLogGetNbOutputs(); } +/* Returns the number of defined filters or -1 in case of an error */ +static int +adminConnectGetLoggingFilters(char **filters, unsigned int flags) +{ + char *tmp = NULL; + int ret = 0; + + virCheckFlags(0, -1); + + if ((ret = virLogGetNbFilters()) > 0 && !(tmp = virLogGetFilters())) + return -1; + + *filters = tmp; + return ret; +} + static int adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, @@ -420,4 +436,35 @@ adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED, return 0; } + +static int +adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + admin_connect_get_logging_filters_args *args, + admin_connect_get_logging_filters_ret *ret) +{ + char *filters = NULL; + int nfilters = 0; + + if ((nfilters = adminConnectGetLoggingFilters(&filters, args->flags)) < 0) { + virNetMessageSaveError(rerr); + return -1; + } + + if (nfilters == 0) { + ret->filters = NULL; + } else { + char **ret_filters = NULL; + if (VIR_ALLOC(ret_filters) < 0) + return -1; + + *ret_filters = filters; + ret->filters = ret_filters; + } + ret->nfilters = nfilters; + + return 0; +} #include "admin_dispatch.h" diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 46d2155..d76ac20 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -408,6 +408,10 @@ int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, char **outputs, unsigned int flags); +int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, + char **filters, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 237632a..8316bc4 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -192,6 +192,15 @@ struct admin_connect_get_logging_outputs_ret { unsigned int noutputs; }; +struct admin_connect_get_logging_filters_args { + unsigned int flags; +}; + +struct admin_connect_get_logging_filters_ret { + admin_string filters; + unsigned int nfilters; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -282,5 +291,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14 + ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14, + + /** + * @generate: none + */ + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index e472484..b29d109 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -460,3 +460,38 @@ remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn, virObjectUnlock(priv); return rv; } + +static int +remoteAdminConnectGetLoggingFilters(virAdmConnectPtr conn, + char **filters, + unsigned int flags) +{ + int rv = -1; + 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) + *filters = ret.filters ? *ret.filters : NULL; + + rv = ret.nfilters; + VIR_FREE(ret.filters); + + done: + virObjectUnlock(priv); + return rv; +} diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 096bf5a..c172557 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -134,6 +134,13 @@ struct admin_connect_get_logging_outputs_ret { admin_nonnull_string outputs; u_int noutputs; }; +struct admin_connect_get_logging_filters_args { + u_int flags; +}; +struct admin_connect_get_logging_filters_ret { + admin_string filters; + u_int nfilters; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -149,4 +156,5 @@ enum admin_procedure { ADMIN_PROC_SERVER_GET_CLIENT_LIMITS = 12, ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13, ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14, + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 13cba1d..29754a8 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -1124,3 +1124,44 @@ virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectGetLoggingFilters: + * @conn: pointer to an active admin connection + * @filters: pointer to a variable to store a string containing all currently + * defined logging filters on daemon (allocated automatically) or + * NULL if just the number of defined outputs is required + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Retrieves a list of currently installed logging filters. Filters returned + * are contained within an automatically allocated string and delimited by + * spaces. The format of each filter conforms to the format described in + * daemon's configuration file (e.g. libvirtd.conf). + * + * To retrieve individual filters, additional parsing needs to be done by the + * caller. Caller is also responsible for freeing @filters correctly. + * + * Returns the number of filters returned in @filters, or -1 in case of + * an error. + */ +int +virAdmConnectGetLoggingFilters(virAdmConnectPtr conn, + char **filters, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, flags=%x", conn, flags); + + virResetLastError(); + virCheckAdmConnectReturn(conn, -1); + + 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 7160af6..f2e03d1 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -10,6 +10,8 @@ xdr_admin_client_close_args; xdr_admin_client_get_info_args; xdr_admin_client_get_info_ret; xdr_admin_connect_get_lib_version_ret; +xdr_admin_connect_get_logging_filters_args; +xdr_admin_connect_get_logging_filters_ret; xdr_admin_connect_get_logging_outputs_args; xdr_admin_connect_get_logging_outputs_ret; xdr_admin_connect_list_servers_args; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 9343a4f..234c50e 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -43,4 +43,5 @@ LIBVIRT_ADMIN_2.0.0 { LIBVIRT_ADMIN_2.5.0 { global: virAdmConnectGetLoggingOutputs; + virAdmConnectGetLoggingFilters; } LIBVIRT_ADMIN_2.0.0; -- 2.5.5

On Fri, Dec 09, 2016 at 06:59:57AM -0500, John Ferlan wrote: I almost forgot to reply to this patch. Unfortunately, VIR_STEAL_PTR is not enough and that's because of the way XDR stores pointers - string that actually can be NULL is defined as char **, so you have to allocate the 1 member array because what virNetServerProgramDispatchCall will then do is call xdr_free() which will try to free both ret->filters and *ret->filters - it's just how XDR defines it. I remember wanting to do it as you suggest (the intuitive way) but then the daemon either crashed or I got an RPC error so I looked at the XDR spec and found out that to be able to pass both NULL and string data in the same data type, double pointer is needed. Erik

On 11/25/2016 08:12 AM, Erik Skultety wrote: [...] s/2.5.0/3.0.0 Of course makes me think "eventually" would someone want or find it useful to have "Add" or "Remove" options too rather than just wholesale replacement... Maybe it's just the fresh coffee talking. ACK John

Enable libvirt users to modify logging filters of a daemon from outside. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 | 38 ++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + 7 files changed, 70 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 16a3453..c5678bb 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -426,6 +426,16 @@ adminConnectSetLoggingOutputs(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, } static int +adminConnectSetLoggingFilters(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *filters, + unsigned int flags) +{ + virCheckFlags(0, -1); + + return virLogSetFilters(filters); +} + +static int adminDispatchConnectGetLoggingOutputs(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 aa33fef..161727e 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -416,6 +416,10 @@ int virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn, const char *outputs, 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 1d2116d..d19d132 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -206,6 +206,11 @@ struct admin_connect_set_logging_outputs_args { unsigned int flags; }; +struct admin_connect_set_logging_filters_args { + admin_string filters; + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -306,5 +311,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16 + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 04bbd7a..a6c5380 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -145,6 +145,10 @@ struct admin_connect_set_logging_outputs_args { admin_string outputs; u_int flags; }; +struct admin_connect_set_logging_filters_args { + admin_string filters; + u_int flags; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -162,4 +166,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14, ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15, ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16, + ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 2bc8b2c..67d9b6b 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -1203,3 +1203,41 @@ virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectSetLoggingFilters: + * @conn: pointer to an active admin connection + * @filters: pointer to a string containing a list of filters to be defined + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Redefine 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. The format of each filter must conform to the format described in + * daemon's configuration file (e.g. libvirtd.conf). + * + * To clear the currently defined (set of) filter(s), pass either an empty + * string ("") or NULL in @filters. + * + * Returns 0 if the new filter or the set of filters has been defined + * successfully, 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); + + 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 ad7fd1f..9526412 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -19,6 +19,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_outputs_args; xdr_admin_server_get_client_limits_args; xdr_admin_server_get_client_limits_ret; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index d4713da..4f0be7f 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -45,4 +45,5 @@ LIBVIRT_ADMIN_2.5.0 { virAdmConnectGetLoggingOutputs; virAdmConnectGetLoggingFilters; virAdmConnectSetLoggingOutputs; + virAdmConnectSetLoggingFilters; } LIBVIRT_ADMIN_2.0.0; -- 2.5.5

Finally, now that all APIs have been introduced, wire them up to virt-admin and introduce daemon-log-outputs and daemon-log-filters commands. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- tools/virt-admin.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 90 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 70b0e51..0b9945a 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -971,6 +971,114 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd) goto cleanup; } +/* -------------------------- + * Command daemon-log-filters + * -------------------------- + */ +static const vshCmdInfo info_daemon_log_filters[] = { + {.name = "help", + .data = N_("fetch or set the currently defined set of logging filters on " + "daemon") + }, + {.name = "desc", + .data = N_("Depending on whether run with or without options, the command " + "fetches or redefines the existing active set of filters on " + "daemon.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_daemon_log_filters[] = { + {.name = "filters", + .type = VSH_OT_STRING, + .help = N_("redefine the existing set of logging filters"), + .flags = VSH_OFLAG_EMPTY_OK + }, + {.name = NULL} +}; + +static bool +cmdDaemonLogFilters(vshControl *ctl, const vshCmd *cmd) +{ + int nfilters; + char *filters = NULL; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptBool(cmd, "filters")) { + if ((vshCommandOptStringReq(ctl, cmd, "filters", + (const char **) &filters) < 0 || + virAdmConnectSetLoggingFilters(priv->conn, filters, 0) < 0)) { + vshError(ctl, _("Unable to change daemon logging settings")); + return false; + } + } else { + if ((nfilters = virAdmConnectGetLoggingFilters(priv->conn, + &filters, 0)) < 0) { + vshError(ctl, _("Unable to get daemon logging filters information")); + return false; + } + + vshPrintExtra(ctl, " %-15s", _("Logging filters: ")); + vshPrint(ctl, "%s\n", filters ? filters : ""); + } + + return true; +} + +/* -------------------------- + * Command daemon-log-outputs + * -------------------------- + */ +static const vshCmdInfo info_daemon_log_outputs[] = { + {.name = "help", + .data = N_("fetch or set the currently defined set of logging outputs on " + "daemon") + }, + {.name = "desc", + .data = N_("Depending on whether run with or without options, the command " + "fetches or redefines the existing active set of outputs on " + "daemon.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_daemon_log_outputs[] = { + {.name = "outputs", + .type = VSH_OT_STRING, + .help = N_("redefine the existing set of logging outputs"), + .flags = VSH_OFLAG_EMPTY_OK + }, + {.name = NULL} +}; + +static bool +cmdDaemonLogOutputs(vshControl *ctl, const vshCmd *cmd) +{ + int noutputs; + char *outputs = NULL; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptBool(cmd, "outputs")) { + if ((vshCommandOptStringReq(ctl, cmd, "outputs", + (const char **) &outputs) < 0 || + virAdmConnectSetLoggingOutputs(priv->conn, outputs, 0) < 0)) { + vshError(ctl, _("Unable to change daemon logging settings")); + return false; + } + } else { + if ((noutputs = virAdmConnectGetLoggingOutputs(priv->conn, + &outputs, 0)) < 0) { + vshError(ctl, _("Unable to get daemon logging filters information")); + return false; + } + + vshPrintExtra(ctl, " %-15s", _("Logging outputs: ")); + vshPrint(ctl, "%s\n", outputs ? outputs : ""); + } + + return true; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -1341,6 +1449,18 @@ static const vshCmdDef managementCmds[] = { .info = info_srv_clients_set, .flags = 0 }, + {.name = "daemon-log-filters", + .handler = cmdDaemonLogFilters, + .opts = opts_daemon_log_filters, + .info = info_daemon_log_filters, + .flags = 0 + }, + {.name = "daemon-log-outputs", + .handler = cmdDaemonLogOutputs, + .opts = opts_daemon_log_outputs, + .info = info_daemon_log_outputs, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 443730e..d3f1a35 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -155,6 +155,96 @@ change its internal configuration. Lists all manageable servers contained within the daemon the client is currently connected to. +=item B<daemon-log-info> [I<--outputs> B<string>] [I<--filters> B<string>] + +Print the currently defined set of logging outputs and/or filters. + +=over 4 + +=item I<--outputs> + +Print the currently defined set of logging outputs + +=item I<--filters> + +Print the currently defined set of logging filters + +=back + +=item B<daemon-log-define> + +Redefine the currently defined sets of logging outputs and/or filters with +completely new ones. + +=over 4 + +=item I<--outputs> + +Replaces the current set of logging outputs with a new one where multiple +outputs are delimited by space and each output must be of the following form: + +I<< X:<destination>:<auxiliary_data> >> where + +=over 4 + +=item * I<X> denotes the minimal debug level: + +=over 4 + +=item * 1: DEBUG + +=item * 2: INFO + +=item * 3: WARNING + +=item * 4: ERROR + +=back + +=item * I<< <destination> >> is one of I<stderr>, I<file>, I<syslog>, or I<journald> + +=item * I<< <auxiliary_data> >> is only required for the I<file> and I<syslog> +destination types and shall therefore contain either a path to a file or a message tag +respectively, see the B<Examples> section below. + +=back + +=item I<--filters> + +Replaces the current set of logging filters with a new one where multiple +filters are delimited by space and each filter must be of the following form: + +I<< X:<match_string> >> where + +=over 4 + +=item * I<X> denotes the minimal debug level the same way as for I<--outputs> + +=item * I<< <match_string> >> references either a specific module in libvirt's +source tree or the whole directory in the source tree, thus affecting all +modules in the directory + +=back + +=back + +B<Examples> + To replace the current logging output with one that writes to a file while + logging logging errors only, the following could be used: + + $ virt-admin daemon-log-define --outputs "4:file:<path_to_the_file>" + + To define multiple outputs at once: + + $ virt-admin daemon-log-define --outputs "4:stderr 2:syslog:<tag>" + + To define a filter which suppresses all e.g. 'virObjectUnref' DEBUG + messages, use the following (note the '.' symbol which can be used to more + fine-grained filters tailored to specific modules, in contrast, to affect + a whole directory containing several modules it would become "4:util"): + + $ virt-admin daemon-log-define --filters "4:util.object" + =back =head1 SERVER COMMANDS -- 2.5.5

On Fri, Dec 09, 2016 at 07:29:36AM -0500, John Ferlan wrote: Is it an issue though? If someone sends a binary garbage for filename, we have to create a logfile with that name on the filesystem. When we retrieve it, I think we should print it as we created it - garbage. The other option we have is to strip all the control characters and print the rest which in the worst case scenario will end up as an empty string... Not sure what you mean, should I just add a reference or replace the sections explaining the format with the reference completely? Erik

Provide a simple C example demonstrating the use of both query APIs as well as setter APIs. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- .gitignore | 1 + examples/Makefile.am | 3 +- examples/admin/logging.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 examples/admin/logging.c diff --git a/.gitignore b/.gitignore index 879ec24..984ad07 100644 --- a/.gitignore +++ b/.gitignore @@ -79,6 +79,7 @@ /examples/admin/client_limits /examples/admin/list_clients /examples/admin/list_servers +/examples/admin/logging /examples/admin/threadpool_params /examples/object-events/event-test /examples/dominfo/info1 diff --git a/examples/Makefile.am b/examples/Makefile.am index 7cb8258..2956e14 100644 --- a/examples/Makefile.am +++ b/examples/Makefile.am @@ -43,7 +43,7 @@ noinst_PROGRAMS=dominfo/info1 dommigrate/dommigrate domsuspend/suspend \ domtop/domtop hellolibvirt/hellolibvirt object-events/event-test \ openauth/openauth rename/rename admin/list_servers admin/list_clients \ admin/threadpool_params admin/client_limits admin/client_info \ - admin/client_close + admin/client_close admin/logging dominfo_info1_SOURCES = dominfo/info1.c dommigrate_dommigrate_SOURCES = dommigrate/dommigrate.c @@ -65,6 +65,7 @@ admin_threadpool_params_SOURCES = admin/threadpool_params.c admin_client_limits_SOURCES = admin/client_limits.c admin_client_info_SOURCES = admin/client_info.c admin_client_close_SOURCES = admin/client_close.c +admin_logging_SOURCES = admin/logging.c if WITH_APPARMOR_PROFILES apparmordir = $(sysconfdir)/apparmor.d/ diff --git a/examples/admin/logging.c b/examples/admin/logging.c new file mode 100644 index 0000000..e28c7d5 --- /dev/null +++ b/examples/admin/logging.c @@ -0,0 +1,102 @@ +#include<stdio.h> +#include<stdlib.h> +#include<stdbool.h> + +#include "config.h" +#include<unistd.h> +#include<libvirt/libvirt-admin.h> +#include<libvirt/virterror.h> + +static void printHelp(const char *argv0) +{ + fprintf(stderr, + ("Usage:\n" + " %s [options]\n" + "\n" + "Options:\n" + " -h Print this message.\n" + " -o [string] Specify new log outputs.\n" + " -f [string] Specify new log filters.\n" + "\n"), + argv0); +} + +int main(int argc, char **argv) +{ + int ret, c; + virAdmConnectPtr conn = NULL; + char *get_outputs = NULL; + char *get_filters = NULL; + const char *set_outputs = NULL; + const char *set_filters = NULL; + + ret = c = -1; + opterr = 0; + + while ((c = getopt(argc, argv, ":hpo:f:")) > 0) { + switch (c) { + case 'h': + printHelp(argv[0]); + exit(EXIT_SUCCESS); + case 'o': + set_outputs = optarg; + break; + case 'f': + set_filters = optarg; + break; + case ':': + fprintf(stderr, "Missing argument for option -%c\n", optopt); + exit(EXIT_FAILURE); + case '?': + fprintf(stderr, "Unrecognized option '-%c'\n", optopt); + exit(EXIT_FAILURE); + } + } + + /* first, open a connection to the daemon */ + if (!(conn = virAdmConnectOpen(NULL, 0))) + goto cleanup; + + /* get the currently defined log outputs and filters */ + if (virAdmConnectGetLoggingOutputs(conn, &get_outputs, 0) < 0 || + virAdmConnectGetLoggingFilters(conn, &get_filters, 0) < 0) + goto cleanup; + + fprintf(stdout, + "Current settings:\n" + " outputs: %s\n" + " filters: %s\n" + "\n", + get_outputs, get_filters ? get_filters : "None"); + + free(get_outputs); + free(get_filters); + + /* now, try to change the redefine the current log output and filters */ + if (virAdmConnectSetLoggingOutputs(conn, set_outputs, 0) < 0) + goto cleanup; + + if (virAdmConnectSetLoggingFilters(conn, set_filters, 0) < 0) + goto cleanup; + + /* get the currently defined log outputs and filters */ + if (virAdmConnectGetLoggingOutputs(conn, &get_outputs, 0) < 0 || + virAdmConnectGetLoggingFilters(conn, &get_filters, 0) < 0) + goto cleanup; + + fprintf(stdout, + "New settings:\n" + " outputs: %s\n" + " filters: %s\n" + "\n", + get_outputs ? get_outputs : "Default", + get_filters ? get_filters : "None"); + + free(get_outputs); + free(get_filters); + + ret = 0; + cleanup: + virAdmConnectClose(conn); + return ret; +} -- 2.5.5

Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- docs/news.html.in | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/news.html.in b/docs/news.html.in index 59bf20d..f2a65f5 100644 --- a/docs/news.html.in +++ b/docs/news.html.in @@ -31,6 +31,10 @@ Add the capability to pass through a scsi_host HBA and the associated LUNs to the guest. </li> + <li>admin-logging: Add support for runtime logging settings adjustment<br/> + Logging-related settings like log outputs and filters can now be + adjusted during runtime without the necessity of the daemon's restart. + </li> </ul> </li> <li><strong>Improvements</strong> -- 2.5.5