From b4c1d3d72acc966e9f172cb59002a8ff59fc399a Mon Sep 17 00:00:00 2001
From: Danny Auble <da@schedmd.com>
Date: Tue, 31 Jul 2012 12:29:23 -0700
Subject: [PATCH] Use mount and umount syscalls when handling cgroup
namespaces.
Using the syscalls directly rather than calling bin/(u)mount via
system() avoids a few fork + exec calls, and provides better error
handling if something goes wrong.
Users of this functionality are also updated to use slurm_strerror in
order to provide a more informative error message.
The mount and umount syscalls are Linux-specific, but so are cgroups
so no portability is lost.
---
NEWS | 2 +
src/common/xcgroup.c | 52 ++++++++-----------
src/common/xcgroup.h | 4 ++
.../cgroup/jobacct_gather_cgroup_cpuacct.c | 3 +-
.../cgroup/jobacct_gather_cgroup_memory.c | 3 +-
.../proctrack/cgroup/proctrack_cgroup.c | 3 +-
src/plugins/task/cgroup/task_cgroup_cpuset.c | 2 +-
src/plugins/task/cgroup/task_cgroup_devices.c | 2 +-
src/plugins/task/cgroup/task_cgroup_memory.c | 2 +-
9 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/NEWS b/NEWS
index 89c6ee95883..8d50187127a 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,8 @@ documents those changes that are of interest to users and admins.
with "none" (e.g. "none.o").
-- BGQ - added version string to the load of the runjob_mux plugin to verify
the current plugin has been loaded when using runjob_mux_refresh_config
+ -- Use system mount/umount function calls instead of doing fork exec of
+ mount/umount from Janne Blomqvist.
* Changes in SLURM 2.4.1
========================
diff --git a/src/common/xcgroup.c b/src/common/xcgroup.c
index 2a03ec0dce0..64661d2ebe9 100644
--- a/src/common/xcgroup.c
+++ b/src/common/xcgroup.c
@@ -54,6 +54,7 @@
#include <string.h>
#include <strings.h>
#include <dirent.h>
+#include <sys/mount.h>
#include "slurm/slurm.h"
#include "slurm/slurm_errno.h"
@@ -129,12 +130,14 @@ int xcgroup_ns_destroy(xcgroup_ns_t* cgns) {
* returned values:
* - XCGROUP_ERROR
* - XCGROUP_SUCCESS
+ *
+ * If an error occurs, errno will be set.
*/
int xcgroup_ns_mount(xcgroup_ns_t* cgns)
{
int fstatus;
- char* mount_cmd_fmt;
- char mount_cmd[1024];
+ char* options;
+ char opt_combined[1024];
char* mnt_point;
char* p;
@@ -182,21 +185,20 @@ int xcgroup_ns_mount(xcgroup_ns_t* cgns)
umask(omask);
if (cgns->mnt_args == NULL ||
- strlen(cgns->mnt_args) == 0) {
- mount_cmd_fmt = "/bin/mount -o %s%s -t cgroup none %s";
- }
- else
- mount_cmd_fmt = "/bin/mount -o %s, %s -t cgroup none %s";
-
- if (snprintf(mount_cmd, 1024, mount_cmd_fmt, cgns->subsystems,
- cgns->mnt_args, cgns->mnt_point) >= 1024) {
- debug2("unable to build cgroup ns mount cmd line");
- return XCGROUP_ERROR;
+ strlen(cgns->mnt_args) == 0)
+ options = cgns->subsystems;
+ else {
+ if (snprintf(opt_combined, sizeof(opt_combined), "%s,%s",
+ cgns->subsystems, cgns->mnt_args)
+ >= sizeof(opt_combined)) {
+ debug2("unable to build cgroup options string");
+ return XCGROUP_ERROR;
+ }
+ options = opt_combined;
}
- else
- debug3("cgroup mount cmd line is '%s'", mount_cmd);
- if (system(mount_cmd))
+ if (mount("cgroup", cgns->mnt_point, "cgroup",
+ MS_NOSUID|MS_NOEXEC|MS_NODEV, options))
return XCGROUP_ERROR;
else {
/* we then set the release_agent if necessary */
@@ -217,26 +219,14 @@ int xcgroup_ns_mount(xcgroup_ns_t* cgns)
* returned values:
* - XCGROUP_ERROR
* - XCGROUP_SUCCESS
+ *
+ * If an error occurs, errno will be set.
*/
int xcgroup_ns_umount(xcgroup_ns_t* cgns)
{
- char* umount_cmd_fmt;
- char umount_cmd[1024];
-
- umount_cmd_fmt = "/bin/umount %s";
-
- if (snprintf(umount_cmd, 1024, umount_cmd_fmt,
- cgns->mnt_point) >= 1024) {
- debug2("unable to build cgroup ns umount cmd line");
+ if (umount(cgns->mnt_point))
return XCGROUP_ERROR;
- }
- else
- debug3("cgroup ns umount cmd line is '%s'", umount_cmd);
-
- if (system(umount_cmd))
- return XCGROUP_ERROR;
- else
- return XCGROUP_SUCCESS;
+ return XCGROUP_SUCCESS;
}
/*
diff --git a/src/common/xcgroup.h b/src/common/xcgroup.h
index 4f03b5cef7a..a5861f8eabf 100644
--- a/src/common/xcgroup.h
+++ b/src/common/xcgroup.h
@@ -97,6 +97,8 @@ int xcgroup_ns_destroy(xcgroup_ns_t* cgns);
* returned values:
* - XCGROUP_ERROR
* - XCGROUP_SUCCESS
+ *
+ * If an error occurs, errno will be set.
*/
int xcgroup_ns_mount(xcgroup_ns_t* cgns);
@@ -106,6 +108,8 @@ int xcgroup_ns_mount(xcgroup_ns_t* cgns);
* returned values:
* - XCGROUP_ERROR
* - XCGROUP_SUCCESS
+ *
+ * If an error occurs, errno will be set.
*/
int xcgroup_ns_umount(xcgroup_ns_t* cgns);
diff --git a/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_cpuacct.c b/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_cpuacct.c
index 4918d93ae6c..449d4641d6a 100644
--- a/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_cpuacct.c
+++ b/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_cpuacct.c
@@ -98,7 +98,8 @@ extern int jobacct_gather_cgroup_cpuacct_init(
if (slurm_cgroup_conf->cgroup_automount) {
if (xcgroup_ns_mount(&cpuacct_ns)) {
error("jobacct_gather/cgroup: unable to mount "
- "cpuacct namespace");
+ "cpuacct namespace: %s",
+ slurm_strerror(errno));
goto clean;
}
info("jobacct_gather/cgroup: cpuacct namespace is now "
diff --git a/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_memory.c b/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_memory.c
index 4220f02a1d8..e311faa5f53 100644
--- a/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_memory.c
+++ b/src/plugins/jobacct_gather/cgroup/jobacct_gather_cgroup_memory.c
@@ -98,7 +98,8 @@ extern int jobacct_gather_cgroup_memory_init(
if (slurm_cgroup_conf->cgroup_automount) {
if (xcgroup_ns_mount(&memory_ns)) {
error("jobacct_gather/cgroup: unable to mount "
- "memory namespace");
+ "memory namespace: %s",
+ slurm_strerror(errno));
goto clean;
}
info("jobacct_gather/cgroup: memory namespace is now "
diff --git a/src/plugins/proctrack/cgroup/proctrack_cgroup.c b/src/plugins/proctrack/cgroup/proctrack_cgroup.c
index f6f73995d05..d7026f9adf4 100644
--- a/src/plugins/proctrack/cgroup/proctrack_cgroup.c
+++ b/src/plugins/proctrack/cgroup/proctrack_cgroup.c
@@ -147,7 +147,8 @@ int _slurm_cgroup_init(void)
if (slurm_cgroup_conf.cgroup_automount) {
if (xcgroup_ns_mount(&freezer_ns)) {
error("unable to mount freezer cgroup"
- " namespace");
+ " namespace: %s",
+ slurm_strerror(errno));
return SLURM_ERROR;
}
info("cgroup namespace '%s' is now mounted", "freezer");
diff --git a/src/plugins/task/cgroup/task_cgroup_cpuset.c b/src/plugins/task/cgroup/task_cgroup_cpuset.c
index 8779a7f97a2..c547341c189 100644
--- a/src/plugins/task/cgroup/task_cgroup_cpuset.c
+++ b/src/plugins/task/cgroup/task_cgroup_cpuset.c
@@ -416,7 +416,7 @@ extern int task_cgroup_cpuset_init(slurm_cgroup_conf_t *slurm_cgroup_conf)
if (slurm_cgroup_conf->cgroup_automount) {
if (xcgroup_ns_mount(&cpuset_ns)) {
error("task/cgroup: unable to mount cpuset "
- "namespace");
+ "namespace: %s", slurm_strerror(errno));
goto clean;
}
info("task/cgroup: cpuset namespace is now mounted");
diff --git a/src/plugins/task/cgroup/task_cgroup_devices.c b/src/plugins/task/cgroup/task_cgroup_devices.c
index 303bb43841e..82f802d4645 100644
--- a/src/plugins/task/cgroup/task_cgroup_devices.c
+++ b/src/plugins/task/cgroup/task_cgroup_devices.c
@@ -123,7 +123,7 @@ extern int task_cgroup_devices_init(slurm_cgroup_conf_t *slurm_cgroup_conf)
if ( slurm_cgroup_conf->cgroup_automount ) {
if ( xcgroup_ns_mount(&devices_ns) ) {
error("task/cgroup: unable to mount devices "
- "namespace");
+ "namespace: %s", slurm_strerror(errno));
goto clean;
}
info("task/cgroup: devices namespace is now mounted");
diff --git a/src/plugins/task/cgroup/task_cgroup_memory.c b/src/plugins/task/cgroup/task_cgroup_memory.c
index ef37a0685d6..a2e24806629 100644
--- a/src/plugins/task/cgroup/task_cgroup_memory.c
+++ b/src/plugins/task/cgroup/task_cgroup_memory.c
@@ -109,7 +109,7 @@ extern int task_cgroup_memory_init(slurm_cgroup_conf_t *slurm_cgroup_conf)
if (slurm_cgroup_conf->cgroup_automount) {
if (xcgroup_ns_mount(&memory_ns)) {
error("task/cgroup: unable to mount memory "
- "namespace");
+ "namespace: %s", slurm_strerror(errno));
goto clean;
}
info("task/cgroup: memory namespace is now mounted");
--
GitLab