[libvirt] virtlockd max_clients limitation

[Daniel P. Berrange]

On Mon, Aug 19, 2013 at 10:14:55AM +0200, David Weber wrote:
> >From 0e6ddd5825792faad5bb4c9ea1d5bd59dd09f821 Mon Sep 17 00:00:00 2001
> From: David Weber <wb at munzinger.de>
> Date: Mon, 19 Aug 2013 09:48:18 +0200
> Subject: [PATCH] Make max_clients in virtlockd configurable
> 
> ---
>  src/locking/lock_daemon.c        | 6 +++---
>  src/locking/lock_daemon_config.c | 2 ++
>  src/locking/lock_daemon_config.h | 1 +
>  src/locking/virtlockd.aug        | 1 +
>  src/locking/virtlockd.conf       | 4 ++++
>  5 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index 77d6e0d..5f675ef 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -128,7 +128,7 @@ static void virLockDaemonLockSpaceDataFree(void *data,
>  }
>  
>  static virLockDaemonPtr
> -virLockDaemonNew(bool privileged)
> +virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged)
>  {
>      virLockDaemonPtr lockd;
>  
> @@ -142,7 +142,7 @@ virLockDaemonNew(bool privileged)
>          return NULL;
>      }
>  
> -    if (!(lockd->srv = virNetServerNew(1, 1, 0, 20,
> +    if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients,
>                                         -1, 0,
>                                         false, NULL,
>                                         virLockDaemonClientNew,
> @@ -1335,7 +1335,7 @@ int main(int argc, char **argv) {
>      /* rv == 1, means we setup everything from saved state,
>       * so we only setup stuff from scratch if rv == 0 */
>      if (rv == 0) {
> -        if (!(lockDaemon = virLockDaemonNew(privileged))) {
> +        if (!(lockDaemon = virLockDaemonNew(config, privileged))) {
>              ret = VIR_LOCK_DAEMON_ERR_INIT;
>              goto cleanup;
>          }
> diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
> index 88c4150..48725be 100644
> --- a/src/locking/lock_daemon_config.c
> +++ b/src/locking/lock_daemon_config.c
> @@ -114,6 +114,7 @@ virLockDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
>          return NULL;
>  
>      data->log_buffer_size = 64;
> +    data->max_clients = 20;

I think we should probably default to a much larger value - say 1024.
Unlike libvirtd, virtlockd is only accessible by privileged clients
so we don't need to be so strict on default limits. We want a value
high enough to not cause bogus failures with people starting lots of
VMs. So 1024 seems like a good high starting point

>  
>      return data;
>  }
> @@ -139,6 +140,7 @@ virLockDaemonConfigLoadOptions(virLockDaemonConfigPtr data,
>      GET_CONF_STR(conf, filename, log_filters);
>      GET_CONF_STR(conf, filename, log_outputs);
>      GET_CONF_INT(conf, filename, log_buffer_size);
> +    GET_CONF_INT(conf, filename, max_clients);
>  
>      return 0;
>  
> diff --git a/src/locking/lock_daemon_config.h b/src/locking/lock_daemon_config.h
> index 8cb0e5d..e75d4a9 100644
> --- a/src/locking/lock_daemon_config.h
> +++ b/src/locking/lock_daemon_config.h
> @@ -34,6 +34,7 @@ struct _virLockDaemonConfig {
>      char *log_filters;
>      char *log_outputs;
>      int log_buffer_size;
> +    int max_clients;
>  };
>  
>  
> diff --git a/src/locking/virtlockd.aug b/src/locking/virtlockd.aug
> index 9d20e72..d0b56c2 100644
> --- a/src/locking/virtlockd.aug
> +++ b/src/locking/virtlockd.aug
> @@ -28,6 +28,7 @@ module Libvirtd =
>                       | str_entry "log_filters"
>                       | str_entry "log_outputs"
>                       | int_entry "log_buffer_size"
> +                     | int_entry "max_clients"
>  
>     (* Each enty in the config is one of the following three ... *)
>     let entry = logging_entry
> diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf
> index b6450b4..37f3ef3 100644
> --- a/src/locking/virtlockd.conf
> +++ b/src/locking/virtlockd.conf
> @@ -58,3 +58,7 @@
>  # the default buffer size in kilobytes.
>  # If value is 0 or less the debug log buffer is deactivated
>  #log_buffer_size = 64
> +
> +# The maximum number of concurrent client connections to allow
> +# over all sockets combined.
> +#max_clients = 20

The comment should probably also say 

"Each running virtual machine will require one open connection
 to virtlockd. So 'max_clients' will affect how many VMs can
 be run on a host"

To make it extra clear what this limit is impacting.

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