Merge pull request #2170 from brauner/2018-02-16/cgfsng_force_cgroup_mount

CODING_STYLE.md

@@ -177,7 +177,7 @@
           }
   ```
 
-#### Functions Not Returning Booleans Must Assigned Return Value Before Performing Checks
+#### Functions Not Returning Booleans Must Assign Return Value Before Performing Checks
 
 - When checking whether a function not returning booleans was successful or not
   the returned value must be assigned before it is checked (`str{n}cmp()`
@@ -201,6 +201,60 @@
           continue;
   ```
 
+#### Non-Boolean Functions That Behave Like Boolean Functions Must Explicitly Check Against A Value
+
+- This rule mainly exists for `str{n}cmp()` type functions. In most cases they
+  are used like a boolean function to check whether a string matches or not.
+  But they return an integer. It is perfectly fine to check `str{n}cmp()`
+  functions directly but you must compare explicitly against a value. That is
+  to say, while they are conceptually boolean functions they shouldn't be
+  treated as such since they don't really behave like boolean functions. So
+  `if (!str{n}cmp())` and `if (str{n}cmp())` checks must not be used. Good
+  examples are found in the following functions:
+  ```
+  static int set_config_hooks(const char *key, const char *value,
+                              struct lxc_conf *lxc_conf, void *data)
+
+          char *copy;
+
+          if (lxc_config_value_empty(value))
+                  return lxc_clear_hooks(lxc_conf, key);
+
+          if (strcmp(key + 4, "hook") == 0) {
+                  ERROR("lxc.hook must not have a value");
+                  return -1;
+          }
+
+          copy = strdup(value);
+          if (!copy)
+                  return -1;
+
+          if (strcmp(key + 9, "pre-start") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_PRESTART, copy);
+          else if (strcmp(key + 9, "start-host") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_START_HOST, copy);
+          else if (strcmp(key + 9, "pre-mount") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_PREMOUNT, copy);
+          else if (strcmp(key + 9, "autodev") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_AUTODEV, copy);
+          else if (strcmp(key + 9, "mount") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_MOUNT, copy);
+          else if (strcmp(key + 9, "start") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_START, copy);
+          else if (strcmp(key + 9, "stop") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_STOP, copy);
+          else if (strcmp(key + 9, "post-stop") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_POSTSTOP, copy);
+          else if (strcmp(key + 9, "clone") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_CLONE, copy);
+          else if (strcmp(key + 9, "destroy") == 0)
+                  return add_hook(lxc_conf, LXCHOOK_DESTROY, copy);
+
+          free(copy);
+          return -1;
+  }
+  ```
+
 #### Do Not Use C99 Variable Length Arrays (VLA)
 
 - They are made optional and there is no guarantee that future C standards

doc/lxc.container.conf.sgml.in

@@ -1126,36 +1126,75 @@ dev/null proc/kcore none bind,relative 0 0
                   <filename>/sys</filename> as read-write
                 </para>
               </listitem>
+
               <listitem>
                 <para>
                   <option>cgroup:mixed</option>:
-                  mount a tmpfs to <filename>/sys/fs/cgroup</filename>,
-                  create directories for all hierarchies to which
-                  the container is added, create subdirectories
-                  there with the name of the cgroup, and bind-mount
-                  the container's own cgroup into that directory.
-                  The container will be able to write to its own
-                  cgroup directory, but not the parents, since they
-                  will be remounted read-only.
+                  Mount a tmpfs to <filename>/sys/fs/cgroup</filename>,
+                  create directories for all hierarchies to which the container
+                  is added, create subdirectories in those hierarchies with the
+                  name of the cgroup, and bind-mount the container's own cgroup
+                  into that directory. The container will be able to write to
+                  its own cgroup directory, but not the parents, since they will
+                  be remounted read-only.
                 </para>
               </listitem>
+
               <listitem>
                 <para>
-                  <option>cgroup:ro</option>: similar to
-                  <option>cgroup:mixed</option>, but everything will
-                be mounted read-only.
+                  <option>cgroup:mixed:force</option>:
+                  The <option>force</option> option will cause LXC to perform
+                  the cgroup mounts for the container under all circumstances.
+                  Otherwise it is similar to <option>cgroup:mixed</option>.
+                  This is mainly useful when the cgroup namespaces are enabled
+                  where LXC will normally leave mounting cgroups to the init
+                  binary of the container since it is perfectly safe to do so.
                 </para>
               </listitem>
