[libvirt] [PATCH] qemu: Support for overriding NPROC limit
This patch adds max_processes option to qemu.conf which can be used to override system default limit on number of processes that are allowed to be running for qemu user. --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 7 +++++++ src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_process.c | 24 ++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 4 ++++ 6 files changed, 44 insertions(+), 0 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index affd74e..ac30b8e 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 ] @@ -45,6 +47,7 @@ module Libvirtd_qemu = | bool_entry "clear_emulator_capabilities" | bool_entry "allow_disk_format_probing" | bool_entry "set_process_name" + | int_entry "max_processes" (* Each enty in the config is one of the following three ... *) let entry = vnc_entry diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 364f555..c70050e 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -273,3 +273,10 @@ # its arguments) appear in process listings. # # set_process_name = 1 + + +# If max_processes is set to a positive integer, libvirt will use it to set +# maximum number of processes that can be run by qemu user. This can be used to +# override default value set by host OS. +# +# max_processes = 0 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 9ba60b1..bb5421b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -424,6 +424,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, CHECK_TYPE ("set_process_name", VIR_CONF_LONG); if (p) driver->setProcessName = p->l; + p = virConfGetValue(conf, "max_processes"); + CHECK_TYPE("max_processes", VIR_CONF_LONG); + if (p) driver->maxProcesses = p->l; + virConfFree (conf); return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 7c6fde7..94918f6 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -105,6 +105,8 @@ struct qemud_driver { unsigned int allowDiskFormatProbing : 1; unsigned int setProcessName : 1; + int maxProcesses; + virCapsPtr caps; /* An array of callbacks */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 48ecd5c..9ada24d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,6 +25,8 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/time.h> +#include <sys/resource.h> #include "qemu_process.h" #include "qemu_domain.h" @@ -1811,6 +1813,25 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, } +static int +qemuProcessLimits(struct qemud_driver *driver) +{ + if (driver->maxProcesses > 0) { + struct rlimit rlim; + + rlim.rlim_cur = rlim.rlim_max = driver->maxProcesses; + if (setrlimit(RLIMIT_NPROC, &rlim) < 0) { + virReportSystemError(errno, + _("cannot limit number of processes to %d"), + driver->maxProcesses); + return -1; + } + } + + return 0; +} + + struct qemuProcessHookData { virConnectPtr conn; virDomainObjPtr vm; @@ -1821,6 +1842,9 @@ static int qemuProcessHook(void *data) { struct qemuProcessHookData *h = data; + if (qemuProcessLimits(h->driver) < 0) + return -1; + /* This must take place before exec(), so that all QEMU * memory allocation is on the correct NUMA node */ diff --git a/src/qemu/test_libvirtd_qemu.aug b/src/qemu/test_libvirtd_qemu.aug index 8e477f5..917bd4f 100644 --- a/src/qemu/test_libvirtd_qemu.aug +++ b/src/qemu/test_libvirtd_qemu.aug @@ -111,6 +111,8 @@ clear_emulator_capabilities = 0 allow_disk_format_probing = 1 vnc_auto_unix_socket = 1 + +max_processes = 12345 " test Libvirtd_qemu.lns get conf = @@ -232,3 +234,5 @@ vnc_auto_unix_socket = 1 { "allow_disk_format_probing" = "1" } { "#empty" } { "vnc_auto_unix_socket" = "1" } +{ "#empty" } +{ "max_processes" = "12345" } -- 1.7.4.1
On 04/05/2011 08:09 AM, Jiri Denemark wrote:
This patch adds max_processes option to qemu.conf which can be used to override system default limit on number of processes that are allowed to be running for qemu user. --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 7 +++++++ src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_process.c | 24 ++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 4 ++++ 6 files changed, 44 insertions(+), 0 deletions(-) @@ -1821,6 +1842,9 @@ static int qemuProcessHook(void *data) { struct qemuProcessHookData *h = data;
+ if (qemuProcessLimits(h->driver) < 0) + return -1;
Which UID is in effect at this point? While setrlimit() adjusts an inherited value, I'm a bit worried that tracks different limits per uid, and if the call is made while the real uid is root instead of qemu, then we might be affecting the wrong limit. Does this call need to be delayed until after the virSecurityManagerSetProcessLabel, or after we are sure that we have changed identities? But other than that question, the code looks sane, and I appreciate seeing the augeas modifications, too. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Apr 05, 2011 at 09:28:00 -0600, Eric Blake wrote:
On 04/05/2011 08:09 AM, Jiri Denemark wrote:
This patch adds max_processes option to qemu.conf which can be used to override system default limit on number of processes that are allowed to be running for qemu user. --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 7 +++++++ src/qemu/qemu_conf.c | 4 ++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_process.c | 24 ++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 4 ++++ 6 files changed, 44 insertions(+), 0 deletions(-) @@ -1821,6 +1842,9 @@ static int qemuProcessHook(void *data) { struct qemuProcessHookData *h = data;
+ if (qemuProcessLimits(h->driver) < 0) + return -1;
Which UID is in effect at this point? While setrlimit() adjusts an inherited value, I'm a bit worried that tracks different limits per uid, and if the call is made while the real uid is root instead of qemu, then we might be affecting the wrong limit. Does this call need to be delayed until after the virSecurityManagerSetProcessLabel, or after we are sure that we have changed identities?
Actually it's important that we call setrlimit() before dropping root privileges since only such process can increase the limit. The NPROC limit is pretty confusing since it logically doesn't fit in setrlimit and by it gets unnatural behavior as a result of it being set via setrlimit. It only affects the limit seen by current process which will soon be executing qemu code. Jirka
On 04/05/2011 01:46 PM, Jiri Denemark wrote:
On Tue, Apr 05, 2011 at 09:28:00 -0600, Eric Blake wrote:
On 04/05/2011 08:09 AM, Jiri Denemark wrote:
This patch adds max_processes option to qemu.conf which can be used to override system default limit on number of processes that are allowed to be running for qemu user.
Actually it's important that we call setrlimit() before dropping root privileges since only such process can increase the limit.
Makes sense.
The NPROC limit is pretty confusing since it logically doesn't fit in setrlimit and by it gets unnatural behavior as a result of it being set via setrlimit. It only affects the limit seen by current process which will soon be executing qemu code.
Yeah, there's no real way to confine the absolute number of processes owned by a single uid using just process inheritance; setrlimit only affects the number of processes that can be forked within a given hierarchy. At any rate, this patch is certainly better than what was previously available, I didn't see anything wrong with it, and you answered my question, so: ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Apr 05, 2011 at 14:12:03 -0600, Eric Blake wrote:
On 04/05/2011 01:46 PM, Jiri Denemark wrote:
On Tue, Apr 05, 2011 at 09:28:00 -0600, Eric Blake wrote:
On 04/05/2011 08:09 AM, Jiri Denemark wrote:
This patch adds max_processes option to qemu.conf which can be used to override system default limit on number of processes that are allowed to be running for qemu user.
Actually it's important that we call setrlimit() before dropping root privileges since only such process can increase the limit.
Makes sense.
The NPROC limit is pretty confusing since it logically doesn't fit in setrlimit and by it gets unnatural behavior as a result of it being set via setrlimit. It only affects the limit seen by current process which will soon be executing qemu code.
Yeah, there's no real way to confine the absolute number of processes owned by a single uid using just process inheritance; setrlimit only affects the number of processes that can be forked within a given hierarchy.
At any rate, this patch is certainly better than what was previously available, I didn't see anything wrong with it, and you answered my question, so:
ACK.
Thanks, pushed. Jirka
participants (2)
-
Eric Blake -
Jiri Denemark