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

Re: [libvirt] [PATCH REPOST 4/5] admin: Introduce virAdmServerSetClientProcessingControls



On 11.05.2016 12:20, Erik Skultety wrote:
> Opposite operation to virAdmServerGetClientProcessingControls. Understandably
> though, setting values for current number of clients connected or still waiting
> for authentication does not make sense, since changes to these values are event
> dependent, i.e. a client connects - counter is increased. Thus only the limits
> to maximum clients connected and waiting for authentication can be set. Should
> a request for other controls to be set arrive (provided such a setting will
> be first introduced to the config), the set of configuration controls can be
> later expanded (thanks to typed params). This patch also introduces a
> constraint that the maximum number of clients waiting for authentication has to
> be less than the overall maximum number of clients connected and any attempt to
> violate this constraint will be denied.
> 
> Signed-off-by: Erik Skultety <eskultet redhat com>
> ---
>  daemon/admin.c                  | 41 ++++++++++++++++++++++++++++++++++++
>  daemon/admin_server.c           | 35 +++++++++++++++++++++++++++++++
>  daemon/admin_server.h           |  5 +++++
>  include/libvirt/libvirt-admin.h |  5 +++++
>  src/admin/admin_protocol.x      | 13 +++++++++++-
>  src/admin/admin_remote.c        | 35 +++++++++++++++++++++++++++++++
>  src/admin_protocol-structs      |  9 ++++++++
>  src/libvirt-admin.c             | 46 +++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_admin_private.syms  |  1 +
>  src/libvirt_admin_public.syms   |  1 +
>  src/rpc/virnetserver.c          | 34 ++++++++++++++++++++++++++++++
>  src/rpc/virnetserver.h          |  4 ++++
>  12 files changed, 228 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/admin.c b/daemon/admin.c
> index 819b1c0..8cdcaed 100644
> --- a/daemon/admin.c
> +++ b/daemon/admin.c
> @@ -346,4 +346,45 @@ adminDispatchServerGetClientProcessingControls(virNetServerPtr server ATTRIBUTE_
>      virObjectUnref(srv);
>      return rv;
>  }
> +
> +static int
> +adminDispatchServerSetClientProcessingControls(virNetServerPtr server ATTRIBUTE_UNUSED,
> +                                               virNetServerClientPtr client,
> +                                               virNetMessagePtr msg ATTRIBUTE_UNUSED,
> +                                               virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED,
> +                                               admin_server_set_client_processing_controls_args *args)

Yet again, if we decide to change the name in previous patch, this one
should be changed as well.

> +{
> +    int rv = -1;
> +    virNetServerPtr srv = NULL;
> +    virTypedParameterPtr params = NULL;
> +    int nparams = 0;
> +    struct daemonAdmClientPrivate *priv =
> +        virNetServerClientGetPrivateData(client);
> +
> +    if (!(srv = virNetDaemonGetServer(priv->dmn, args->srv.name))) {
> +        virReportError(VIR_ERR_NO_SERVER,
> +                       _("no server with matching name '%s' found"),
> +                       args->srv.name);
> +        goto cleanup;
> +    }
> +
> +    if (virTypedParamsDeserialize((virTypedParameterRemotePtr) args->params.params_val,
> +        args->params.params_len,
> +        ADMIN_SERVER_CLIENT_PROCESSING_CONTROLS_MAX,
> +        &params,
> +        &nparams) < 0)
> +        goto cleanup;
> +
> +    if (adminServerSetClientProcessingControls(srv, params, nparams,
> +                                               args->flags) < 0)
> +        goto cleanup;
> +
> +    rv = 0;
> + cleanup:
> +    if (rv < 0)
> +        virNetMessageSaveError(rerr);
> +    virTypedParamsFree(params, nparams);
> +    virObjectUnref(srv);
> +    return rv;
> +}
>  #include "admin_dispatch.h"
> diff --git a/daemon/admin_server.c b/daemon/admin_server.c
> index 79437a1..65c35a7 100644
> --- a/daemon/admin_server.c
> +++ b/daemon/admin_server.c
> @@ -352,3 +352,38 @@ adminServerGetClientProcessingControls(virNetServerPtr srv,
>      virTypedParamsFree(tmpparams, *nparams);
>      return ret;
>  }
> +
> +int
> +adminServerSetClientProcessingControls(virNetServerPtr srv,
> +                                       virTypedParameterPtr params,
> +                                       int nparams,
> +                                       unsigned int flags)
> +{
> +    long long int maxClients = -1;
> +    long long int maxClientsUnauth = -1;
> +    virTypedParameterPtr param = NULL;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (virTypedParamsValidate(params, nparams,
> +                               VIR_SERVER_CLIENTS_MAX,
> +                               VIR_TYPED_PARAM_UINT,
> +                               VIR_SERVER_CLIENTS_UNAUTH_MAX,
> +                               VIR_TYPED_PARAM_UINT,
> +                               NULL) < 0)
> +        return -1;

Same comment here as in the previous patch. Should we accept ULL value
instead of UINT? Or am I just bikeshedding...

> +
> +    if ((param = virTypedParamsGet(params, nparams,
> +                                   VIR_SERVER_CLIENTS_MAX)))
> +        maxClients = param->value.ui;
> +
> +    if ((param = virTypedParamsGet(params, nparams,
> +                                   VIR_SERVER_CLIENTS_UNAUTH_MAX)))
> +        maxClientsUnauth = param->value.ui;
> +
> +    if (virNetServerSetClientProcessingControls(srv, maxClients,
> +                                                maxClientsUnauth) < 0)
> +        return -1;
> +
> +    return 0;
> +}

> diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
> index 0b317c5..302212f 100644
> --- a/src/libvirt-admin.c
> +++ b/src/libvirt-admin.c
> @@ -1043,3 +1043,49 @@ virAdmServerGetClientProcessingControls(virAdmServerPtr srv,
>      virDispatchError(NULL);
>      return -1;
>  }
> +
> +/**
> + * virAdmServerSetClientProcessingControls:
> + * @srv: a valid server object reference
> + * @params: pointer to client processing controls object
> + * @nparams: number of parameters in @params
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Change client processing controls configuration on server @srv.
> + *
> + * Caller is responsible for allocating @params prior to calling this function.
> + * See 'Manage per-server client processing controls' in libvirt-admin.h for
> + * supported parameters in @params.
> + *
> + * Returns 0 if the controls have been changed successfully or -1 in case of an
> + * error.
> + */
> +int
> +virAdmServerSetClientProcessingControls(virAdmServerPtr srv,
> +                                        virTypedParameterPtr params,
> +                                        int nparams,
> +                                        unsigned int flags)
> +{
> +    int ret = -1;
> +
> +    VIR_DEBUG("srv=%p, params=%p, nparams=%d, flags=%x", srv, params, nparams,
> +              flags);
> +    VIR_TYPED_PARAMS_DEBUG(params, nparams);
> +
> +    virResetLastError();
> +
> +    virCheckAdmServerGoto(srv, error);
> +    virCheckNonNullArgGoto(params, error);
> +    virCheckPositiveArgGoto(nparams, error);
> +    virCheckFlagsGoto(0, error);

Yet again, drop this check.

> +
> +    if ((ret = remoteAdminServerSetClientProcessingControls(srv, params,
> +                                                            nparams,
> +                                                            flags)) < 0)
> +        goto error;
> +
> +    return ret;
> + error:
> +    virDispatchError(NULL);
> +    return ret;
> +}
> diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms
> index 9a30ff5..d0da0d6 100644
> --- a/src/libvirt_admin_private.syms
> +++ b/src/libvirt_admin_private.syms
> @@ -23,6 +23,7 @@ xdr_admin_server_list_clients_args;
>  xdr_admin_server_list_clients_ret;
>  xdr_admin_server_lookup_client_args;
>  xdr_admin_server_lookup_client_ret;
> +xdr_admin_server_set_client_processing_controls_args;
>  xdr_admin_server_set_threadpool_parameters_args;
>  
>  # datatypes.h
> diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms
> index 59b0160..411dd2c 100644
> --- a/src/libvirt_admin_public.syms
> +++ b/src/libvirt_admin_public.syms
> @@ -36,4 +36,5 @@ LIBVIRT_ADMIN_1.3.0 {
>          virAdmClientGetInfo;
>          virAdmClientClose;
>          virAdmServerGetClientProcessingControls;
> +        virAdmServerSetClientProcessingControls;
>  };
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 2cd1765..4c4b144 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -1044,3 +1044,37 @@ virNetServerGetClient(virNetServerPtr srv,
>                         _("No client with matching ID '%llu'"), id);
>      return ret;
>  }
> +
> +int
> +virNetServerSetClientProcessingControls(virNetServerPtr srv,
> +                                        long long int maxClients,
> +                                        long long int maxClientsUnauth)
> +{
> +    int ret = -1;
> +    size_t max, max_unauth;
> +
> +    virObjectLock(srv);
> +
> +    max = maxClients >= 0 ? maxClients : srv->nclients_max;
> +    max_unauth = maxClientsUnauth >= 0 ?
> +        maxClientsUnauth : srv->nclients_unauth_max;
> +
> +    if (max < max_unauth) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("The overall maximum number of clients must be "
> +                         "greater than the maximum number of clients waiting "
> +                         "for authentication"));
> +        goto cleanup;
> +    }

Whoa, we don't have this check when creating new server. I guess we
should do that.

> +
> +    if (maxClients >= 0)
> +        srv->nclients_max = maxClients;
> +
> +    if (maxClientsUnauth >= 0)
> +        srv->nclients_unauth_max = maxClientsUnauth;
> +
> +    ret = 0;
> + cleanup:
> +    virObjectUnlock(srv);
> +    return ret;
> +}
> diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
> index a08cab0..38107b4 100644
> --- a/src/rpc/virnetserver.h
> +++ b/src/rpc/virnetserver.h
> @@ -116,4 +116,8 @@ size_t virNetServerGetCurrentClients(virNetServerPtr srv);
>  size_t virNetServerGetMaxUnauthClients(virNetServerPtr srv);
>  size_t virNetServerGetCurrentUnauthClients(virNetServerPtr srv);
>  
> +int virNetServerSetClientProcessingControls(virNetServerPtr srv,
> +                                            long long int maxClients,
> +                                            long long int maxClientsUnauth);
> +
>  #endif /* __VIR_NET_SERVER_H__ */
> 

Michal


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