On Tue, Jul 12, 2016 at 11:38:22AM +0200, Martin Kletzander wrote:
On Tue, Jul 12, 2016 at 10:12:36AM +0100, Daniel P. Berrange wrote:
Currently the QEMU processes inherit their core dump rlimit from libvirtd, which is really suboptimal. This change allows their limit to be directly controller from qemu.conf instead. ---
Changed in v2:
- Allow use of string "unlimited"
src/libvirt_private.syms | 2 ++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 16 +++++++++++++++- src/qemu/qemu_conf.c | 17 +++++++++++++++++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_process.c | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + src/util/vircommand.c | 14 ++++++++++++++ src/util/vircommand.h | 1 + src/util/virprocess.c | 36 ++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 1 + 11 files changed, 90 insertions(+), 1 deletion(-)
[...]
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 8bc23ba..a8edc2b 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -72,6 +72,7 @@ module Libvirtd_qemu = | bool_entry "set_process_name" | int_entry "max_processes" | int_entry "max_files" + | int_entry "max_core"
This should be expanded to allow "unlimited" as well.
Opps, yes.
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 510cd9a..b730202 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -148,6 +148,7 @@ struct _virQEMUDriverConfig {
unsigned int maxProcesses; unsigned int maxFiles; + unsigned long long maxCore;
This is not initialized anywhere, effectively making the limit default to 0 IIUC.
Yes, that's correct as per the qemu.conf docs - we don't allow core dumps by default, since they can be absolutely massive and so have a significant impact on the host OS in terms of time & I/O bandwidth required to dump the guest, as well as disk space usage. So core dumps are an opt-in thing.
diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 09dd3c9..2b71445 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -914,6 +914,42 @@ virProcessSetMaxFiles(pid_t pid ATTRIBUTE_UNUSED, unsigned int files) } #endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_NOFILE)) */
+#if HAVE_SETRLIMIT && defined(RLIMIT_CORE) +int +virProcessSetMaxCoreSize(pid_t pid, unsigned long long bytes) +{ + struct rlimit rlim; + + rlim.rlim_cur = rlim.rlim_max = bytes;
Shouldn't be this set to RLIM_INFINITY if "unlimited" was requested? Not taht ULLONG_MAX would not be enough, it's just that it's rlim_t and it would be nicer to use it as described.
IIUC, you're effectively saying that we should do if (bytes == ULLONG_MAX) rlim_max = RLIM_INFINITY; else rlim_max = bytes; RLIM_INFINITY is just an integer constant - if that's not the same as ULONG_MAX, then we ought to deal with that right up front a time of parsing in QEMU. eg add a virConfGetValueRLin() that returns an rlim_t value so we get range checking. 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 :|