On Fri, Dec 09, 2016 at 07:29:36AM -0500, John Ferlan wrote:
On 11/25/2016 08:12 AM, Erik Skultety wrote:
> 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>
> ---
> tools/virt-admin.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/virt-admin.pod | 90 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 210 insertions(+)
>
Now it *is* the coffee - should there be any concerns along the way over
escaping characters? I think this is the only place where we
read/print, but via the API if someone passes a string - do we need to
worry about that? Yes, I know - not the normal thing, but you know
someone will try...
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...
> 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
All of this is way off - no such command as daemon-log-info or
daemon-log-define.
I think you should reference /etc/libvirt/libvirtd.conf in order to
point at the place which describes the format.
Not sure what you mean, should I just add a reference or replace the sections
explaining the format with the reference completely?
Erik
Although I trust you know what should be there so...
ACK once you clean this up.
John
> @@ -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
>