[libvirt] virtlockd max_clients limitation
Hi, we recently ran into a problem when trying to start more than 20 guests if direct locking with virtlockd was enabled. The error message locked like this: # start test7 error: Failed to start domain test7 error: Cannot recv data: Connection reset by peer Our research indicated there is a max_clients parameter set in src/locking/lock_daemon.c which causes the limitation. Simply increasing this number works as a workaround. I don't have a deep understanding of the code, so I don't know if there is a bug and the connection should be dropped after the lock is set or if max_clients really is the way to go. If the latter I guess max_clients should be configurable like in virtlockd or at least be higher than 20 :) Libvirt version 1.1.1 Cheers, David
From 5187231a973a9956723683c7fad14c8bb3dfcac2 Mon Sep 17 00:00:00 2001 From: David Weber <wb@munzinger.de> Date: Sun, 18 Aug 2013 16:16:36 +0200 Subject: [PATCH] Increase virtlockd max clients
--- src/locking/lock_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c4c1727..d96d0fc 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c virLockDaemonNew(bool privileged) return NULL; } - if (!(lockd->srv = virNetServerNew(1, 1, 0, 20, + if (!(lockd->srv = virNetServerNew(1, 1, 0, 100, -1, 0, false, NULL, virLockDaemonClientNew, -- 1.8.1.2
On 18.08.2013 16:46, David Weber wrote:
Hi,
we recently ran into a problem when trying to start more than 20 guests if direct locking with virtlockd was enabled. The error message locked like this: # start test7 error: Failed to start domain test7 error: Cannot recv data: Connection reset by peer
Our research indicated there is a max_clients parameter set in src/locking/lock_daemon.c which causes the limitation. Simply increasing this number works as a workaround.
I don't have a deep understanding of the code, so I don't know if there is a bug and the connection should be dropped after the lock is set or if max_clients really is the way to go. If the latter I guess max_clients should be configurable like in virtlockd or at least be higher than 20 :)
Libvirt version 1.1.1
Cheers, David
From 5187231a973a9956723683c7fad14c8bb3dfcac2 Mon Sep 17 00:00:00 2001 From: David Weber <wb@munzinger.de> Date: Sun, 18 Aug 2013 16:16:36 +0200 Subject: [PATCH] Increase virtlockd max clients
--- src/locking/lock_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c4c1727..d96d0fc 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c virLockDaemonNew(bool privileged) return NULL; }
- if (!(lockd->srv = virNetServerNew(1, 1, 0, 20, + if (!(lockd->srv = virNetServerNew(1, 1, 0, 100, -1, 0, false, NULL, virLockDaemonClientNew,
This will work for a while until somebody tries starting up more than 100 libvirtds. Maybe it's time for /etc/libvirt/virtlockd.conf introduction. Michal
Am Montag, 19. August 2013, 08:08:52 schrieb Michal Privoznik:
On 18.08.2013 16:46, David Weber wrote:
Hi,
we recently ran into a problem when trying to start more than 20 guests if direct locking with virtlockd was enabled. The error message locked like this: # start test7 error: Failed to start domain test7 error: Cannot recv data: Connection reset by peer
Our research indicated there is a max_clients parameter set in src/locking/lock_daemon.c which causes the limitation. Simply increasing this number works as a workaround.
I don't have a deep understanding of the code, so I don't know if there is a bug and the connection should be dropped after the lock is set or if max_clients really is the way to go. If the latter I guess max_clients should be configurable like in virtlockd or at least be higher than 20 :)
Libvirt version 1.1.1
Cheers, David
From 5187231a973a9956723683c7fad14c8bb3dfcac2 Mon Sep 17 00:00:00 2001
From: David Weber <wb@munzinger.de> Date: Sun, 18 Aug 2013 16:16:36 +0200 Subject: [PATCH] Increase virtlockd max clients
---
src/locking/lock_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c4c1727..d96d0fc 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c
virLockDaemonNew(bool privileged)
return NULL;
}
- if (!(lockd->srv = virNetServerNew(1, 1, 0, 20, + if (!(lockd->srv = virNetServerNew(1, 1, 0, 100,
-1, 0, false, NULL, virLockDaemonClientNew,
This will work for a while until somebody tries starting up more than 100 libvirtds. Maybe it's time for /etc/libvirt/virtlockd.conf introduction.
Attached patch makes max_clients configurable in virtlockd.conf Would it make sense to also modify the worker count?
Michal
From 0e6ddd5825792faad5bb4c9ea1d5bd59dd09f821 Mon Sep 17 00:00:00 2001 From: David Weber <wb@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; 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 -- 1.8.1.2
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@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 :|
Am Montag, 19. August 2013, 10:49:35 schrieb Daniel P. Berrange:
On Mon, Aug 19, 2013 at 10:14:55AM +0200, David Weber wrote:
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
Agreed
return data;
}
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.
Ok. Updated patch attached.
From 8889516ec3375463573a819a6e5bec7fdbafbbf3 Mon Sep 17 00:00:00 2001 From: David Weber <wb@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 | 8 ++++++++ 5 files changed, 15 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..8e632f5 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 = 1024; 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..fa4d3f7 100644 --- a/src/locking/virtlockd.conf +++ b/src/locking/virtlockd.conf @@ -58,3 +58,11 @@ # 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. +# 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 +#max_clients = 1024 + -- 1.8.1.2
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 Mon, Aug 19, 2013 at 12:34:27PM +0200, David Weber wrote:
From 8889516ec3375463573a819a6e5bec7fdbafbbf3 Mon Sep 17 00:00:00 2001 From: David Weber <wb@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 | 8 ++++++++ 5 files changed, 15 insertions(+), 3 deletions(-)
ACK, pushed to master. Thanks for your contribution 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 Sun, Aug 18, 2013 at 02:46:54PM +0000, David Weber wrote:
Hi,
we recently ran into a problem when trying to start more than 20 guests if direct locking with virtlockd was enabled. The error message locked like this: # start test7 error: Failed to start domain test7 error: Cannot recv data: Connection reset by peer
Our research indicated there is a max_clients parameter set in src/locking/lock_daemon.c which causes the limitation. Simply increasing this number works as a workaround.
I don't have a deep understanding of the code, so I don't know if there is a bug and the connection should be dropped after the lock is set or if max_clients really is the way to go. If the latter I guess max_clients should be configurable like in virtlockd or at least be higher than 20 :)
Yikes, that was a stupid hardcoded limt on my part ! 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 (3)
-
Daniel P. Berrange -
David Weber -
Michal Privoznik