+
+              <listitem>
+                <para>
+                  <option>cgroup:ro</option>:
+                  similar to <option>cgroup:mixed</option>, but everything will
+                  be mounted read-only.
+                </para>
+              </listitem>
+
+              <listitem>
+                <para>
+                  <option>cgroup:ro:force</option>:
+                  The <option>force</option> option will cause LXC to perform
+                  the cgroup mounts for the container under all circumstances.
+                  Otherwise it is similar to <option>cgroup:ro</option>.
+                  This is mainly useful when the cgroup namespaces are enabled
+                  where LXC will normally leave mounting cgroups to the init
+                  binary of the container since it is perfectly safe to do so.
+                </para>
+              </listitem>
+
               <listitem>
                 <para>
                   <option>cgroup:rw</option>: similar to
-                  <option>cgroup:mixed</option>, but everything will
-                  be mounted read-write. Note that the paths leading
-                  up to the container's own cgroup will be writable,
-                  but will not be a cgroup filesystem but just part
-                  of the tmpfs of <filename>/sys/fs/cgroup</filename>
+                  <option>cgroup:mixed</option>, but everything will be mounted
+                  read-write. Note that the paths leading up to the container's
+                  own cgroup will be writable, but will not be a cgroup
+                  filesystem but just part of the tmpfs of
+                  <filename>/sys/fs/cgroup</filename>
+                </para>
+              </listitem>
+
+              <listitem>
+                <para>
+                  <option>cgroup:rw:force</option>:
+                  The <option>force</option> option will cause LXC to perform
+                  the cgroup mounts for the container under all circumstances.
+                  Otherwise it is similar to <option>cgroup:rw</option>.
+                  This is mainly useful when the cgroup namespaces are enabled
+                  where LXC will normally leave mounting cgroups to the init
+                  binary of the container since it is perfectly safe to do so.
                 </para>
               </listitem>
+
               <listitem>
                 <para>
                   <option>cgroup</option> (without specifier):

src/lxc/cgroups/cgfsng.c

