Currently the QEMU processes inherit their core dump rlimit from libvirtd, which is really suboptimal. This change allows their limit to be directly controlled from qemu.conf instead. --- src/libvirt_private.syms | 2 ++ src/qemu/libvirtd_qemu.aug | 4 ++++ src/qemu/qemu.conf | 23 ++++++++++++++++++++++- 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, 100 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 419c33d..6dc2b23 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1389,6 +1389,7 @@ virCommandSetErrorFD; virCommandSetGID; virCommandSetInputBuffer; virCommandSetInputFD; +virCommandSetMaxCoreSize; virCommandSetMaxFiles; virCommandSetMaxMemLock; virCommandSetMaxProcesses; @@ -2199,6 +2200,7 @@ virProcessRunInMountNamespace; virProcessSchedPolicyTypeFromString; virProcessSchedPolicyTypeToString; virProcessSetAffinity; +virProcessSetMaxCoreSize; virProcessSetMaxFiles; virProcessSetMaxMemLock; virProcessSetMaxProcesses; diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 8bc23ba..9ec8250 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -22,6 +22,9 @@ module Libvirtd_qemu = let int_entry (kw:string) = [ key kw . value_sep . int_val ] let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ] + let unlimited_val = del /\"/ "\"" . store /unlimited/ . del /\"/ "\"" + let limits_entry (kw:string) = [ key kw . value_sep . unlimited_val ] | [ key kw . value_sep . int_val ] + (* Config entry grouped by function - same order as example config *) let vnc_entry = str_entry "vnc_listen" @@ -72,6 +75,7 @@ module Libvirtd_qemu = | bool_entry "set_process_name" | int_entry "max_processes" | int_entry "max_files" + | limits_entry "max_core" | str_entry "stdio_handler" let device_entry = bool_entry "mac_filter" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 7964273..abf9938 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -401,7 +401,28 @@ #max_processes = 0 #max_files = 0 - +# If max_core is set to a positive integer, then QEMU will be +# permitted to create core dumps when it crashes, provided its +# RAM size is smaller than the limit set. +# +# Be warned that the core dump will include a full copy of the +# guest RAM, unless it has been disabled via the guest XML by +# setting: +# +# <memory dumpcore="off">...guest ram...</memory> +# +# If guest RAM is to be included, ensure the max_core limit +# is set to at least the size of the largest expected guest +# plus another 1GB for any QEMU host side memory mappings. +# +# As a special case it can be set to the string "unlimited" to +# to allow arbitrarily sized core dumps. +# +# By default the core dump size is set to 0 disabling all dumps +# +# Size is in bytes or string "unlimited" +# +#max_core = "unlimited" # mac_filter enables MAC addressed based filtering on bridge ports. # This currently requires ebtables to be installed. diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fa9d65e..45d039c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -393,6 +393,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, char **controllers = NULL; char **hugetlbfs = NULL; char **nvram = NULL; + char *corestr = NULL; /* Just check the file is readable before opening it, otherwise * libvirt emits an error. @@ -633,6 +634,21 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0) goto cleanup; + if (virConfGetValueType(conf, "max_core") == VIR_CONF_STRING) { + if (virConfGetValueString(conf, "max_core", &corestr) < 0) + goto cleanup; + if (STREQ(corestr, "unlimited")) { + cfg->maxCore = ULLONG_MAX; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown core size '%s'"), + corestr); + goto cleanup; + } + } else if (virConfGetValueULLong(conf, "max_core", &cfg->maxCore) < 0) { + goto cleanup; + } + if (virConfGetValueString(conf, "lock_manager", &cfg->lockManagerName) < 0) goto cleanup; if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) < 0) @@ -715,6 +731,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, virStringFreeList(controllers); virStringFreeList(hugetlbfs); virStringFreeList(nvram); + VIR_FREE(corestr); VIR_FREE(user); VIR_FREE(group); virConfFree(conf); 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; unsigned int maxQueuedJobs; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f9efff9..2270fce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5056,6 +5056,7 @@ qemuProcessLaunch(virConnectPtr conn, virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses); virCommandSetMaxFiles(cmd, cfg->maxFiles); + virCommandSetMaxCoreSize(cmd, cfg->maxCore); virCommandSetUmask(cmd, 0x002); VIR_DEBUG("Setting up security labelling"); diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index c4d4f19..a4797af 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -62,6 +62,7 @@ module Test_libvirtd_qemu = { "set_process_name" = "1" } { "max_processes" = "0" } { "max_files" = "0" } +{ "max_core" = "unlimited" } { "mac_filter" = "1" } { "relaxed_acs_check" = "1" } { "allow_disk_format_probing" = "1" } diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 3c67c90..2a59bd1 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -124,6 +124,8 @@ struct _virCommand { unsigned long long maxMemLock; unsigned int maxProcesses; unsigned int maxFiles; + bool setMaxCore; + unsigned long long maxCore; uid_t uid; gid_t gid; @@ -687,6 +689,9 @@ virExec(virCommandPtr cmd) goto fork_error; if (virProcessSetMaxFiles(0, cmd->maxFiles) < 0) goto fork_error; + if (cmd->setMaxCore && + virProcessSetMaxCoreSize(0, cmd->maxCore) < 0) + goto fork_error; if (cmd->hook) { VIR_DEBUG("Run hook %p %p", cmd->hook, cmd->opaque); @@ -1105,6 +1110,15 @@ virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files) cmd->maxFiles = files; } +void virCommandSetMaxCoreSize(virCommandPtr cmd, unsigned long long bytes) +{ + if (!cmd || cmd->has_error) + return; + + cmd->maxCore = bytes; + cmd->setMaxCore = true; +} + void virCommandSetUmask(virCommandPtr cmd, int mask) { if (!cmd || cmd->has_error) diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 44818ef..99dcdeb 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -75,6 +75,7 @@ void virCommandSetUID(virCommandPtr cmd, uid_t uid); void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes); void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs); void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files); +void virCommandSetMaxCoreSize(virCommandPtr cmd, unsigned long long bytes); void virCommandSetUmask(virCommandPtr cmd, int umask); void virCommandClearCaps(virCommandPtr cmd); 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; + if (pid == 0) { + if (setrlimit(RLIMIT_CORE, &rlim) < 0) { + virReportSystemError(errno, + _("cannot limit core file size to %llu"), + bytes); + return -1; + } + } else { + if (virProcessPrLimit(pid, RLIMIT_CORE, &rlim, NULL) < 0) { + virReportSystemError(errno, + _("cannot limit core file size " + "of process %lld to %llu"), + (long long int)pid, bytes); + return -1; + } + } + return 0; +} +#else /* ! (HAVE_SETRLIMIT && defined(RLIMIT_CORE)) */ +int +virProcessSetMaxCoreSize(pid_t pid ATTRIBUTE_UNUSED, + unsigned long long bytes ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} +#endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_CORE)) */ + + #ifdef __linux__ /* * Port of code from polkitunixprocess.c under terms diff --git a/src/util/virprocess.h b/src/util/virprocess.h index a7a1fe9..04e9802 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -75,6 +75,7 @@ int virProcessSetNamespaces(size_t nfdlist, int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes); int virProcessSetMaxProcesses(pid_t pid, unsigned int procs); int virProcessSetMaxFiles(pid_t pid, unsigned int files); +int virProcessSetMaxCoreSize(pid_t pid, unsigned long long bytes); int virProcessGetMaxMemLock(pid_t pid, unsigned long long *bytes); -- 2.7.4

On Wed, Aug 17, 2016 at 05:04:48PM -0400, John Ferlan wrote: There's no built-in types at all in augeas - we just define the grammar entirely ourselves. Ok, i'll reword it. Currently our config file format is essentially python variable initialization syntax, so you can have unquoted numbers, or quoted strings. So we could not have a bare 5G - it would have to be input as a string which I think would be confusing - eg users would expect these to work: max_core = 5368709120 max_core = 5G but would in fact have to use max_core = 5368709120 max_core = "5G" we could solve this by extending the virConf parsing code to deal with bare sized values, but I don't fancy attacking that problem right now. Yep, we can check for 0. Yep. 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 :|

On 08/03/2016 11:31 AM, Daniel P. Berrange wrote: Couldn't git am -3 this one, so I'm sure you have some merges to do in qemu_command.c (at least 90b42f0f and others afterwards from Michal) as well as qemuxml2argvtest.c... Strange alignment... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Will this even matter? Since there's a ternary below? IOW: There's no way to set this to "off" and I assume the by not providing it, it's off. Maybe the ternary doesn't matter... ACK with what appear to be obvious adjustments. John