On 11/01/2016 06:27 AM, Erik Skultety wrote:
> Enable libvirt users to modify daemon's logging output settings from outside.
> If an empty set is passed, a default logging output will be used the same way
> as it would be in case writing an empty string to the libvirtd.conf
>
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
> daemon/admin.c | 21 +++++++++++----------
> 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 +
> src/util/virlog.c | 5 ++++-
> 8 files changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/daemon/admin.c b/daemon/admin.c
> index c0598e0..79961b2 100644
> --- a/daemon/admin.c
> +++ b/daemon/admin.c
> @@ -392,17 +392,8 @@ adminConnectGetLoggingOutputs(char **outputs, unsigned int flags)
>
> virCheckFlags(0, -1);
>
> - if ((ret = virLogGetNbOutputs()) > 0) {
> - if (!(tmp = virLogGetOutputs()))
> + if ((ret = virLogGetNbOutputs()) > 0 && !(tmp = virLogGetOutputs()))
> return -1;
Indention is off
> - } else {
> - /* there were no outputs defined, return an empty string */
> - if (!tmp) {
> - if (VIR_ALLOC(tmp) < 0)
> - return -1;
> - memset(tmp, 0, 1);
> - }
> - }
>
> *outputs = tmp;
> return ret;
> @@ -433,6 +424,16 @@ adminConnectGetLoggingFilters(char **filters, unsigned int flags)
> }
>
> static int
> +adminConnectSetLoggingOutputs(virNetDaemonPtr dmn ATTRIBUTE_UNUSED,
> + const char *outputs,
> + unsigned int flags)
> +{
> + virCheckFlags(0, -1);
> +
> + return virLogSetOutputs(outputs);
> +}
> +
> +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 d76ac20..aa33fef 100644
> --- a/include/libvirt/libvirt-admin.h
> +++ b/include/libvirt/libvirt-admin.h
> @@ -412,6 +412,10 @@ int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn,
> 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 69aeb54..4a05928 100644
> --- a/src/admin/admin_protocol.x
> +++ b/src/admin/admin_protocol.x
> @@ -201,6 +201,11 @@ struct admin_connect_get_logging_filters_ret {
> unsigned int nfilters;
> };
>
> +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;
> @@ -296,5 +301,10 @@ enum admin_procedure {
> /**
> * @generate: none
> */
> - ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15
> + ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
> +
> + /**
> + * @generate: both
> + */
> + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16
> };
> diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
> index 41d065d..cbc99e3 100644
> --- a/src/admin_protocol-structs
> +++ b/src/admin_protocol-structs
> @@ -141,6 +141,10 @@ struct admin_connect_get_logging_filters_ret {
> admin_nonnull_string filters;
> u_int nfilters;
> };
> +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,
> @@ -157,4 +161,5 @@ enum admin_procedure {
> ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13,
> ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14,
> ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
> + ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
> };
> diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
> index 61a538c..01ae26c 100644
> --- a/src/libvirt-admin.c
> +++ b/src/libvirt-admin.c
> @@ -1185,3 +1185,39 @@ virAdmConnectGetLoggingFilters(virAdmConnectPtr conn,
> virDispatchError(NULL);
> return -1;
> }
> +
> +/**
> + * virAdmConnectSetLoggingOutputs:
> + * @conn: pointer to an active admin connection
> + * @outputs: pointer to a string containing a list of outputs to be defined
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Redefine the existing (set of) outputs(s) with a new one specified in
> + * @outputs. If multiple outputs are specified, they need to be delimited by
> + * spaces. The format of each output must conform to the format described in
> + * daemon's configuration file (e.g. libvirtd.conf).
If an empty sring ("") is passed, then a default will be used (restated
more cleanly of course!)
> + *
> + * Returns 0 if the new output or the set of outputs has been defined
> + * successfully, or -1 in case of an error.
> + */
> +int
> +virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
> + const char *outputs,
> + unsigned int flags)
> +{
> + int ret = -1;
> +
> + VIR_DEBUG("conn=%p, outputs=%s, flags=%x", conn, outputs, flags);
> +
> + virResetLastError();
> + virCheckAdmConnectReturn(conn, -1);
> + virCheckNonNullArgGoto(outputs, error);
This won't allow NULL outputs...
> +
> + 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 b36880d..d1ca034 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_outputs_args;
> xdr_admin_server_get_client_limits_args;
> xdr_admin_server_get_client_limits_ret;
> xdr_admin_server_get_threadpool_parameters_args;
> diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms
> index e6ee0e8..d39de42 100644
> --- a/src/libvirt_admin_public.syms
> +++ b/src/libvirt_admin_public.syms
> @@ -40,4 +40,5 @@ LIBVIRT_ADMIN_2.0.0 {
> virAdmServerSetClientLimits;
> virAdmConnectGetLoggingFilters;
> virAdmConnectGetLoggingOutputs;
> + virAdmConnectSetLoggingOutputs;
see patch 4 w/r/t new stanza...
> };
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 4ac72dc..d04a461 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -1820,6 +1820,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, a default output is used according to the daemon's
> + * runtime attributes.
> *
> * Returns 0 on success or -1 in case of an error.
> */
> @@ -1828,12 +1830,13 @@ virLogSetOutputs(const char *src)
> {
> int ret = -1;
> int noutputs = 0;
> + const char *outputstr = *src ? src : virLogGetDefaultOutput();
Not sure this is right - virAdmConnectSetLoggingOutputs says 'outputs'
must be NonNull.
I assume the generated remoteAdminConnectSetLoggingOutputs will end up
calling adminConnectSetLoggingOutputs with a NonNull 'outputs'.
Thus this code gets called with NonNull outputs.
Anyway, what you're really looking to do is check if the contents of
'outputs' is empty, then use some default value. In this case, since
this code "owns" virLogDefaultOutput is there really a reason to call
the API?
NB: even though virlog.h says outputs cannot be passed as NULL that's
somewhat of a misnomer since all the compiler is checking is that the
argument in the call stack isn't NULL - it doesn't check if the argument
being passed in actually NULL.
John
> virLogOutputPtr *outputs = NULL;
>
> if (virLogInitialize() < 0)
> return -1;
>
> - if ((noutputs = virLogParseOutputs(src, &outputs)) < 0)
> + if ((noutputs = virLogParseOutputs(outputstr, &outputs)) < 0)
> goto cleanup;
>
> if (virLogDefineOutputs(outputs, noutputs) < 0)
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
