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@xxxxxxxxxx> > --- > 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list