On Thu, Mar 11, 2010 at 06:00:55PM -0500, Chris Lalancette wrote:
> Currently we have a hard-coded maximum of 100 VNC autoports
> that the qemu driver can use. This may not be enough for
> running large numbers of guests on larger machines.
> However, we don't necessarily necessarily want to make
> it an open-ended number; that could lead to Denial of
> Service. Allow a user-settable option to control the
> number of autoports we will assign. The default is still
> 100, but now users can increase that if they wish.
>
> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
> ---
> src/qemu/libvirtd_qemu.aug | 3 +++
> src/qemu/qemu.conf | 4 ++++
> src/qemu/qemu_conf.c | 6 ++++++
> src/qemu/qemu_conf.h | 2 ++
> src/qemu/qemu_driver.c | 5 +++--
> src/qemu/test_libvirtd_qemu.aug | 14 ++++++++++++++
> 6 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 5bd60b3..0dd89ae 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -13,11 +13,13 @@ module Libvirtd_qemu =
>
> let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\""
> let bool_val = store /0|1/
> + let int_val = store /[0-9]+/
> let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
> let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end
>
> let str_entry (kw:string) = [ key kw . value_sep . str_val ]
> let bool_entry (kw:string) = [ key kw . value_sep . bool_val ]
> + let int_entry (kw:string) = [ key kw . value_sep . int_val ]
> let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
>
>
> @@ -29,6 +31,7 @@ module Libvirtd_qemu =
> | str_entry "vnc_password"
> | bool_entry "vnc_sasl"
> | str_entry "vnc_sasl_dir"
> + | int_entry "vnc_max_connections"
> | str_entry "security_driver"
> | str_entry "user"
> | str_entry "group"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 3da332f..edcf083 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -79,6 +79,10 @@
> # vnc_sasl_dir = "/some/directory/sasl2"
>
>
> +# The maximum number of VNC connections we will allow. Once
> +# libvirtd has created this many qemu guests with VNC ports,
> +# no more guests can be started. The default is 100 guests.
> +# vnc_max_connections = 100
>
>
> # The default security driver is SELinux. If SELinux is disabled
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 40ca221..02186fd 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -113,6 +113,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
> return -1;
> }
>
> + driver->vncMaxConnections = 100;
> +
> #ifdef HAVE_MNTENT_H
> /* For privileged driver, try and find hugepage mount automatically.
> * Non-privileged driver requires admin to create a dir for the
> @@ -211,6 +213,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
> }
> }
>
> + p = virConfGetValue (conf, "vnc_max_connections");
> + CHECK_TYPE ("vnc_max_connections", VIR_CONF_LONG);
> + if (p) driver->vncMaxConnections = p->l;
> +
> p = virConfGetValue (conf, "user");
> CHECK_TYPE ("user", VIR_CONF_STRING);
> if (!(user = strdup(p && p->str ? p->str : QEMU_USER))) {
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 6a9de5e..e0c4a9b 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -122,6 +122,8 @@ struct qemud_driver {
> char *vncListen;
> char *vncPassword;
> char *vncSASLdir;
> + unsigned int vncMaxConnections;
> +
> char *hugetlbfs_mount;
> char *hugepage_path;
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 49983dd..8324075 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2147,10 +2147,11 @@ qemuInitPCIAddresses(struct qemud_driver *driver,
> return ret;
> }
>
> -static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
> +static int qemudNextFreeVNCPort(struct qemud_driver *driver)
> +{
> int i;
>
> - for (i = 5900 ; i < 6000 ; i++) {
> + for (i = 5900 ; i < (5900 + driver->vncMaxConnections) ; i++) {
I would just simplify the whole patch to just
+ for (i = 5900 ; i < 65535 ; i++) {
> int fd;
> int reuse = 1;
> struct sockaddr_in addr;
> diff --git a/src/qemu/test_libvirtd_qemu.aug b/src/qemu/test_libvirtd_qemu.aug
> index 2feedc0..3c2190a 100644
> --- a/src/qemu/test_libvirtd_qemu.aug
> +++ b/src/qemu/test_libvirtd_qemu.aug
> @@ -80,6 +80,13 @@ vnc_sasl = 1
> #
> vnc_sasl_dir = \"/some/directory/sasl2\"
>
> +
> +# The maximum number of VNC connections we will allow. Once
> +# libvirtd has created this many qemu guests with VNC ports,
> +# no more guests can be started. The default is 100 guests.
> +#
> +vnc_max_connections = 100
> +
> security_driver = \"selinux\"
>
> user = \"root\"
> @@ -180,6 +187,13 @@ relaxed_acs_check = 1
> { "#comment" = "" }
> { "vnc_sasl_dir" = "/some/directory/sasl2" }
> { "#empty" }
> +{ "#empty" }
> +{ "#comment" = "The maximum number of VNC connections we will allow. Once" }
> +{ "#comment" = "libvirtd has created this many qemu guests with VNC ports," }
> +{ "#comment" = "no more guests can be started. The default is 100 guests." }
> +{ "#comment" = "" }
> +{ "vnc_max_connections" = "100" }
> +{ "#empty" }
> { "security_driver" = "selinux" }
> { "#empty" }
> { "user" = "root" }
Hum, why do we need any hardcoded limit ? If someone can exhaust
the node capacity just by creating as many VMs as he likes I think
trying to protect the port set is really not the right approach, plus
that's yet another tuneable which will get in the way for normal uses.
My instinct would be to just drop that upper limit (well we will be
limited by 2^^16 in any case, but the day we manage to run that many
VM the port exhaution will be a fairly minor draback :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list