On 08/03/2016 11:31 AM, 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 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 ]
+
Is there no 'uulong_val'? At the very least uint? I don't know much
about this aug stuff, so I defer to you on this.
(* 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.
Providing a negative value will cause libvirtd startup failure... The
way this is worded could lead one to believe they could provide a signed
value.
+#
+# 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"
Would it be overly complicated to alter max_core to be "unlimited" or a
sized value? I would think it would be easier to type/see "5G" rather
than do the math! or fat-finger or cut-n-paste wrong...
# 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;
Should this check if bytes == 0 before setting to true? If not, then
another change probably would be necessary below... [1]
+ 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)
+{
[1] Similar to other such functions should this have a :
if (!bytes)
return 0;
and of course a removed ATTRIBUTE_UNUSED on bytes above...
logic:
maxCore defaults to 0.
qemuProcessLaunch calls virCommandSetMaxCoreSize (setting setMaxCore)
in virExec if setMaxCore is true, then call virProcessSetMaxCoreSize
ACK - in principal - your call on the sized/scaled value, but I do think
this !bytes should be done...
John
+ 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);