Re: [PATCH 4/8] admin: Introduce virAdmConnectGetLoggingOutputs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux