[libvirt] [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
This patch adds two sets of functions:
1) lower level functions that will immediately set the
RLIMIT_MEMLOCK. RLIMIT_NPROC, or RLIMIT_NOFILE of either the current
process (using setrlimit()) or any other process (using prlimit()).
2) functions for virCommand* that will setup a virCommand object to
set those limits at a later time just after it has forked a new
process, but before it execs the new program.
configure.ac has prlimit and setrlimit added to the list of functions
to check for, and the low level functions are hopefully properly
unsupported error) on all platforms.
You'll notice some ugly typecasting around pid_t; this is all an
attempt to follow the formula given to me by Eric for getting it to
compile without warnings on all platforms.
---
configure.ac | 2 +-
src/libvirt_private.syms | 6 ++
src/util/vircommand.c| 38
src/util/vircommand.h| 4 ++
src/util/virutil.c | 146 +++
src/util/virutil.h | 4 ++
6 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 89dae3d..23c24d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -194,7 +194,7 @@ dnl Availability of various common functions (non-fatal if
missing),
dnl and various less common threadsafe functions
AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \
getpwuid_r getuid initgroups kill mmap newlocale posix_fallocate \
- posix_memalign regexec sched_getaffinity setns symlink])
+ posix_memalign prlimit regexec sched_getaffinity setns setrlimit symlink])
dnl Availability of pthread functions (if missing, win32 threading is
dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2a2c40e..c988c21 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1185,6 +1185,9 @@ virCommandSetErrorFD;
virCommandSetGID;
virCommandSetInputBuffer;
virCommandSetInputFD;
+virCommandSetMaxFiles;
+virCommandSetMaxMemLock;
+virCommandSetMaxProcesses;
virCommandSetOutputBuffer;
virCommandSetOutputFD;
virCommandSetPidFile;
@@ -1904,6 +1907,9 @@ virSetBlocking;
virSetCloseExec;
virSetDeviceUnprivSGIO;
virSetInherit;
+virSetMaxFiles;
+virSetMaxMemLock;
+virSetMaxProcesses;
virSetNonBlock;
virSetUIDGID;
virSetUIDGIDWithCaps;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index ac56a63..d0def8c 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -107,6 +107,10 @@ struct _virCommand {
char *pidfile;
bool reap;
+unsigned long long maxMemLock;
+unsigned int maxProcesses;
+unsigned int maxFiles;
+
uid_t uid;
gid_t gid;
unsigned long long capabilities;
@@ -598,6 +602,13 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (virSetMaxMemLock(-1, cmd-maxMemLock) 0)
+goto fork_error;
+if (virSetMaxProcesses(-1, cmd-maxProcesses) 0)
+goto fork_error;
+if (virSetMaxFiles(-1, cmd-maxFiles) 0)
+goto fork_error;
+
if (cmd-hook) {
VIR_DEBUG(Run hook %p %p, cmd-hook, cmd-opaque);
ret = cmd-hook(cmd-opaque);
@@ -958,6 +969,33 @@ virCommandSetUID(virCommandPtr cmd, uid_t uid)
cmd-uid = uid;
}
+void
+virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxMemLock = bytes;
+}
+
+void
+virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxProcesses = procs;
+}
+
+void
+virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files)
+{
+if (!cmd || cmd-has_error)
+return;
+
+cmd-maxFiles = files;
+}
+
/**
* virCommandClearCaps:
* @cmd: the command to modify
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 6c13795..18568fe 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -65,6 +65,10 @@ void virCommandSetGID(virCommandPtr cmd, gid_t gid);
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 virCommandClearCaps(virCommandPtr cmd);
void virCommandAllowCap(virCommandPtr cmd,
diff --git a/src/util/virutil.c b/src/util/virutil.c
index b9de33c..058a069 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -38,6 +38,10 @@
#include sys/types.h
#include sys/ioctl.h
#include sys/wait.h
+#if HAVE_SETRLIMIT
+# include sys/time.h
+# include sys/resource.h
+#endif
#if HAVE_MMAP
# include sys/mman.h
#endif
@@ -2992,6 +2996,148 @@ virGetGroupName(gid_t gid ATTRIBUTE_UNUSED)
}
#endif /* HAVE_GETPWUID_R */
+#if HAVE_SETRLIMIT
+
+# if HAVE_PRLIMIT
+static int
+virPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+{
+
Re: [libvirt] [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
On Thu, Apr 25, 2013 at 01:57:56PM -0400, Laine Stump wrote:
diff --git a/src/util/virutil.c b/src/util/virutil.c
index b9de33c..058a069 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -38,6 +38,10 @@
#include sys/types.h
#include sys/ioctl.h
#include sys/wait.h
+#if HAVE_SETRLIMIT
+# include sys/time.h
+# include sys/resource.h
+#endif
#if HAVE_MMAP
# include sys/mman.h
#endif
@@ -2992,6 +2996,148 @@ virGetGroupName(gid_t gid ATTRIBUTE_UNUSED)
}
#endif /* HAVE_GETPWUID_R */
+#if HAVE_SETRLIMIT
+
+# if HAVE_PRLIMIT
+static int
+virPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+{
+return prlimit(pid, resource, rlim, NULL);
+}
+# else
+static int
+virPrLimit(pid_t pid ATTRIBUTE_UNUSED,
+ int resource ATTRIBUTE_UNUSED,
+ struct rlimit *rlim ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+# endif
+
+int
+virSetMaxMemLock(pid_t pid, unsigned long long bytes)
+{
+struct rlimit rlim;
+
+if (bytes == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = bytes;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_MEMLOCK, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit locked memory to %llu),
+ bytes);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_MEMLOCK, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit locked memory
+ of process %lld to %llu),
+ (long long int)pid, bytes);
+return -1;
+}
+}
+return 0;
+}
+
+int
+virSetMaxProcesses(pid_t pid, unsigned int procs)
+{
+struct rlimit rlim;
+
+if (procs == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = procs;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_NPROC, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of subprocesses to
%u),
+ procs);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_NPROC, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of subprocesses
+ of process %lld to %u),
+ (long long int)pid, procs);
+return -1;
+}
+}
+return 0;
+}
+
+int
+virSetMaxFiles(pid_t pid, unsigned int files)
+{
+struct rlimit rlim;
+
+if (files == 0)
+return 0;
+
+ /* Max number of opened files is one greater than actual limit. See
+* man setrlimit.
+*
+* NB: That indicates to me that we would want the following code
+* to say files - 1, but the original of this code in
+* qemu_process.c also had files + 1, so this preserves current
+* behavior.
+*/
+rlim.rlim_cur = rlim.rlim_max = files + 1;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_NOFILE, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of open files to
%u),
+ files);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_NOFILE, rlim) 0) {
+virReportSystemError(errno,
+ _(cannot limit number of open files
+ of process %lld to %u),
+ (long long int)pid, files);
+return -1;
+}
+}
+return 0;
+}
+#else
+int
+virSetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED, unsigned long long bytes)
+{
+if (bytes == 0)
+return 0;
+
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+
+int
+virSetMaxProcesses(pid_t pid ATTRIBUTE_UNUSED, unsigned int procs)
+{
+if (procs == 0)
+return 0;
+
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+
+int
+virSetMaxFiles(pid_t pid ATTRIBUTE_UNUSED, unsigned int files)
+{
+if (files == 0)
+return 0;
+
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
+}
+#endif
Since these functions all take pid_t as their first arg,
they should all be named virProcessX and be located
in virprocess.{c,h} rather than virutil.{c,h}
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 :|
--
Re: [libvirt] [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
On 04/25/2013 03:27 PM, Daniel P. Berrange wrote:
Since these functions all take pid_t as their first arg, they should
all be named virProcessX and be located in virprocess.{c,h} rather
than virutil.{c,h}
Yes, of course! The thought Move it somewhere else! was trying to come
to my mind while I was writing it, but I didn't give myself time enough
to think.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] util: new virCommandSetMax(MemLock|Processes|Files)
On 04/25/2013 11:57 AM, Laine Stump wrote:
This patch adds two sets of functions:
1) lower level functions that will immediately set the
RLIMIT_MEMLOCK. RLIMIT_NPROC, or RLIMIT_NOFILE of either the current
process (using setrlimit()) or any other process (using prlimit()).
2) functions for virCommand* that will setup a virCommand object to
set those limits at a later time just after it has forked a new
process, but before it execs the new program.
configure.ac has prlimit and setrlimit added to the list of functions
to check for, and the low level functions are hopefully properly
unsupported error) on all platforms.
You'll notice some ugly typecasting around pid_t; this is all an
attempt to follow the formula given to me by Eric for getting it to
compile without warnings on all platforms.
---
configure.ac | 2 +-
src/libvirt_private.syms | 6 ++
src/util/vircommand.c| 38
src/util/vircommand.h| 4 ++
src/util/virutil.c | 146
+++
src/util/virutil.h | 4 ++
6 files changed, 199 insertions(+), 1 deletion(-)
@@ -598,6 +602,13 @@ virExec(virCommandPtr cmd)
goto fork_error;
}
+if (virSetMaxMemLock(-1, cmd-maxMemLock) 0)
+goto fork_error;
+if (virSetMaxProcesses(-1, cmd-maxProcesses) 0)
+goto fork_error;
+if (virSetMaxFiles(-1, cmd-maxFiles) 0)
+goto fork_error;
Hmm. uid_t and gid_t can validly be 0, hence the sentinel has to be -1.
But pid_t can never be 0 (it starts with 1), so it is easier if you let
0 be the sentinel that means current process.
+#if HAVE_SETRLIMIT
+
+# if HAVE_PRLIMIT
+static int
+virPrLimit(pid_t pid, int resource, struct rlimit *rlim)
+{
+return prlimit(pid, resource, rlim, NULL);
This doesn't report errors;
+}
+# else
+static int
+virPrLimit(pid_t pid ATTRIBUTE_UNUSED,
+ int resource ATTRIBUTE_UNUSED,
+ struct rlimit *rlim ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, %s, _(Not supported on this platform));
+return -1;
while this does. Either the caller should always be responsible for
reporting the errors, or you should fix the HAVE_PRLIMIT version to call
virReportSystemError() as needed. Looking below at your callers, I go
for the latter - simplify the non-prlimit variant to just be:
errno = ENOSYS;
return -1;
[1]...
+}
+# endif
+
+int
+virSetMaxMemLock(pid_t pid, unsigned long long bytes)
+{
+struct rlimit rlim;
+
+if (bytes == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = bytes;
+if (pid == (pid_t)-1) {
Overkill; since there is never a process id of 0, that makes a sentinel
that is much easier to check for. (You did get the cast right, per our
IRC conversation; only I didn't realize why you needed the cast in the
first place - pid_t -1 is the process id that represents the entire
process group of the init(1) process).
+if (setrlimit(RLIMIT_MEMLOCK, rlim) 0) {
RLIMIT_MEMLOCK is not portable. POSIX lists only the following:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/setrlimit.html
RLIMIT_CORE
RLIMIT_CPU
RLIMIT_DATA
RLIMIT_FSIZE
RLIMIT_NOFILE
RLIMIT_STACK
RLIMIT_AS
The rest are extensions, so you'll need to wrap this code in #ifdef
RLIMIT_MEMLOCK and provide a fallback when it is not present. You need
#ifdef instead of #if (POSIX guarantees that all RLIMIT_* extensions in
sys/resource.h will be macros, but does not guarantee which, if any,
of those macros will have the value 0), but at least you don't need to
touch configure.ac for this particular probe.
+virReportSystemError(errno,
+ _(cannot limit locked memory to %llu),
+ bytes);
+return -1;
+}
+} else {
+if (virPrLimit(pid, RLIMIT_MEMLOCK, rlim) 0) {
prlimit(0, ...) is identical to prlimit(getpid(), ...), which in turn is
identical to setrlimit(...). Then again, since setrlimit() is more
portable than prlimit, I agree with you doing the differentiation on the
sentinel pid yourself instead of blindly trying virPrLimit().
+virReportSystemError(errno,
+ _(cannot limit locked memory
+ of process %lld to %llu),
+ (long long int)pid, bytes);
+return -1;
...[1] since here we always report any failure.
+}
+}
+return 0;
+}
+
+int
+virSetMaxProcesses(pid_t pid, unsigned int procs)
+{
+struct rlimit rlim;
+
+if (procs == 0)
+return 0;
+
+rlim.rlim_cur = rlim.rlim_max = procs;
+if (pid == (pid_t)-1) {
+if (setrlimit(RLIMIT_NPROC, rlim) 0) {
Another non-portable limit, needing #ifdef treatment.
...
+rlim.rlim_cur = rlim.rlim_max = files + 1;
+if (pid == (pid_t)-1) {
+if
