Re: [libvirt] [PATCH 2/2] daemon: Introduce max_anonymous_clients

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(a)redhat.com&gt; --- 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

# 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,

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,

-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,

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)) {