[libvirt] [PATCH v2 0/2] Introduce max_anonymous_clients
https://bugzilla.redhat.com/show_bug.cgi?id=981729 So far we can limit how many clients are connected, how many are waiting in accept() line but we could not control the count of accepted but not authenticated yet. Michal Privoznik (2): virNetServer: Introduce unauth clients counter daemon: Introduce max_anonymous_clients daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 +++ daemon/remote.c | 21 ++++++----- daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 88 ++++++++++++++++++++++++++++++++++++++++++--- src/rpc/virnetserver.h | 3 ++ 11 files changed, 110 insertions(+), 16 deletions(-) -- 1.9.0
The counter gets incremented on each unauthenticated client added to the server and decremented whenever the client authenticates. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 21 +++++++++++++-------- src/rpc/virnetserver.c | 36 +++++++++++++++++++++++++++++++++--- src/rpc/virnetserver.h | 2 ++ 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 932f65f..b70d298 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2619,7 +2619,7 @@ cleanup: /*-------------------------------------------------------------*/ static int -remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthList(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2649,6 +2649,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; VIR_INFO("Bypass polkit auth for privileged client %s", ident); virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; VIR_FREE(ident); } @@ -2764,7 +2765,8 @@ authfail: * Returns 0 if ok, -1 on error, -2 if rejected */ static int -remoteSASLFinish(virNetServerClientPtr client) +remoteSASLFinish(virNetServerPtr server, + virNetServerClientPtr client) { const char *identity; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); @@ -2789,6 +2791,7 @@ remoteSASLFinish(virNetServerClientPtr client) return -2; virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virNetServerClientSetSASLSession(client, priv->sasl); VIR_DEBUG("Authentication successful %d", virNetServerClientGetFD(client)); @@ -2810,7 +2813,7 @@ error: * This starts the SASL authentication negotiation. */ static int -remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthSaslStart(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2868,7 +2871,7 @@ remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 0; } else { /* Check username whitelist ACL */ - if ((err = remoteSASLFinish(client)) < 0) { + if ((err = remoteSASLFinish(server, client)) < 0) { if (err == -2) goto authdeny; else @@ -2908,7 +2911,7 @@ error: static int -remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthSaslStep(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2966,7 +2969,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 0; } else { /* Check username whitelist ACL */ - if ((err = remoteSASLFinish(client)) < 0) { + if ((err = remoteSASLFinish(server, client)) < 0) { if (err == -2) goto authdeny; else @@ -3051,7 +3054,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, #if WITH_POLKIT1 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -3143,6 +3146,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 1; virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virMutexUnlock(&priv->lock); virCommandFree(cmd); VIR_FREE(pkout); @@ -3183,7 +3187,7 @@ authdeny: } #elif WITH_POLKIT0 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -3298,6 +3302,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 1; virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virMutexUnlock(&priv->lock); VIR_FREE(ident); return 0; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index f70e260..6b3f5f0 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -88,9 +88,10 @@ struct _virNetServer { size_t nprograms; virNetServerProgramPtr *programs; - size_t nclients; - size_t nclients_max; - virNetServerClientPtr *clients; + size_t nclients; /* Current clients count */ + virNetServerClientPtr *clients; /* Clients */ + size_t nclients_max; /* Max allowed clients count */ + size_t nclients_unauth; /* Unauthenticated clients count */ int keepaliveInterval; unsigned int keepaliveCount; @@ -118,6 +119,8 @@ static virClassPtr virNetServerClass; static void virNetServerDispose(void *obj); static void virNetServerUpdateServicesLocked(virNetServerPtr srv, bool enabled); +static size_t virNetServerClientAuthLocked(virNetServerPtr srv, + bool need_auth); static int virNetServerOnceInit(void) { @@ -273,6 +276,9 @@ static int virNetServerAddClient(virNetServerPtr srv, srv->clients[srv->nclients-1] = client; virObjectRef(client); + if (virNetServerClientNeedAuth(client)) + virNetServerClientAuthLocked(srv, true); + if (srv->nclients == srv->nclients_max) { /* Temporarily stop accepting new clients */ VIR_DEBUG("Temporarily suspending services due to max_clients"); @@ -1140,6 +1146,9 @@ void virNetServerRun(virNetServerPtr srv) srv->nclients = 0; } + if (virNetServerClientNeedAuth(client)) + 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) { @@ -1236,3 +1245,24 @@ bool virNetServerKeepAliveRequired(virNetServerPtr srv) virObjectUnlock(srv); return required; } + +static size_t +virNetServerClientAuthLocked(virNetServerPtr srv, + bool need_auth) +{ + if (need_auth) + srv->nclients_unauth++; + else + srv->nclients_unauth--; + return srv->nclients_unauth; +} + +size_t virNetServerClientAuth(virNetServerPtr srv, + bool need_auth) +{ + size_t ret; + virObjectLock(srv); + ret = virNetServerClientAuthLocked(srv, need_auth); + virObjectUnlock(srv); + return ret; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 1a85c02..703a733 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -97,4 +97,6 @@ void virNetServerClose(virNetServerPtr srv); bool virNetServerKeepAliveRequired(virNetServerPtr srv); +size_t virNetServerClientAuth(virNetServerPtr srv, + bool need_auth); #endif -- 1.9.0
On Mon, Mar 03, 2014 at 06:22:43PM +0100, Michal Privoznik wrote:
The counter gets incremented on each unauthenticated client added to the server and decremented whenever the client authenticates.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 21 +++++++++++++-------- src/rpc/virnetserver.c | 36 +++++++++++++++++++++++++++++++++--- src/rpc/virnetserver.h | 2 ++ 3 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 932f65f..b70d298 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2619,7 +2619,7 @@ cleanup: /*-------------------------------------------------------------*/
static int -remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthList(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2649,6 +2649,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; VIR_INFO("Bypass polkit auth for privileged client %s", ident); virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; VIR_FREE(ident); } @@ -2764,7 +2765,8 @@ authfail: * Returns 0 if ok, -1 on error, -2 if rejected */ static int -remoteSASLFinish(virNetServerClientPtr client) +remoteSASLFinish(virNetServerPtr server, + virNetServerClientPtr client) { const char *identity; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); @@ -2789,6 +2791,7 @@ remoteSASLFinish(virNetServerClientPtr client) return -2;
virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virNetServerClientSetSASLSession(client, priv->sasl);
VIR_DEBUG("Authentication successful %d", virNetServerClientGetFD(client)); @@ -2810,7 +2813,7 @@ error: * This starts the SASL authentication negotiation. */ static int -remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthSaslStart(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2868,7 +2871,7 @@ remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 0; } else { /* Check username whitelist ACL */ - if ((err = remoteSASLFinish(client)) < 0) { + if ((err = remoteSASLFinish(server, client)) < 0) { if (err == -2) goto authdeny; else @@ -2908,7 +2911,7 @@ error:
static int -remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthSaslStep(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2966,7 +2969,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 0; } else { /* Check username whitelist ACL */ - if ((err = remoteSASLFinish(client)) < 0) { + if ((err = remoteSASLFinish(server, client)) < 0) { if (err == -2) goto authdeny; else @@ -3051,7 +3054,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED,
#if WITH_POLKIT1 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -3143,6 +3146,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 1;
virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virMutexUnlock(&priv->lock); virCommandFree(cmd); VIR_FREE(pkout); @@ -3183,7 +3187,7 @@ authdeny: } #elif WITH_POLKIT0 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -3298,6 +3302,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 1;
virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virMutexUnlock(&priv->lock); VIR_FREE(ident); return 0; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index f70e260..6b3f5f0 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -88,9 +88,10 @@ struct _virNetServer { size_t nprograms; virNetServerProgramPtr *programs;
- size_t nclients; - size_t nclients_max; - virNetServerClientPtr *clients; + size_t nclients; /* Current clients count */ + virNetServerClientPtr *clients; /* Clients */ + size_t nclients_max; /* Max allowed clients count */ + size_t nclients_unauth; /* Unauthenticated clients count */
int keepaliveInterval; unsigned int keepaliveCount; @@ -118,6 +119,8 @@ static virClassPtr virNetServerClass; static void virNetServerDispose(void *obj); static void virNetServerUpdateServicesLocked(virNetServerPtr srv, bool enabled); +static size_t virNetServerClientAuthLocked(virNetServerPtr srv, + bool need_auth);
static int virNetServerOnceInit(void) { @@ -273,6 +276,9 @@ static int virNetServerAddClient(virNetServerPtr srv, srv->clients[srv->nclients-1] = client; virObjectRef(client);
+ if (virNetServerClientNeedAuth(client)) + virNetServerClientAuthLocked(srv, true); + if (srv->nclients == srv->nclients_max) { /* Temporarily stop accepting new clients */ VIR_DEBUG("Temporarily suspending services due to max_clients"); @@ -1140,6 +1146,9 @@ void virNetServerRun(virNetServerPtr srv) srv->nclients = 0; }
+ if (virNetServerClientNeedAuth(client)) + 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) { @@ -1236,3 +1245,24 @@ bool virNetServerKeepAliveRequired(virNetServerPtr srv) virObjectUnlock(srv); return required; } + +static size_t +virNetServerClientAuthLocked(virNetServerPtr srv, + bool need_auth) +{ + if (need_auth) + srv->nclients_unauth++; + else + srv->nclients_unauth--; + return srv->nclients_unauth; +} + +size_t virNetServerClientAuth(virNetServerPtr srv, + bool need_auth) +{ + size_t ret; + virObjectLock(srv); + ret = virNetServerClientAuthLocked(srv, need_auth); + virObjectUnlock(srv); + return ret; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 1a85c02..703a733 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -97,4 +97,6 @@ void virNetServerClose(virNetServerPtr srv);
bool virNetServerKeepAliveRequired(virNetServerPtr srv);
+size_t virNetServerClientAuth(virNetServerPtr srv, + bool need_auth);
I'm finding this API naming rather confusing in reviewing the patch. The name suggests it is operating on the virNetServerClientPtr instance. I think it might also be clearer to have separate methods for add/dec. eg perhaps virNetServerTrackPendingAuth(srv); virNetServerTrackCompletedAuth(srv); Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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@redhat.com> --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 ++++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 62 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 b27c6fd..6e4c06f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1376,6 +1376,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 073c178..880f46a 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,10 @@ # connection succeeds. #max_queued_clients = 1000 +# The maximum length of queue of accepted but not yet not +# authenticated clients. The default value is zero, meaning +# the feature is disabled. +#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 e047751..054ece2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -145,8 +145,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) } if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - -1, 0, - false, NULL, + 0, -1, 0, false, NULL, virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 5ca960f..dcb8264 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 6b3f5f0..c4e8dbc 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -92,6 +92,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; @@ -279,6 +280,14 @@ static int virNetServerAddClient(virNetServerPtr srv, if (virNetServerClientNeedAuth(client)) virNetServerClientAuthLocked(srv, true); + 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"); + virNetServerUpdateServicesLocked(srv, false); + } + if (srv->nclients == srv->nclients_max) { /* Temporarily stop accepting new clients */ VIR_DEBUG("Temporarily suspending services due to max_clients"); @@ -362,6 +371,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, @@ -388,6 +398,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; @@ -457,6 +468,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int max_workers; unsigned int priority_workers; unsigned int max_clients; + unsigned int nclients_unauth_max; unsigned int keepaliveInterval; unsigned int keepaliveCount; bool keepaliveRequired; @@ -482,6 +494,11 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, _("Missing max_clients data in JSON document")); goto error; } + if (virJSONValueObjectGetNumberUint(object, "nclients_unauth_max", &nclients_unauth_max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing nclients_unauth_max data in JSON document")); + goto error; + } if (virJSONValueObjectGetNumberUint(object, "keepaliveInterval", &keepaliveInterval) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing keepaliveInterval data in JSON document")); @@ -507,6 +524,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, if (!(srv = virNetServerNew(min_workers, max_clients, priority_workers, max_clients, + nclients_unauth_max, keepaliveInterval, keepaliveCount, keepaliveRequired, mdnsGroupName, clientPrivNew, clientPrivPreExecRestart, @@ -625,6 +643,12 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) _("Cannot set max_clients data in JSON document")); goto error; } + if (virJSONValueObjectAppendNumberUint(object, "nclients_unauth_max", + srv->nclients_unauth_max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set nclients_unauth_max data in JSON document")); + goto error; + } if (virJSONValueObjectAppendNumberUint(object, "keepaliveInterval", srv->keepaliveInterval) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot set keepaliveInterval data in JSON document")); @@ -1150,8 +1174,17 @@ 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 && + (!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); @@ -1262,7 +1295,22 @@ size_t virNetServerClientAuth(virNetServerPtr srv, { size_t ret; virObjectLock(srv); + ret = virNetServerClientAuthLocked(srv, need_auth); + + 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 && + (!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); + } + virObjectUnlock(srv); return ret; } diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 703a733..4b165c3 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -39,6 +39,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, -- 1.9.0
On Mon, Mar 03, 2014 at 06:22:44PM +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@redhat.com> --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 ++++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 62 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);
You need a 'data->max_anonymous_clients = 20' initialization somewher in this file, otherwise it'll default to 0. Also, don't we want to increase 'max_clients' to something much larger like 5000 now that we rely on max_anonymous_clients to protect against DOS attack.
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 073c178..880f46a 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,10 @@ # connection succeeds. #max_queued_clients = 1000
+# The maximum length of queue of accepted but not yet not +# authenticated clients. The default value is zero, meaning +# the feature is disabled. +#max_anonymous_clients = 20
same not about increasing max_clients value here.
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index e047751..054ece2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -145,8 +145,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) }
if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - -1, 0, - false, NULL, + 0, -1, 0, false, NULL, virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree,
We need to support max_anonymous_clients in the lock daemon config file too and increase its max clients to something huge too. Each VM started corresponds to an open client with the lock daemon, so we need that value to be quite high.
@@ -457,6 +468,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int max_workers; unsigned int priority_workers; unsigned int max_clients; + unsigned int nclients_unauth_max; unsigned int keepaliveInterval; unsigned int keepaliveCount; bool keepaliveRequired; @@ -482,6 +494,11 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, _("Missing max_clients data in JSON document")); goto error; } + if (virJSONValueObjectGetNumberUint(object, "nclients_unauth_max", &nclients_unauth_max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing nclients_unauth_max data in JSON document"));
We can't raise an error here - that'll cause failure upon RPM upgrade from version which didn't have this setting. Need to set some kind of sensible default value upon upgrade. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 04.03.2014 12:56, Daniel P. Berrange wrote:
On Mon, Mar 03, 2014 at 06:22:44PM +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@redhat.com> --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 ++++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 62 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);
You need a 'data->max_anonymous_clients = 20' initialization somewher in this file, otherwise it'll default to 0.
And I think we want to leave it that way. The value of zero means the feature is disabled. That is, libvirt won't take any actions if count of anonymous clients exceed certain value. It will still take an action though if the number of total clients (both auth and unauth) exceeds max_clients. The action is stop accept()-ing new clients. Trying to invent a bright formula to compute the correct value may lead to us adapting to a certain use case while forgetting about other. And we've been there before (remember 'Set reasonable RSS limit on domain startup'?). If a specific management application using libvirt requires these values it should set it explicitly in the config file rather than relying on libvirt defaults (which would change as libvirt adapts to different management application). What we can do is change the commented default value in config file. Is that what you had in mind in the first place?
Also, don't we want to increase 'max_clients' to something much larger like 5000 now that we rely on max_anonymous_clients to protect against DOS attack.
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 073c178..880f46a 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,10 @@ # connection succeeds. #max_queued_clients = 1000
+# The maximum length of queue of accepted but not yet not +# authenticated clients. The default value is zero, meaning +# the feature is disabled. +#max_anonymous_clients = 20
same not about increasing max_clients value here.
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index e047751..054ece2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -145,8 +145,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) }
if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - -1, 0, - false, NULL, + 0, -1, 0, false, NULL, virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree,
We need to support max_anonymous_clients in the lock daemon config file too and increase its max clients to something huge too. Each VM started corresponds to an open client with the lock daemon, so we need that value to be quite high.
@@ -457,6 +468,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int max_workers; unsigned int priority_workers; unsigned int max_clients; + unsigned int nclients_unauth_max; unsigned int keepaliveInterval; unsigned int keepaliveCount; bool keepaliveRequired; @@ -482,6 +494,11 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, _("Missing max_clients data in JSON document")); goto error; } + if (virJSONValueObjectGetNumberUint(object, "nclients_unauth_max", &nclients_unauth_max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing nclients_unauth_max data in JSON document"));
We can't raise an error here - that'll cause failure upon RPM upgrade from version which didn't have this setting. Need to set some kind of sensible default value upon upgrade.
The sensible default is zero in my opinion. If management application wants to override this, they still can set the value in the config and then softly reset libvirtd (by softly I mean sending it signal to reload the config). Michal
On Tue, Mar 04, 2014 at 04:46:22PM +0100, Michal Privoznik wrote:
On 04.03.2014 12:56, Daniel P. Berrange wrote:
On Mon, Mar 03, 2014 at 06:22:44PM +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@redhat.com> --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 ++++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 62 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);
You need a 'data->max_anonymous_clients = 20' initialization somewher in this file, otherwise it'll default to 0.
And I think we want to leave it that way. The value of zero means the feature is disabled. That is, libvirt won't take any actions if count of anonymous clients exceed certain value. It will still take an action though if the number of total clients (both auth and unauth) exceeds max_clients. The action is stop accept()-ing new clients. Trying to invent a bright formula to compute the correct value may lead to us adapting to a certain use case while forgetting about other. And we've been there before (remember 'Set reasonable RSS limit on domain startup'?). If a specific management application using libvirt requires these values it should set it explicitly in the config file rather than relying on libvirt defaults (which would change as libvirt adapts to different management application).
What we can do is change the commented default value in config file. Is that what you had in mind in the first place?
The intent behind this was to get better out of the box behaviour for client limits. Currently with max_clients==20, we are very limited out of the box. People frequently hit the max clients limit, eg due to having many 'virsh console' sessions open at once. We can't just raise max_clients because that opens up a denial of service from unauthenticated clients. The idea behind tracking unauthenticated clients separately was that we can have a low value for max_anonymous_clients to prevent the denial of service possibility. We can then safely have a very high value for max_clients to cope with the users who want to open many connections.
The sensible default is zero in my opinion. If management application wants to override this, they still can set the value in the config and then softly reset libvirtd (by softly I mean sending it signal to reload the config).
A default value of zero means this feature has achieved nothing to improve our out of the box behaviour, which is what's really hurting us from users POV. Thus IMHO our default should be max_anonymous_clients=20 and max_clients=5000 Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 04.03.2014 16:46, Michal Privoznik wrote:
On 04.03.2014 12:56, Daniel P. Berrange wrote:
On Mon, Mar 03, 2014 at 06:22:44PM +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@redhat.com> --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 ++++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 62 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);
You need a 'data->max_anonymous_clients = 20' initialization somewher in this file, otherwise it'll default to 0.
And I think we want to leave it that way. The value of zero means the feature is disabled. That is, libvirt won't take any actions if count of anonymous clients exceed certain value. It will still take an action though if the number of total clients (both auth and unauth) exceeds max_clients. The action is stop accept()-ing new clients. Trying to invent a bright formula to compute the correct value may lead to us adapting to a certain use case while forgetting about other. And we've been there before (remember 'Set reasonable RSS limit on domain startup'?). If a specific management application using libvirt requires these values it should set it explicitly in the config file rather than relying on libvirt defaults (which would change as libvirt adapts to different management application).
What we can do is change the commented default value in config file. Is that what you had in mind in the first place?
Also, don't we want to increase 'max_clients' to something much larger like 5000 now that we rely on max_anonymous_clients to protect against DOS attack.
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 073c178..880f46a 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,10 @@ # connection succeeds. #max_queued_clients = 1000
+# The maximum length of queue of accepted but not yet not +# authenticated clients. The default value is zero, meaning +# the feature is disabled. +#max_anonymous_clients = 20
same not about increasing max_clients value here.
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index e047751..054ece2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -145,8 +145,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) }
if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - -1, 0, - false, NULL, + 0, -1, 0, false, NULL, virLockDaemonClientNew,
virLockDaemonClientPreExecRestart, virLockDaemonClientFree,
We need to support max_anonymous_clients in the lock daemon config file too and increase its max clients to something huge too. Each VM started corresponds to an open client with the lock daemon, so we need that value to be quite high.
I forgot to reply to this block. But my argumentation stays the same here. Moreover, there's no auth or unauth users in virlockd. I mean, all users are unauth by default and virlockd has no authentication mechanisms to make them authenticate. So either we will pass 0 here and let the rest of code deal with it as if the feature is disabled or pass config->max_clients which results in the same behavior in the end.
@@ -457,6 +468,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int max_workers; unsigned int priority_workers; unsigned int max_clients; + unsigned int nclients_unauth_max; unsigned int keepaliveInterval; unsigned int keepaliveCount; bool keepaliveRequired; @@ -482,6 +494,11 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, _("Missing max_clients data in JSON document")); goto error; } + if (virJSONValueObjectGetNumberUint(object, "nclients_unauth_max", &nclients_unauth_max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing nclients_unauth_max data in JSON document"));
We can't raise an error here - that'll cause failure upon RPM upgrade from version which didn't have this setting. Need to set some kind of sensible default value upon upgrade.
The sensible default is zero in my opinion. If management application wants to override this, they still can set the value in the config and then softly reset libvirtd (by softly I mean sending it signal to reload the config).
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Mar 04, 2014 at 04:56:24PM +0100, Michal Privoznik wrote:
On 04.03.2014 16:46, Michal Privoznik wrote:
On 04.03.2014 12:56, Daniel P. Berrange wrote:
On Mon, Mar 03, 2014 at 06:22:44PM +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@redhat.com> --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 4 ++++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 3 +-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 62 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);
You need a 'data->max_anonymous_clients = 20' initialization somewher in this file, otherwise it'll default to 0.
And I think we want to leave it that way. The value of zero means the feature is disabled. That is, libvirt won't take any actions if count of anonymous clients exceed certain value. It will still take an action though if the number of total clients (both auth and unauth) exceeds max_clients. The action is stop accept()-ing new clients. Trying to invent a bright formula to compute the correct value may lead to us adapting to a certain use case while forgetting about other. And we've been there before (remember 'Set reasonable RSS limit on domain startup'?). If a specific management application using libvirt requires these values it should set it explicitly in the config file rather than relying on libvirt defaults (which would change as libvirt adapts to different management application).
What we can do is change the commented default value in config file. Is that what you had in mind in the first place?
Also, don't we want to increase 'max_clients' to something much larger like 5000 now that we rely on max_anonymous_clients to protect against DOS attack.
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 073c178..880f46a 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,10 @@ # connection succeeds. #max_queued_clients = 1000
+# The maximum length of queue of accepted but not yet not +# authenticated clients. The default value is zero, meaning +# the feature is disabled. +#max_anonymous_clients = 20
same not about increasing max_clients value here.
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index e047751..054ece2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -145,8 +145,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) }
if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - -1, 0, - false, NULL, + 0, -1, 0, false, NULL, virLockDaemonClientNew,
virLockDaemonClientPreExecRestart, virLockDaemonClientFree,
We need to support max_anonymous_clients in the lock daemon config file too and increase its max clients to something huge too. Each VM started corresponds to an open client with the lock daemon, so we need that value to be quite high.
I forgot to reply to this block. But my argumentation stays the same here. Moreover, there's no auth or unauth users in virlockd. I mean, all users are unauth by default and virlockd has no authentication mechanisms to make them authenticate. So either we will pass 0 here and let the rest of code deal with it as if the feature is disabled or pass config->max_clients which results in the same behavior in the end.
I forgot we didn't do auth here. We immediately check the UID of the client and drop them if not root, so we'd actually be perfectly safe to increase max_clients for the lock daemon already. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Michal Privoznik