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@xxxxxxxxxx>
> > ---
> > 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
> >
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