@@ -2043,26 +2043,31 @@ static int cg_mount_in_cgroup_namespace(int type, struct hierarchy *h,
 
 static bool cgfsng_mount(void *hdata, const char *root, int type)
 {
-	int i;
+	int i, ret;
 	char *tmpfspath = NULL;
 	bool retval = false;
 	struct lxc_handler *handler = hdata;
 	struct cgfsng_handler_data *d = handler->cgroup_data;
-	bool has_cgns = false, has_sys_admin = true;
+	bool has_cgns = false, wants_force_mount = false;
 
 	if ((type & LXC_AUTO_CGROUP_MASK) == 0)
 		return true;
 
-	has_cgns = cgns_supported();
-	if (!lxc_list_empty(&handler->conf->keepcaps))
-		has_sys_admin = in_caplist(CAP_SYS_ADMIN, &handler->conf->keepcaps);
-	else
-		has_sys_admin = !in_caplist(CAP_SYS_ADMIN, &handler->conf->caps);
+	if (type & LXC_AUTO_CGROUP_FORCE) {
+		type &= ~LXC_AUTO_CGROUP_FORCE;
+		wants_force_mount = true;
+	}
 
-	if (has_cgns && has_sys_admin)
-		return true;
+	if (!wants_force_mount){
+		if (!lxc_list_empty(&handler->conf->keepcaps))
+			wants_force_mount = !in_caplist(CAP_SYS_ADMIN, &handler->conf->keepcaps);
+		else
+			wants_force_mount = in_caplist(CAP_SYS_ADMIN, &handler->conf->caps);
+	}
 
-	tmpfspath = must_make_path(root, "/sys/fs/cgroup", NULL);
+	has_cgns = cgns_supported();
+	if (has_cgns && !wants_force_mount)
+		return true;
 
 	if (type == LXC_AUTO_CGROUP_NOSPEC)
 		type = LXC_AUTO_CGROUP_MIXED;
@@ -2070,17 +2075,17 @@ static bool cgfsng_mount(void *hdata, const char *root, int type)
 		type = LXC_AUTO_CGROUP_FULL_MIXED;
 
 	/* Mount tmpfs */
-	if (safe_mount("cgroup_root", tmpfspath, "tmpfs",
-			MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_RELATIME,
-			"size=10240k,mode=755",
-			root) < 0)
-		goto  bad;
+	tmpfspath = must_make_path(root, "/sys/fs/cgroup", NULL);
+	ret = safe_mount("cgroup_root", tmpfspath, "tmpfs",
+			 MS_NOSUID | MS_NODEV | MS_NOEXEC | MS_RELATIME,
+			 "size=10240k,mode=755", root);
+	if (ret < 0)
+		goto on_error;
 
 	for (i = 0; hierarchies[i]; i++) {
 		char *controllerpath, *path2;
 		struct hierarchy *h = hierarchies[i];
 		char *controller = strrchr(h->mountpoint, '/');
-		int r;
 
 		if (!controller)
 			continue;
@@ -2090,49 +2095,56 @@ static bool cgfsng_mount(void *hdata, const char *root, int type)
 			free(controllerpath);
 			continue;
 		}
-		if (mkdir(controllerpath, 0755) < 0) {
+		ret = mkdir(controllerpath, 0755);
+		if (ret < 0) {
 			SYSERROR("Error creating cgroup path: %s", controllerpath);
 			free(controllerpath);
-			goto bad;
+			goto on_error;
 		}
 
-		if (has_cgns && !has_sys_admin) {
+		if (has_cgns && wants_force_mount) {
 			/* If cgroup namespaces are supported but the container
 			 * will not have CAP_SYS_ADMIN after it has started we
 			 * need to mount the cgroups manually.
 			 */
-			r = cg_mount_in_cgroup_namespace(type, h, controllerpath);
+			ret = cg_mount_in_cgroup_namespace(type, h, controllerpath);
 			free(controllerpath);
-			if (r < 0)
-				goto bad;
+			if (ret < 0)
+				goto on_error;
+
 			continue;
 		}
 
-		if (mount_cgroup_full(type, h, controllerpath, d->container_cgroup) < 0) {
+		ret = mount_cgroup_full(type, h, controllerpath, d->container_cgroup);
+		if (ret < 0) {
 			free(controllerpath);
-			goto bad;
+			goto on_error;
 		}
+
 		if (!cg_mount_needs_subdirs(type)) {
 			free(controllerpath);
 			continue;
 		}
-		path2 = must_make_path(controllerpath, h->base_cgroup, d->container_cgroup, NULL);
-		if (mkdir_p(path2, 0755) < 0) {
+
+		path2 = must_make_path(controllerpath, h->base_cgroup,
+				       d->container_cgroup, NULL);
+		ret = mkdir_p(path2, 0755);
+		if (ret < 0) {
 			free(controllerpath);
 			free(path2);
-			goto bad;
+			goto on_error;
 		}
 
-		r = do_secondstage_mounts_if_needed(type, h, controllerpath, path2,
-						    d->container_cgroup);
+		ret = do_secondstage_mounts_if_needed(
+		    type, h, controllerpath, path2, d->container_cgroup);
 		free(controllerpath);
 		free(path2);
-		if (r < 0)
-			goto bad;
+		if (ret < 0)
+			goto on_error;
 	}
 	retval = true;
 
-bad:
+on_error:
 	free(tmpfspath);
 	return retval;
 }

src/lxc/conf.c

@@ -715,7 +715,7 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
 	if (flags & LXC_AUTO_CGROUP_MASK) {
 		int cg_flags;
 
-		cg_flags = flags & LXC_AUTO_CGROUP_MASK;
+		cg_flags = flags & (LXC_AUTO_CGROUP_MASK & ~LXC_AUTO_CGROUP_FORCE);
 		/* If the type of cgroup mount was not specified, it depends on the
 		 * container's capabilities as to what makes sense: if we have
 		 * CAP_SYS_ADMIN, the read-only part can be remounted read-write
@@ -737,7 +737,8 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
 			else
 				cg_flags = has_sys_admin ? LXC_AUTO_CGROUP_FULL_RW : LXC_AUTO_CGROUP_FULL_MIXED;
 		}
-
+		if (flags & LXC_AUTO_CGROUP_FORCE)
+				cg_flags |= LXC_AUTO_CGROUP_FORCE;
 		if (!cgroup_mount(conf->rootfs.path ? conf->rootfs.mount : "", handler, cg_flags)) {
 			SYSERROR("error mounting /sys/fs/cgroup");
 			return -1;
@@ -3179,7 +3180,7 @@ void remount_all_slave(void)
 	free(line);
 }
 
-void lxc_execute_bind_init(struct lxc_conf *conf)
+static int lxc_execute_bind_init(struct lxc_conf *conf)
 {
 	int ret;
 	char path[PATH_MAX], destpath[PATH_MAX], *p;
@@ -3188,39 +3189,44 @@ void lxc_execute_bind_init(struct lxc_conf *conf)
 	p = choose_init(conf->rootfs.mount);
 	if (p) {
 		free(p);
-		return;
+		return 0;
 	}
 
 	ret = snprintf(path, PATH_MAX, SBINDIR "/init.lxc.static");
 	if (ret < 0 || ret >= PATH_MAX) {
-		WARN("Path name too long searching for lxc.init.static");
-		return;
+		ERROR("Path name too long searching for lxc.init.static");
+		return -1;
 	}
 
 	if (!file_exists(path)) {
-		INFO("%s does not exist on host", path);
-		return;
+		ERROR("%s does not exist on host", path);
+		return -1;
 	}
 
 	ret = snprintf(destpath, PATH_MAX, "%s%s", conf->rootfs.mount, "/init.lxc.static");
 	if (ret < 0 || ret >= PATH_MAX) {
-		WARN("Path name too long for container's lxc.init.static");
-		return;
+		ERROR("Path name too long for container's lxc.init.static");
+		return -1;
 	}
 
 	if (!file_exists(destpath)) {
-		FILE * pathfile = fopen(destpath, "wb");
+		FILE *pathfile = fopen(destpath, "wb");
 		if (!pathfile) {
-			SYSERROR("Failed to create mount target '%s'", destpath);
-			return;
+			SYSERROR("Failed to create mount target \"%s\"", destpath);
+			return -1;
 		}
+
 		fclose(pathfile);
 	}
 
 	ret = safe_mount(path, destpath, "none", MS_BIND, NULL, conf->rootfs.mount);
-	if (ret < 0)
+	if (ret < 0) {
 		SYSERROR("Failed to bind lxc.init.static into container");
-	INFO("lxc.init.static bound into container at %s", path);
+		return -1;
+	}
+
+	INFO("Bind mounted lxc.init.static into container at \"%s\"", path);
+	return 0;
 }
 
 /*
@@ -3290,45 +3296,52 @@ int lxc_setup(struct lxc_handler *handler)
 	struct lxc_conf *lxc_conf = handler->conf;
 	const char *lxcpath = handler->lxcpath;
 
-	if (do_rootfs_setup(lxc_conf, name, lxcpath) < 0) {
-		ERROR("Error setting up rootfs mount after spawn");
+	ret = do_rootfs_setup(lxc_conf, name, lxcpath);
+	if (ret < 0) {
+		ERROR("Failed to setup rootfs");
 		return -1;
 	}
 
 	if (handler->nsfd[LXC_NS_UTS] == -1) {
-		if (setup_utsname(lxc_conf->utsname)) {
+		ret = setup_utsname(lxc_conf->utsname);
+		if (ret < 0) {
 			ERROR("failed to setup the utsname for '%s'", name);
 			return -1;
 		}
 	}
 
-	if (lxc_setup_network_in_child_namespaces(lxc_conf, &lxc_conf->network)) {
-		ERROR("failed to setup the network for '%s'", name);
+	ret = lxc_setup_network_in_child_namespaces(lxc_conf, &lxc_conf->network);
+	if (ret < 0) {
+		ERROR("Failed to setup network");
 		return -1;
 	}
 
-	if (lxc_network_send_name_and_ifindex_to_parent(handler) < 0) {
-		ERROR("Failed to network device names and ifindices to parent");
+	ret = lxc_network_send_name_and_ifindex_to_parent(handler);
+	if (ret < 0) {
+		ERROR("Failed to send network device names and ifindices to parent");
 		return -1;
 	}
 
 	if (lxc_conf->autodev > 0) {
-		if (mount_autodev(name, &lxc_conf->rootfs, lxcpath)) {
-			ERROR("failed to mount /dev in the container");
+		ret = mount_autodev(name, &lxc_conf->rootfs, lxcpath);
+		if (ret < 0) {
+			ERROR("Failed to mount \"/dev\"");
 			return -1;
 		}
 	}
 
-	/* do automatic mounts (mainly /proc and /sys), but exclude
-	 * those that need to wait until other stuff has finished
+	/* Do automatic mounts (mainly /proc and /sys), but exclude those that
+	 * need to wait until other stuff has finished.
 	 */
-	if (lxc_mount_auto_mounts(lxc_conf, lxc_conf->auto_mounts & ~LXC_AUTO_CGROUP_MASK, handler) < 0) {
-		ERROR("failed to setup the automatic mounts for '%s'", name);
+	ret = lxc_mount_auto_mounts(lxc_conf, lxc_conf->auto_mounts & ~LXC_AUTO_CGROUP_MASK, handler);
+	if (ret < 0) {
+		ERROR("Failed to setup first automatic mounts");
 		return -1;
 	}
 
-	if (setup_mount(lxc_conf, &lxc_conf->rootfs, lxc_conf->fstab, name, lxcpath)) {
-		ERROR("failed to setup the mounts for '%s'", name);
+	ret = setup_mount(lxc_conf, &lxc_conf->rootfs, lxc_conf->fstab, name, lxcpath);
+	if (ret < 0) {
+		ERROR("Failed to setup mounts");
 		return -1;
 	}
 
@@ -3336,38 +3349,51 @@ int lxc_setup(struct lxc_handler *handler)
 	if (!verify_start_hooks(lxc_conf))
 		return -1;
 
-	if (lxc_conf->is_execute)
-		lxc_execute_bind_init(lxc_conf);
+	if (lxc_conf->is_execute) {
+		ret = lxc_execute_bind_init(lxc_conf);
+		if (ret < 0) {
+			ERROR("Failed to bind-mount the lxc init system");
+			return -1;
+		}
+	}
 
-	/* now mount only cgroup, if wanted;
-	 * before, /sys could not have been mounted
-	 * (is either mounted automatically or via fstab entries)
+	/* Now mount only cgroups, if wanted. Before, /sys could not have been
+	 * mounted. It is guaranteed to be mounted now either through
+	 * automatically or via fstab entries.
 	 */
-	if (lxc_mount_auto_mounts(lxc_conf, lxc_conf->auto_mounts & LXC_AUTO_CGROUP_MASK, handler) < 0) {
-		ERROR("failed to setup the automatic mounts for '%s'", name);
+	ret = lxc_mount_auto_mounts(lxc_conf, lxc_conf->auto_mounts & LXC_AUTO_CGROUP_MASK, handler);
+	if (ret < 0) {
+		ERROR("Failed to setup remaining automatic mounts");
 		return -1;
 	}
 
+	ret = run_lxc_hooks(name, "mount", lxc_conf, NULL);
 	if (run_lxc_hooks(name, "mount", lxc_conf, NULL)) {
-		ERROR("failed to run mount hooks for container '%s'.", name);
+		ERROR("Failed to run mount hooks");
 		return -1;
 	}
 
 	if (lxc_conf->autodev > 0) {
-		if (run_lxc_hooks(name, "autodev", lxc_conf, NULL)) {
-			ERROR("failed to run autodev hooks for container '%s'.", name);
+		ret = run_lxc_hooks(name, "autodev", lxc_conf, NULL);
+		if (ret < 0) {
+			ERROR("Failed to run autodev hooks");
 			return -1;
 		}
 
-		if (lxc_fill_autodev(&lxc_conf->rootfs)) {
-			ERROR("failed to populate /dev in the container");
+		ret = lxc_fill_autodev(&lxc_conf->rootfs);
+		if (ret < 0) {
+			ERROR("Failed to populate \"/dev\"");
 			return -1;
 		}
 	}
 
-	if (!lxc_list_empty(&lxc_conf->mount_list) && setup_mount_entries(lxc_conf, &lxc_conf->rootfs, &lxc_conf->mount_list, name, lxcpath)) {
-		ERROR("failed to setup the mount entries for '%s'", name);
-		return -1;
+	if (!lxc_list_empty(&lxc_conf->mount_list)) {
+		ret = setup_mount_entries(lxc_conf, &lxc_conf->rootfs,
+					  &lxc_conf->mount_list, name, lxcpath);
+		if (ret < 0) {
+			ERROR("Failed to setup mount entries");
+			return -1;
+		}
 	}
 
 	ret = lxc_setup_console(&lxc_conf->rootfs, &lxc_conf->console,
@@ -3379,23 +3405,25 @@ int lxc_setup(struct lxc_handler *handler)
 
 	ret = lxc_setup_dev_symlinks(&lxc_conf->rootfs);
 	if (ret < 0) {
-		ERROR("Failed to setup /dev symlinks");
+		ERROR("Failed to setup \"/dev\" symlinks");
 		return -1;
 	}
 
-	/* mount /proc if it's not already there */
-	if (lxc_create_tmp_proc_mount(lxc_conf) < 0) {
-		ERROR("failed to LSM mount proc for '%s'", name);
+	ret = lxc_create_tmp_proc_mount(lxc_conf);
+	if (ret < 0) {
+		ERROR("Failed to \"/proc\" LSMs");
 		return -1;
 	}
 
-	if (setup_pivot_root(&lxc_conf->rootfs)) {
-		ERROR("failed to set rootfs for '%s'", name);
+	ret = setup_pivot_root(&lxc_conf->rootfs);
+	if (ret < 0) {
+		ERROR("Failed to pivot root into rootfs");
 		return -1;
 	}
 
-	if (lxc_setup_devpts(lxc_conf)) {
-		ERROR("failed to setup the new pts instance");
+	ret = lxc_setup_devpts(lxc_conf);
+	if (ret < 0) {
+		ERROR("Failed to setup new devpts instance");
 		return -1;
 	}
 
@@ -3403,35 +3431,42 @@ int lxc_setup(struct lxc_handler *handler)
 	if (ret < 0)
 		return -1;
 
-	if (setup_personality(lxc_conf->personality)) {
-		ERROR("failed to setup personality");
+	ret = setup_personality(lxc_conf->personality);
+	if (ret < 0) {
+		ERROR("Failed to set personality");
 		return -1;
 	}
 
-	/* set sysctl value to a path under /proc/sys as determined from the key.
-	 * For e.g. net.ipv4.ip_forward translated to /proc/sys/net/ipv4/ip_forward.
+	/* Set sysctl value to a path under /proc/sys as determined from the
+	 * key. For e.g. net.ipv4.ip_forward translated to
+	 * /proc/sys/net/ipv4/ip_forward.
 	 */
 	if (!lxc_list_empty(&lxc_conf->sysctls)) {
 		ret = setup_sysctl_parameters(&lxc_conf->sysctls);
-		if (ret < 0)
+		if (ret < 0) {
+			ERROR("Failed to setup sysctl parameters");
 			return -1;
+		}
 	}
 
 	if (!lxc_list_empty(&lxc_conf->keepcaps)) {
 		if (!lxc_list_empty(&lxc_conf->caps)) {
-			ERROR("Container requests lxc.cap.drop and lxc.cap.keep: either use lxc.cap.drop or lxc.cap.keep, not both.");
+			ERROR("Container requests lxc.cap.drop and "
+			      "lxc.cap.keep: either use lxc.cap.drop or "
+			      "lxc.cap.keep, not both");
 			return -1;
 		}
+
 		if (dropcaps_except(&lxc_conf->keepcaps)) {
-			ERROR("failed to keep requested caps");
+			ERROR("Failed to keep capabilities");
 			return -1;
 		}
 	} else if (setup_caps(&lxc_conf->caps)) {
-		ERROR("failed to drop capabilities");
+		ERROR("Failed to drop capabilities");
 		return -1;
 	}
 
-	NOTICE("Container \"%s\" is set up", name);
+	NOTICE("The container \"%s\" is set up", name);
 
 	return 0;
 }

src/lxc/conf.h

@@ -233,9 +233,9 @@ enum {
 	 * variants, which is safe. */
 	LXC_AUTO_CGROUP_NOSPEC        = 0x0B0,   /* /sys/fs/cgroup (partial mount, r/w or mixed, depending on caps) */
 	LXC_AUTO_CGROUP_FULL_NOSPEC   = 0x0E0,   /* /sys/fs/cgroup (full mount, r/w or mixed, depending on caps) */
-	LXC_AUTO_CGROUP_MASK          = 0x0F0,
-
-	LXC_AUTO_ALL_MASK             = 0x0FF,   /* all known settings */
+	LXC_AUTO_CGROUP_FORCE         = 0x100,   /* mount cgroups even when cgroup namespaces are supported */
+	LXC_AUTO_CGROUP_MASK          = 0x1F0,   /* all known cgroup options, doe not contain LXC_AUTO_CGROUP_FORCE */
+	LXC_AUTO_ALL_MASK             = 0x1FF,   /* all known settings */
 };
 
 /*

src/lxc/confile.c

@@ -1706,26 +1706,30 @@ static int set_config_mount_auto(const char *key, const char *value,
 		int mask;
 		int flag;
 	} allowed_auto_mounts[] = {
-	    { "proc",              LXC_AUTO_PROC_MASK,   LXC_AUTO_PROC_MIXED         },
-	    { "proc:mixed",        LXC_AUTO_PROC_MASK,   LXC_AUTO_PROC_MIXED         },
-	    { "proc:rw",           LXC_AUTO_PROC_MASK,   LXC_AUTO_PROC_RW            },
-	    { "sys",               LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_MIXED          },
-	    { "sys:ro",            LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_RO             },
-	    { "sys:mixed",         LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_MIXED          },
-	    { "sys:rw",            LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_RW             },
-	    { "cgroup",            LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_NOSPEC      },
-	    { "cgroup:mixed",      LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_MIXED       },
-	    { "cgroup:ro",         LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_RO          },
-	    { "cgroup:rw",         LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_RW          },
-	    { "cgroup-full",       LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_NOSPEC },
-	    { "cgroup-full:mixed", LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_MIXED  },
-	    { "cgroup-full:ro",    LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_RO     },
-	    { "cgroup-full:rw",    LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_RW     },
+	    { "proc",                    LXC_AUTO_PROC_MASK,   LXC_AUTO_PROC_MIXED                            },
+	    { "proc:mixed",              LXC_AUTO_PROC_MASK,   LXC_AUTO_PROC_MIXED                            },
+	    { "proc:rw",                 LXC_AUTO_PROC_MASK,   LXC_AUTO_PROC_RW                               },
+	    { "sys",                     LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_MIXED                             },
+	    { "sys:ro",                  LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_RO                                },
+	    { "sys:mixed",               LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_MIXED                             },
+	    { "sys:rw",                  LXC_AUTO_SYS_MASK,    LXC_AUTO_SYS_RW                                },
+	    { "cgroup",                  LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_NOSPEC                         },
+	    { "cgroup:mixed",            LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_MIXED                          },
+	    { "cgroup:ro",               LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_RO                             },
+	    { "cgroup:rw",               LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_RW                             },
+	    { "cgroup:force",            LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_NOSPEC | LXC_AUTO_CGROUP_FORCE },
+	    { "cgroup:mixed:force",      LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_MIXED | LXC_AUTO_CGROUP_FORCE  },
+	    { "cgroup:ro:force",         LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_RO | LXC_AUTO_CGROUP_FORCE     },
+	    { "cgroup:rw:force",         LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_RW | LXC_AUTO_CGROUP_FORCE     },
+	    { "cgroup-full",             LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_NOSPEC                    },
+	    { "cgroup-full:mixed",       LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_MIXED                     },
+	    { "cgroup-full:ro",          LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_RO                        },
+	    { "cgroup-full:rw",          LXC_AUTO_CGROUP_MASK, LXC_AUTO_CGROUP_FULL_RW                        },
 	    /* For adding anything that is just a single on/off, but has no
-	     * options: keep mask and flag identical and just define the enum
-	     * value as an unused bit so far
+	    *  options: keep mask and flag identical and just define the enum
+	    *  value as an unused bit so far
 	     */
-	    { NULL,                0,                    0                           }
+	    { NULL,                      0,                    0                                              }
 	};
 
 	if (lxc_config_value_empty(value)) {

src/lxc/utils.c

@@ -509,10 +509,14 @@ struct lxc_popen_FILE *lxc_popen(const char *command)
 	fp = malloc(sizeof(*fp));
 	if (!fp)
 		goto on_error;
+	memset(fp, 0, sizeof(*fp));
 
 	fp->child_pid = child_pid;
 	fp->pipe = pipe_fds[0];
 
+	/* From now on, closing fp->f will also close fp->pipe. So only ever
+	 * call fclose(fp->f).
+	 */
 	fp->f = fdopen(pipe_fds[0], "r");
 	if (!fp->f)
 		goto on_error;
@@ -520,15 +524,22 @@ struct lxc_popen_FILE *lxc_popen(const char *command)
 	return fp;
 
 on_error:
-	if (fp)
-		free(fp);
-
-	if (pipe_fds[0] >= 0)
+	/* We can only close pipe_fds[0] if fdopen() didn't succeed or wasn't
+	 * called yet. Otherwise the fd belongs to the file opened by fdopen()
+	 * since it isn't dup()ed.
+	 */
+	if (fp && !fp->f && pipe_fds[0] >= 0)
 		close(pipe_fds[0]);
 
 	if (pipe_fds[1] >= 0)
 		close(pipe_fds[1]);
 
+	if (fp && fp->f)
+		fclose(fp->f);
+
+	if (fp)
+		free(fp);
+
 	return NULL;
 }
 
@@ -544,7 +555,6 @@ int lxc_pclose(struct lxc_popen_FILE *fp)
 		wait_pid = waitpid(fp->child_pid, &wstatus, 0);
 	} while (wait_pid < 0 && errno == EINTR);
 
-	close(fp->pipe);
 	fclose(fp->f);
 	free(fp);
 

src/tests/Makefile.am

@@ -79,7 +79,6 @@ bin_SCRIPTS += \
 	lxc-test-checkpoint-restore \
 	lxc-test-snapdeps \
 	lxc-test-symlink \
-	lxc-test-ubuntu \
 	lxc-test-unpriv \
 	lxc-test-usernic
 endif
@@ -115,7 +114,6 @@ EXTRA_DIST = \
 	lxc-test-no-new-privs \
 	lxc-test-snapdeps \
 	lxc-test-symlink \
-	lxc-test-ubuntu \
 	lxc-test-unpriv \
 	lxc-test-utils.c \
 	may_control.c \

src/tests/lxc-test-ubuntu

-#!/bin/sh
-
-# lxc-test-ubuntu: some tests of ubuntu-specific features of lxc.
-# Some features of lxc - networking and LSM configuration for instance -
-# are generally configured by the distro packages.  This program
-# tests the Ubuntu configuration.
-
-# These require the ubuntu lxc package to be installed.
-
-# General lxc functionality testing does not belong here.
-
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU Lesser General Public
-# License as published by the Free Software Foundation; either
-# version 2.1 of the License, or (at your option) any later version.
-
-# This library is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# Lesser General Public License for more details.
-
-# You should have received a copy of the GNU Lesser General Public
-# License along with this library; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-
-set -e
-
-FAIL() {
-	echo -n "Failed " >&2
-	echo "$*" >&2
-	exit 1
-}
-
-# Only run on a normally configured ubuntu lxc system
-if [ ! -d /sys/class/net/lxcbr0 ]; then
-	echo "lxcbr0 is not configured."
-	exit 1
-fi
-if [ "$(id -u)" != "0" ]; then
-	echo "ERROR: Must run as root."
-	exit 1
-fi
-
-for template in ubuntu ubuntu-cloud; do
-	# need a different name for each container so dnsmasq doesn't
-	# mess us up with its caching
-	if which uuidgen 2>&1 > /dev/null; then
-		name=$(uuidgen)
-	else
-		name=lxc-test-$template
-	fi
-
-	lxc-create -t $template -n $name || FAIL "creating $template container"
-	lxc-start -n $name -d || FAIL "starting $template container"
-	lxc-wait -n $name -s RUNNING || FAIL "waiting for $template container to run"
-
-	for tries in `seq 1 20`; do
-		lxcip=$(lxc-info -i -n $name -H | head -n 1)
-		[ -z "$lxcip" ] || break
-		sleep 1
-	done
-	[ -n "$lxcip" ] || FAIL "to start networking in $template container"
-
-	if echo "${lxcip}" | grep -q ":"; then
-		ping6 -c 1 $lxcip || FAIL "to ping $template container"
-	else
-		ping -c 1 $lxcip || FAIL "to ping $template container"
-	fi
-
-	# Check apparmor
-	lxcpid=`lxc-info -n $name -p -H`
-	aa=`cat /proc/$lxcpid/attr/current`
-	if [ "$aa" != "lxc-container-default-with-nesting (enforce)" -a \
-			"$aa" != "lxc-container-default-cgns (enforce)" -a \
-			"$aa" != "lxc-container-default (enforce)" ]; then
-		FAIL " to correctly set apparmor profile (profile is \"$aa\")"
-	fi
-	lxc-stop -n $name -k
-	lxc-destroy -n $name
-done
-
-exit 0