On 03/18/2015 08:36 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 controller from qemu.conf instead.
---
src/libvirt_private.syms | 2 ++
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 12 ++++++++++++
src/qemu/qemu_conf.c | 3 +++
src/qemu/qemu_conf.h | 2 ++
src/qemu/qemu_process.c | 2 ++
src/qemu/test_libvirtd_qemu.aug.in | 1 +
src/util/vircommand.c | 14 ++++++++++++++
src/util/vircommand.h | 1 +
src/util/virprocess.c | 35 +++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 1 +
11 files changed, 74 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ca3520d..7446357 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1240,6 +1240,7 @@ virCommandSetErrorFD;
virCommandSetGID;
virCommandSetInputBuffer;
virCommandSetInputFD;
+virCommandSetMaxCoreSize;
virCommandSetMaxFiles;
virCommandSetMaxMemLock;
virCommandSetMaxProcesses;
@@ -1951,6 +1952,7 @@ virProcessRunInMountNamespace;
virProcessSchedPolicyTypeFromString;
virProcessSchedPolicyTypeToString;
virProcessSetAffinity;
+virProcessSetMaxCoreSize;
virProcessSetMaxFiles;
virProcessSetMaxMemLock;
virProcessSetMaxProcesses;
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 62951da..029a55a 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -71,6 +71,7 @@ module Libvirtd_qemu =
| bool_entry "set_process_name"
| int_entry "max_processes"
| int_entry "max_files"
+ | int_entry "max_core"
let device_entry = bool_entry "mac_filter"
| bool_entry "relaxed_acs_check"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 1c589a2..12e4326 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -390,6 +390,18 @@
#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, so if
+# the largest guest is 32 GB in size, the max_core limit will
+# have to be at least 33/34 GB to allow enough overhead.
+#
+# By default it will inherit core limit from libvirtd, which
+# is usually set to 0 by systemd/init.
+#
+# Size is in bytes
+#max_core = 0
It's too bad it cannot be a "sized" value... Reading 20G in a file for
me is so much easier than the comparable byte value say nothing if we
get to 128G, 1T, etc. (some day).
# mac_filter enables MAC addressed based filtering on bridge ports.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index bca05c9..97bb8b8 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -770,6 +770,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_VALUE_BOOL("set_process_name", cfg->setProcessName);
GET_VALUE_ULONG("max_processes", cfg->maxProcesses);
GET_VALUE_ULONG("max_files", cfg->maxFiles);
+ GET_VALUE_ULONG("max_core", cfg->maxCore);
+ if (virConfGetValue(conf, "max_core"))
+ cfg->setMaxCore = true;
Should we ensure cfg->maxCore >= 0 (or does that get magically done by
one the many macros). Paranoia based on shared LL/LLU algorithm (and
that my eyes/brain is tired). Which leads me down the path of - if
'max_core' is set to zero or doesn't exist, then cfg->maxCore would be
zero - so is the 'setMaxCore' necessary.
GET_VALUE_STR("lock_manager", cfg->lockManagerName);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index cb01fb6..ac7a1bf 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -143,6 +143,8 @@ struct _virQEMUDriverConfig {
int maxProcesses;
int maxFiles;
+ bool setMaxCore;
+ unsigned long long maxCore;
int maxQueuedJobs;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 37cdb8f..995d6ef 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4731,6 +4731,8 @@ int qemuProcessStart(virConnectPtr conn,
virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData);
virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
virCommandSetMaxFiles(cmd, cfg->maxFiles);
+ if (cfg->setMaxCore)
+ 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 fc4935b..9bb3683 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -61,6 +61,7 @@ module Test_libvirtd_qemu =
{ "set_process_name" = "1" }
{ "max_processes" = "0" }
{ "max_files" = "0" }
+{ "max_core" = "0" }
{ "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 648f5ed..1982401 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -123,6 +123,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;
@@ -686,6 +688,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);
@@ -1108,6 +1113,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 198da2f..4f2874d 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 aa00a99..02d91d9 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -825,6 +825,41 @@ 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) < 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)
Aren't they both ATTRIBUTE_UNUSED
In general an ACK with at least this change... I just keep wondering
what happens if someone provides -1 for the value...
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 c812882..cd7b7a2 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);
/* Callback to run code within the mount namespace tied to the given
* pid. This function must use only async-signal-safe functions, as