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

[Daniel P. Berrange]

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 at 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 :|