[libvirt] [PATCH 2/2] daemon: Introduce max_anonymous_clients
Martin Kletzander
mkletzan at redhat.com
Thu Jan 23 10:18:31 UTC 2014
On Mon, Dec 09, 2013 at 03:35:53PM +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=981729
>
> This config tunable allows users to determine the maximum number of
> accepted but yet not authenticated users.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> daemon/libvirtd-config.c | 1 +
> daemon/libvirtd-config.h | 1 +
> daemon/libvirtd.aug | 1 +
> daemon/libvirtd.c | 1 +
> daemon/libvirtd.conf | 3 +++
> daemon/test_libvirtd.aug.in | 1 +
> src/locking/lock_daemon.c | 4 ++--
> src/lxc/lxc_controller.c | 2 +-
> src/rpc/virnetserver.c | 37 +++++++++++++++++++++++++++++++++++--
> src/rpc/virnetserver.h | 1 +
> 10 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c
> index c816fda..04482c5 100644
> --- a/daemon/libvirtd-config.c
> +++ b/daemon/libvirtd-config.c
> @@ -415,6 +415,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
> GET_CONF_INT(conf, filename, max_workers);
> GET_CONF_INT(conf, filename, max_clients);
> GET_CONF_INT(conf, filename, max_queued_clients);
> + GET_CONF_INT(conf, filename, max_anonymous_clients);
>
> GET_CONF_INT(conf, filename, prio_workers);
>
> diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h
> index a24d5d2..66dc80b 100644
> --- a/daemon/libvirtd-config.h
> +++ b/daemon/libvirtd-config.h
> @@ -64,6 +64,7 @@ struct daemonConfig {
> int max_workers;
> int max_clients;
> int max_queued_clients;
> + int max_anonymous_clients;
>
> int prio_workers;
>
> diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
> index 70fce5c..5a0807c 100644
> --- a/daemon/libvirtd.aug
> +++ b/daemon/libvirtd.aug
> @@ -57,6 +57,7 @@ module Libvirtd =
> | int_entry "max_workers"
> | int_entry "max_clients"
> | int_entry "max_queued_clients"
> + | int_entry "max_anonymous_clients"
> | int_entry "max_requests"
> | int_entry "max_client_requests"
> | int_entry "prio_workers"
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 49c42ad..61c706c 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1367,6 +1367,7 @@ int main(int argc, char **argv) {
> config->max_workers,
> config->prio_workers,
> config->max_clients,
> + config->max_anonymous_clients,
> config->keepalive_interval,
> config->keepalive_count,
> !!config->keepalive_required,
> diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
> index 5353927..28d3735 100644
> --- a/daemon/libvirtd.conf
> +++ b/daemon/libvirtd.conf
> @@ -263,6 +263,9 @@
> # connection succeeds.
> #max_queued_clients = 1000
>
> +# The maximum length of queue of accepted but not yet not
> +# authenticated clients.
> +#max_anonymous_clients = 20
>
Please mention what's the default here.
> # The minimum limit sets the number of workers to start up
> # initially. If the number of active clients exceeds this,
> diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in
> index a7e8515..b03451c 100644
> --- a/daemon/test_libvirtd.aug.in
> +++ b/daemon/test_libvirtd.aug.in
> @@ -36,6 +36,7 @@ module Test_libvirtd =
> }
> { "max_clients" = "20" }
> { "max_queued_clients" = "1000" }
> + { "max_anonymous_clients" = "20" }
> { "min_workers" = "5" }
> { "max_workers" = "20" }
> { "prio_workers" = "5" }
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index 35ccb4e..c97bc61 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -143,8 +143,8 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged)
> }
>
> if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients,
> - -1, 0,
> - false, NULL,
> + config->max_clients,
> + -1, 0, false, NULL,
Since the default (when there's nothing in the config) is 0, it should
mean the feature is disabled). If that's what you've intended,
wouldn't it make more sense to use 0 so it's visible that the feature
is not in effect?
> virLockDaemonClientNew,
> virLockDaemonClientPreExecRestart,
> virLockDaemonClientFree,
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index c013147..578a135 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -736,7 +736,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl)
> LXC_STATE_DIR, ctrl->name) < 0)
> return -1;
>
> - if (!(ctrl->server = virNetServerNew(0, 0, 0, 1,
> + if (!(ctrl->server = virNetServerNew(0, 0, 0, 1, 1,
Same here?
> -1, 0, false,
> NULL,
> virLXCControllerClientPrivateNew,
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 1b2c6d4..bbb91d4 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -91,6 +91,7 @@ struct _virNetServer {
> virNetServerClientPtr *clients; /* Clients */
> size_t nclients_max; /* Max allowed clients count */
> size_t nclients_unauth; /* Unauthenticated clients count */
> + size_t nclients_unauth_max; /* Max allowed unauth clients count */
>
> int keepaliveInterval;
> unsigned int keepaliveCount;
> @@ -278,6 +279,13 @@ static int virNetServerAddClient(virNetServerPtr srv,
> if (virNetServerClientNeedAuth(client))
> virNetServerClientAuthLocked(srv, true);
>
> + if (srv->nclients_unauth == srv->nclients_unauth_max) {
> + /* Temporarily stop accepting new clients */
> + VIR_DEBUG("Temporarily suspending services "
> + "due to max_anonymous_clients");
> + virNetServerUpdateServicesLocked(srv, false);
> + }
> +
> if (srv->nclients == srv->nclients_max) {
> /* Temporarily stop accepting new clients */
> VIR_DEBUG("Temporarily suspending services due to max_clients");
> @@ -361,6 +369,7 @@ virNetServerPtr virNetServerNew(size_t min_workers,
> size_t max_workers,
> size_t priority_workers,
> size_t max_clients,
> + size_t max_anonymous_clients,
> int keepaliveInterval,
> unsigned int keepaliveCount,
> bool keepaliveRequired,
> @@ -387,6 +396,7 @@ virNetServerPtr virNetServerNew(size_t min_workers,
> goto error;
>
> srv->nclients_max = max_clients;
> + srv->nclients_unauth_max = max_anonymous_clients;
> srv->keepaliveInterval = keepaliveInterval;
> srv->keepaliveCount = keepaliveCount;
> srv->keepaliveRequired = keepaliveRequired;
> @@ -506,6 +516,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
>
> if (!(srv = virNetServerNew(min_workers, max_clients,
> priority_workers, max_clients,
> + max_clients,
Here it should be taken from the JSON object (and 0 if not found)
which should be populated from the save file. Also update that save
file populating so the value is kept throughout all restarts.
> keepaliveInterval, keepaliveCount,
> keepaliveRequired, mdnsGroupName,
> clientPrivNew, clientPrivPreExecRestart,
> @@ -1145,8 +1156,16 @@ void virNetServerRun(virNetServerPtr srv)
> virNetServerClientAuthLocked(srv, false);
>
> /* Enable services if we can accept a new client.
> - * The new client can be accepted if we are at the limit. */
> - if (srv->nclients == srv->nclients_max - 1) {
> + * The new client can be accepted if both max_clients and
> + * max_anonymous_clients wouldn't get overcommitted by
> + * accepting it. */
> + VIR_DEBUG("Considering re-enabling services: "
> + "nclients=%zu nclients_max=%zu "
> + "nclients_unauth=%zu nclients_unauth_max=%zu",
> + srv->nclients, srv->nclients_max,
> + srv->nclients_unauth, srv->nclients_unauth_max);
> + if ((srv->nclients == srv->nclients_max - 1) &&
> + (srv->nclients_unauth < srv->nclients_unauth_max)) {
Spurious parentheses, plus even considering your squash-in, this and
similar conditions don't count on nclients_unauth_max being 0;
consider squashing this in:
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 62490a7..332641e 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -279,7 +279,8 @@ static int virNetServerAddClient(virNetServerPtr srv,
if (virNetServerClientNeedAuth(client))
virNetServerClientAuthLocked(srv, true);
- if (srv->nclients_unauth == srv->nclients_unauth_max) {
+ if (srv->nclients_unauth_max &&
+ srv->nclients_unauth == srv->nclients_unauth_max) {
/* Temporarily stop accepting new clients */
VIR_DEBUG("Temporarily suspending services "
"due to max_anonymous_clients");
@@ -1164,8 +1165,9 @@ void virNetServerRun(virNetServerPtr srv)
"nclients_unauth=%zu
nclients_unauth_max=%zu",
srv->nclients, srv->nclients_max,
srv->nclients_unauth,
srv->nclients_unauth_max);
- if ((srv->nclients < srv->nclients_max) &&
- (srv->nclients_unauth < srv->nclients_unauth_max)) {
+ if (srv->nclients < srv->nclients_max &&
+ (!srv->nclients_unauth_max ||
+ srv->nclients_unauth < srv->nclients_unauth_max)) {
/* Now it makes sense to accept() a new
client. */
VIR_DEBUG("Re-enabling services");
virNetServerUpdateServicesLocked(srv, true);
@@ -1284,8 +1286,9 @@ size_t virNetServerClientAuth(virNetServerPtr srv,
"nclients_unauth=%zu nclients_unauth_max=%zu",
srv->nclients, srv->nclients_max,
srv->nclients_unauth, srv->nclients_unauth_max);
- if ((srv->nclients < srv->nclients_max) &&
- (srv->nclients_unauth < srv->nclients_unauth_max)) {
+ if (srv->nclients < srv->nclients_max &&
+ (!srv->nclients_unauth_max ||
+ srv->nclients_unauth < srv->nclients_unauth_max)) {
/* Now it makes sense to accept() a new client. */
VIR_DEBUG("Re-enabling services");
virNetServerUpdateServicesLocked(srv, true);
--
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140123/52fbf577/attachment-0001.sig>
More information about the libvir-list
mailing list