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