On 11/01/2016 06:27 AM, Erik Skultety wrote:
> Enable libvirt users to query logging output settings.
>
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
> daemon/admin.c | 52 +++++++++++++++++++++++++++++++++++++++++
> 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 | 39 +++++++++++++++++++++++++++++++
> src/libvirt_admin_private.syms | 2 ++
> src/libvirt_admin_public.syms | 1 +
> 8 files changed, 164 insertions(+), 1 deletion(-)
>
> diff --git a/daemon/admin.c b/daemon/admin.c
> index a3c8b89..b31b125 100644
> --- a/daemon/admin.c
> +++ b/daemon/admin.c
> @@ -383,4 +383,56 @@ adminDispatchServerSetClientLimits(virNetServerPtr server ATTRIBUTE_UNUSED,
> virObjectUnref(srv);
> return rv;
> }
> +
/* Returns: Count of outputs? */
> +static int
> +adminConnectGetLoggingOutputs(char **outputs, unsigned int flags)
> +{
> + char *tmp = NULL;
> + int ret = -1;
> +
> + virCheckFlags(0, -1);
> +
> + if ((ret = virLogGetNbOutputs()) > 0) {
> + if (!(tmp = virLogGetOutputs()))
> + return -1;
> + } else {
> + /* there were no outputs defined, return an empty string */
> + if (!tmp) {
How would tmp have anything?
> + if (VIR_ALLOC(tmp) < 0)
> + return -1;
> + memset(tmp, 0, 1);
if (VIR_STRDUP(tmp, "") < 0)
return -1;
ret = 1;
> + }
> + }
> +
> + *outputs = tmp;
> + return ret;
> +}
> +
> +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;
> +
> + if (VIR_STRDUP(ret->outputs, outputs) < 0)
> + goto cleanup;
Would this be better served as VIR_STEAL_PTR(ret->outputs, outputs);
> +
> + ret->noutputs = noutputs;
> + rv = 0;
> +
> + cleanup:
> + if (rv < 0)
> + virNetMessageSaveError(rerr);
rerr not really UNUSED then?
The failure path could just be inlined thus removing need for cleanup
label and local rv
> + VIR_FREE(outputs);
> + return rv;
> +}
> #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 10a3b18..de3662c 100644
> --- a/src/admin/admin_remote.c
> +++ b/src/admin/admin_remote.c
> @@ -429,3 +429,46 @@ remoteAdminServerSetClientLimits(virAdmServerPtr srv,
> 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) {
virAdmConnectGetLoggingOutputs checks for NonNullArg(outputs)
So this doesn't seem necessary...
> + if (VIR_STRDUP(tmp_outputs, ret.outputs) < 0)
> + goto cleanup;
Similar - VIR_STEAL_PTR(*outputs, ret.outputs); should work, right?
> +
> + *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 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 88eef54..264411f 100644
> --- a/src/libvirt-admin.c
> +++ b/src/libvirt-admin.c
> @@ -1106,3 +1106,42 @@ 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)
> + * @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).
space between paragraphs
> + * 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);
So this seems to go against the remoteAdminConnectGetLoggingOutputs "if
(outputs)" needs/check.
> +
> + 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 8c173ab..c95aa3c 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..c1420ee 100644
> --- a/src/libvirt_admin_public.syms
> +++ b/src/libvirt_admin_public.syms
> @@ -38,4 +38,5 @@ LIBVIRT_ADMIN_2.0.0 {
> virAdmClientClose;
> virAdmServerGetClientLimits;
> virAdmServerSetClientLimits;
> + virAdmConnectGetLoggingOutputs;
> };
Doesn't this need to be a new stanza similar to libvirt_public.syms, e.g.:
LIBVIRT_ADMIN_2.5.0 {
global:
virAdmConnectGetLoggingOutputs;
} LIBVIRT_ADMIN_2.0.0;
with obvious additions for the next 3 patches.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
