[lxc-devel] [lxc/master] cleanup attach

brauner on Github lxc-bot at linuxcontainers.org
Thu Nov 24 07:22:28 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20161124/e7233c30/attachment.bin>
-------------- next part --------------
From 82b1f317c8fbf9b81cb68a2c9be8ff36260f8ebc Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 24 Nov 2016 07:58:01 +0100
Subject: [PATCH 1/2] attach: simplify lsm_openat()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/attach.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 2f095b4..5e89e7d 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -85,12 +85,13 @@
 
 lxc_log_define(lxc_attach, lxc);
 
+/* /proc/pid-to-str/current\0 = (5 + 21 + 7 + 1) */
+#define __LSMATTRLEN (5 + 21 + 7 + 1)
 static int lsm_openat(int procfd, pid_t pid, int on_exec)
 {
 	int ret = -1;
 	int labelfd = -1;
-	const char* name;
-#define __LSMATTRLEN /* /proc */ (5 + /* /pid-to-str */ 21 + /* /current */ 7 + /* \0 */ 1)
+	const char *name;
 	char path[__LSMATTRLEN];
 
 	name = lsm_name();
@@ -105,20 +106,16 @@ static int lsm_openat(int procfd, pid_t pid, int on_exec)
 	if (strcmp(name, "AppArmor") == 0)
 		on_exec = 0;
 
-	if (on_exec) {
+	if (on_exec)
 		ret = snprintf(path, __LSMATTRLEN, "%d/attr/exec", pid);
-		if (ret < 0 || ret >= __LSMATTRLEN)
-			return -1;
-		labelfd = openat(procfd, path, O_RDWR);
-	} else {
+	else
 		ret = snprintf(path, __LSMATTRLEN, "%d/attr/current", pid);
-		if (ret < 0 || ret >= __LSMATTRLEN)
-			return -1;
-		labelfd = openat(procfd, path, O_RDWR);
-	}
+	if (ret < 0 || ret >= __LSMATTRLEN)
+		return -1;
 
+	labelfd = openat(procfd, path, O_RDWR);
 	if (labelfd < 0) {
-		SYSERROR("Unable to open LSM label");
+		SYSERROR("Unable to open file descriptor to set LSM label.");
 		return -1;
 	}
 

From b8b9a71a0173a84c8b26662575854fdbbe9f74f0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 24 Nov 2016 08:16:59 +0100
Subject: [PATCH 2/2] attach: non-functional changes

- improve logging
- simplify functions

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/attach.c | 241 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 121 insertions(+), 120 deletions(-)

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 5e89e7d..5cf4bbb 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -145,13 +145,13 @@ static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
 
 		command = malloc(strlen(lsm_label) + strlen("changeprofile ") + 1);
 		if (!command) {
-			SYSERROR("Failed to write apparmor profile");
+			SYSERROR("Failed to write apparmor profile.");
 			goto out;
 		}
 
 		size = sprintf(command, "changeprofile %s", lsm_label);
 		if (size < 0) {
-			SYSERROR("Failed to write apparmor profile");
+			SYSERROR("Failed to write apparmor profile.");
 			goto out;
 		}
 
@@ -162,12 +162,12 @@ static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
 		INFO("Set LSM label to: %s.", command);
 	} else if (strcmp(name, "SELinux") == 0) {
 		if (write(lsm_labelfd, lsm_label, strlen(lsm_label) + 1) < 0) {
-			SYSERROR("Unable to set LSM label");
+			SYSERROR("Unable to set LSM label: %s.", lsm_label);
 			goto out;
 		}
 		INFO("Set LSM label to: %s.", lsm_label);
 	} else {
-		ERROR("Unable to restore label for unknown LSM: %s", name);
+		ERROR("Unable to restore label for unknown LSM: %s.", name);
 		goto out;
 	}
 	fret = 0;
@@ -181,34 +181,40 @@ static int lsm_set_label_at(int lsm_labelfd, int on_exec, char *lsm_label)
 	return fret;
 }
 
+/* /proc/pid-to-str/status\0 = (5 + 21 + 7 + 1) */
+#define __PROC_STATUS_LEN (5 + 21 + 7 + 1)
 static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 {
-	struct lxc_proc_context_info *info = calloc(1, sizeof(*info));
 	FILE *proc_file;
-	char proc_fn[MAXPATHLEN];
+	char proc_fn[__PROC_STATUS_LEN];
+	bool found;
+	int ret;
 	char *line = NULL;
 	size_t line_bufsz = 0;
-	int ret, found;
-
-	if (!info) {
-		SYSERROR("Could not allocate memory.");
-		return NULL;
-	}
+	struct lxc_proc_context_info *info = NULL;
 
-	/* read capabilities */
-	snprintf(proc_fn, MAXPATHLEN, "/proc/%d/status", pid);
+	/* Read capabilities. */
+	ret = snprintf(proc_fn, __PROC_STATUS_LEN, "/proc/%d/status", pid);
+	if (ret < 0 || ret >= __PROC_STATUS_LEN)
+		goto on_error;
 
 	proc_file = fopen(proc_fn, "r");
 	if (!proc_file) {
-		SYSERROR("Could not open %s", proc_fn);
-		goto out_error;
+		SYSERROR("Could not open %s.", proc_fn);
+		goto on_error;
+	}
+
+	info = calloc(1, sizeof(*info));
+	if (!info) {
+		SYSERROR("Could not allocate memory.");
+		return NULL;
 	}
 
-	found = 0;
+	found = false;
 	while (getline(&line, &line_bufsz, proc_file) != -1) {
 		ret = sscanf(line, "CapBnd: %llx", &info->capability_mask);
-		if (ret != EOF && ret > 0) {
-			found = 1;
+		if (ret != EOF && ret == 1) {
+			found = true;
 			break;
 		}
 	}
@@ -217,16 +223,16 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 	fclose(proc_file);
 
 	if (!found) {
-		SYSERROR("Could not read capability bounding set from %s", proc_fn);
+		SYSERROR("Could not read capability bounding set from %s.", proc_fn);
 		errno = ENOENT;
-		goto out_error;
+		goto on_error;
 	}
 
 	info->lsm_label = lsm_process_label_get(pid);
 
 	return info;
 
-out_error:
+on_error:
 	free(info);
 	return NULL;
 }
@@ -246,7 +252,7 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 
 
 	if (access("/proc/self/ns", X_OK)) {
-		ERROR("Does this kernel version support 'attach' ?");
+		ERROR("Does this kernel version support namespaces?");
 		return -1;
 	}
 
@@ -270,7 +276,7 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 				close(fd[j]);
 
 			errno = saved_errno;
-			SYSERROR("failed to open namespace: '%s'.", ns_info[i].proc_name);
+			SYSERROR("Failed to open namespace: \"%s\".", ns_info[i].proc_name);
 			return -1;
 		}
 	}
@@ -304,13 +310,13 @@ static int lxc_attach_remount_sys_proc(void)
 
 	ret = unshare(CLONE_NEWNS);
 	if (ret < 0) {
-		SYSERROR("failed to unshare mount namespace");
+		SYSERROR("Failed to unshare mount namespace.");
 		return -1;
 	}
 
 	if (detect_shared_rootfs()) {
 		if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL)) {
-			SYSERROR("Failed to make / rslave");
+			SYSERROR("Failed to make / rslave.");
 			ERROR("Continuing...");
 		}
 	}
@@ -318,29 +324,28 @@ static int lxc_attach_remount_sys_proc(void)
 	/* assume /proc is always mounted, so remount it */
 	ret = umount2("/proc", MNT_DETACH);
 	if (ret < 0) {
-		SYSERROR("failed to unmount /proc");
+		SYSERROR("Failed to unmount /proc.");
 		return -1;
 	}
 
 	ret = mount("none", "/proc", "proc", 0, NULL);
 	if (ret < 0) {
-		SYSERROR("failed to remount /proc");
+		SYSERROR("Failed to remount /proc.");
 		return -1;
 	}
 
-	/* try to umount /sys - if it's not a mount point,
-	 * we'll get EINVAL, then we ignore it because it
-	 * may not have been mounted in the first place
+	/* Try to umount /sys. If it's not a mount point, we'll get EINVAL, then
+	 * we ignore it because it may not have been mounted in the first place.
 	 */
 	ret = umount2("/sys", MNT_DETACH);
 	if (ret < 0 && errno != EINVAL) {
-		SYSERROR("failed to unmount /sys");
+		SYSERROR("Failed to unmount /sys.");
 		return -1;
 	} else if (ret == 0) {
 		/* remount it */
 		ret = mount("none", "/sys", "sysfs", 0, NULL);
 		if (ret < 0) {
-			SYSERROR("failed to remount /sys");
+			SYSERROR("Failed to remount /sys.");
 			return -1;
 		}
 	}
@@ -358,7 +363,7 @@ static int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx)
 			continue;
 
 		if (prctl(PR_CAPBSET_DROP, cap, 0, 0, 0)) {
-			SYSERROR("failed to remove capability id %d", cap);
+			SYSERROR("Failed to remove capability id %d.", cap);
 			return -1;
 		}
 	}
@@ -379,8 +384,8 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 
 			extra_keep_store = calloc(count, sizeof(char *));
 			if (!extra_keep_store) {
-				SYSERROR("failed to allocate memory for storing current "
-				         "environment variable values that will be kept");
+				SYSERROR("Failed to allocate memory for storing current "
+				         "environment variable values that will be kept.");
 				return -1;
 			}
 			for (i = 0; i < count; i++) {
@@ -388,8 +393,8 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 				if (v) {
 					extra_keep_store[i] = strdup(v);
 					if (!extra_keep_store[i]) {
-						SYSERROR("failed to allocate memory for storing current "
-						         "environment variable values that will be kept");
+						SYSERROR("Failed to allocate memory for storing current "
+						         "environment variable values that will be kept.");
 						while (i > 0)
 							free(extra_keep_store[--i]);
 						free(extra_keep_store);
@@ -405,7 +410,7 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 
 		if (clearenv()) {
 			char **p;
-			SYSERROR("failed to clear environment");
+			SYSERROR("Failed to clear environment.");
 			if (extra_keep_store) {
 				for (p = extra_keep_store; *p; p++)
 					free(*p);
@@ -419,7 +424,7 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 			for (i = 0; extra_keep[i]; i++) {
 				if (extra_keep_store[i]) {
 					if (setenv(extra_keep[i], extra_keep_store[i], 1) < 0)
-						SYSERROR("Unable to set environment variable");
+						SYSERROR("Unable to set environment variable.");
 				}
 				free(extra_keep_store[i]);
 			}
@@ -436,7 +441,7 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 	}
 
 	if (putenv("container=lxc")) {
-		SYSERROR("failed to set environment variable");
+		SYSERROR("Failed to set environment variable.");
 		return -1;
 	}
 
@@ -450,8 +455,8 @@ static int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char*
 			/* we just assume the user knows what they
 			 * are doing, so we don't do any checks */
 			if (!p) {
-				SYSERROR("failed to allocate memory for additional environment "
-				         "variables");
+				SYSERROR("Failed to allocate memory for additional environment "
+				         "variables.");
 				return -1;
 			}
 			putenv(p);
@@ -617,16 +622,16 @@ static char *lxc_attach_getpwshell(uid_t uid)
 static void lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid)
 {
 	FILE *proc_file;
-	char proc_fn[MAXPATHLEN];
+	char proc_fn[__PROC_STATUS_LEN];
+	int ret;
 	char *line = NULL;
 	size_t line_bufsz = 0;
-	int ret;
 	long value = -1;
 	uid_t uid = (uid_t)-1;
 	gid_t gid = (gid_t)-1;
 
 	/* read capabilities */
-	snprintf(proc_fn, MAXPATHLEN, "/proc/%d/status", 1);
+	snprintf(proc_fn, __PROC_STATUS_LEN, "/proc/%d/status", 1);
 
 	proc_file = fopen(proc_fn, "r");
 	if (!proc_file)
@@ -637,11 +642,11 @@ static void lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid)
 		 * we only care about real uid
 		 */
 		ret = sscanf(line, "Uid: %ld", &value);
-		if (ret != EOF && ret > 0) {
+		if (ret != EOF && ret == 1) {
 			uid = (uid_t) value;
 		} else {
 			ret = sscanf(line, "Gid: %ld", &value);
-			if (ret != EOF && ret > 0)
+			if (ret != EOF && ret == 1)
 				gid = (gid_t) value;
 		}
 		if (uid != (uid_t)-1 && gid != (gid_t)-1)
@@ -709,7 +714,7 @@ static bool fetch_seccomp(struct lxc_container *c,
 
 	/* Attempt to parse the resulting config */
 	if (lxc_read_seccomp_config(c->lxc_conf) < 0) {
-		ERROR("Error reading seccomp policy");
+		ERROR("Error reading seccomp policy.");
 		return false;
 	}
 
@@ -771,19 +776,20 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 
 	init_pid = lxc_cmd_get_init_pid(name, lxcpath);
 	if (init_pid < 0) {
-		ERROR("failed to get the init pid");
+		ERROR("Failed to get init pid.");
 		return -1;
 	}
 
 	init_ctx = lxc_proc_get_context_info(init_pid);
 	if (!init_ctx) {
-		ERROR("failed to get context of the init process, pid = %ld", (long)init_pid);
+		ERROR("Failed to get context of init process: %ld.",
+		      (long)init_pid);
 		return -1;
 	}
 
 	personality = get_personality(name, lxcpath);
 	if (init_ctx->personality < 0) {
-		ERROR("Failed to get personality of the container");
+		ERROR("Failed to get personality of the container.");
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
@@ -794,7 +800,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		return -1;
 
 	if (!fetch_seccomp(init_ctx->container, options))
-		WARN("Failed to get seccomp policy");
+		WARN("Failed to get seccomp policy.");
 
 	if (!no_new_privs(init_ctx->container, options))
 		WARN("Could not determine whether PR_SET_NO_NEW_PRIVS is set.");
@@ -808,27 +814,28 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		options->namespaces = lxc_cmd_get_clone_flags(name, lxcpath);
 		/* call failed */
 		if (options->namespaces == -1) {
-			ERROR("failed to automatically determine the "
-			      "namespaces which the container unshared");
+			ERROR("Failed to automatically determine the "
+			      "namespaces which the container uses.");
 			free(cwd);
 			lxc_proc_put_context_info(init_ctx);
 			return -1;
 		}
 	}
 
-	/* create a socket pair for IPC communication; set SOCK_CLOEXEC in order
-	 * to make sure we don't irritate other threads that want to fork+exec away
+	/* Create a socket pair for IPC communication; set SOCK_CLOEXEC in order
+	 * to make sure we don't irritate other threads that want to fork+exec
+	 * away
 	 *
 	 * IMPORTANT: if the initial process is multithreaded and another call
 	 * just fork()s away without exec'ing directly after, the socket fd will
 	 * exist in the forked process from the other thread and any close() in
-	 * our own child process will not really cause the socket to close properly,
-	 * potentiall causing the parent to hang.
+	 * our own child process will not really cause the socket to close
+	 * properly, potentiall causing the parent to hang.
 	 *
 	 * For this reason, while IPC is still active, we have to use shutdown()
-	 * if the child exits prematurely in order to signal that the socket
-	 * is closed and cannot assume that the child exiting will automatically
-	 * do that.
+	 * if the child exits prematurely in order to signal that the socket is
+	 * closed and cannot assume that the child exiting will automatically do
+	 * that.
 	 *
 	 * IPC mechanism: (X is receiver)
 	 *   initial process        intermediate          attached
@@ -850,28 +857,26 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 	 */
 	ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
 	if (ret < 0) {
-		SYSERROR("could not set up required IPC mechanism for attaching");
+		SYSERROR("Could not set up required IPC mechanism for attaching.");
 		free(cwd);
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
 
-	/* create intermediate subprocess, three reasons:
-	 *       1. runs all pthread_atfork handlers and the
-	 *          child will no longer be threaded
-	 *          (we can't properly setns() in a threaded process)
-	 *       2. we can't setns() in the child itself, since
-	 *          we want to make sure we are properly attached to
-	 *          the pidns
-	 *       3. also, the initial thread has to put the attached
-	 *          process into the cgroup, which we can only do if
-	 *          we didn't already setns() (otherwise, user
-	 *          namespaces will hate us)
+	/* Create intermediate subprocess, three reasons:
+	 *       1. Runs all pthread_atfork handlers and the child will no
+	 *          longer be threaded (we can't properly setns() in a threaded
+	 *          process).
+	 *       2. We can't setns() in the child itself, since we want to make
+	 *          sure we are properly attached to the pidns.
+	 *       3. Also, the initial thread has to put the attached process
+	 *          into the cgroup, which we can only do if we didn't already
+	 *          setns() (otherwise, user namespaces will hate us).
 	 */
 	pid = fork();
 
 	if (pid < 0) {
-		SYSERROR("failed to create first subprocess");
+		SYSERROR("Failed to create first subprocess.");
 		free(cwd);
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
@@ -890,7 +895,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		/* attach to cgroup, if requested */
 		if (options->attach_flags & LXC_ATTACH_MOVE_TO_CGROUP) {
 			if (!cgroup_attach(name, lxcpath, pid))
-				goto cleanup_error;
+				goto on_error;
 		}
 
 		/* Open /proc before setns() to the containers namespace so we
@@ -899,23 +904,24 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		procfd = open("/proc", O_DIRECTORY | O_RDONLY | O_CLOEXEC);
 		if (procfd < 0) {
 			SYSERROR("Unable to open /proc.");
-			goto cleanup_error;
+			goto on_error;
 		}
 
 		/* Let the child process know to go ahead */
 		status = 0;
 		ret = lxc_write_nointr(ipc_sockets[0], &status, sizeof(status));
 		if (ret <= 0) {
-			ERROR("error using IPC to notify attached process for initialization (0)");
-			goto cleanup_error;
+			ERROR("Intended to send sequence number 0: %s.",
+			      strerror(errno));
+			goto on_error;
 		}
 
 		/* get pid from intermediate process */
 		ret = lxc_read_nointr_expect(ipc_sockets[0], &attached_pid, sizeof(attached_pid), NULL);
 		if (ret <= 0) {
 			if (ret != 0)
-				ERROR("error using IPC to receive pid of attached process");
-			goto cleanup_error;
+				ERROR("Expected to receive pid: %s.", strerror(errno));
+			goto on_error;
 		}
 
 		/* ignore SIGKILL (CTRL-C) and SIGQUIT (CTRL-\) - issue #313 */
@@ -927,7 +933,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		/* reap intermediate process */
 		ret = wait_for_pid(pid);
 		if (ret < 0)
-			goto cleanup_error;
+			goto on_error;
 
 		/* we will always have to reap the grandchild now */
 		to_cleanup_pid = attached_pid;
@@ -936,8 +942,8 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		status = 0;
 		ret = lxc_write_nointr(ipc_sockets[0], &status, sizeof(status));
 		if (ret <= 0) {
-			ERROR("error using IPC to notify attached process for initialization (0)");
-			goto cleanup_error;
+			ERROR("Intended to send sequence number 0: %s.", strerror(errno));
+			goto on_error;
 		}
 
 		/* wait for the attached process to finish initializing */
@@ -945,18 +951,16 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		ret = lxc_read_nointr_expect(ipc_sockets[0], &status, sizeof(status), &expected);
 		if (ret <= 0) {
 			if (ret != 0)
-				ERROR("error using IPC to receive notification "
-				      "from attached process (1)");
-			goto cleanup_error;
+				ERROR("Expected to receive sequence number 1: %s.", strerror(errno));
+			goto on_error;
 		}
 
 		/* tell attached process we're done */
 		status = 2;
 		ret = lxc_write_nointr(ipc_sockets[0], &status, sizeof(status));
 		if (ret <= 0) {
-			ERROR("Error using IPC to notify attached process for "
-			      "initialization (2): %s.", strerror(errno));
-			goto cleanup_error;
+			ERROR("Intended to send sequence number 2: %s.", strerror(errno));
+			goto on_error;
 		}
 
 		/* Wait for the (grand)child to tell us that it's ready to set
@@ -965,9 +969,9 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		expected = 3;
 		ret = lxc_read_nointr_expect(ipc_sockets[0], &status, sizeof(status), &expected);
 		if (ret <= 0) {
-			ERROR("Error using IPC for the child to tell us to open LSM fd (3): %s.",
+			ERROR("Expected to receive sequence number 3: %s.",
 			      strerror(errno));
-			goto cleanup_error;
+			goto on_error;
 		}
 
 		/* Open LSM fd and send it to child. */
@@ -977,14 +981,13 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 			/* Open fd for the LSM security module. */
 			labelfd = lsm_openat(procfd, attached_pid, on_exec);
 			if (labelfd < 0)
-				goto cleanup_error;
+				goto on_error;
 
 			/* Send child fd of the LSM security module to write to. */
 			ret = lxc_abstract_unix_send_fd(ipc_sockets[0], labelfd, NULL, 0);
 			if (ret <= 0) {
-				ERROR("Error using IPC to send child LSM fd (4): %s.",
-						strerror(errno));
-				goto cleanup_error;
+				ERROR("Intended to send file descriptor %d: %s.", labelfd, strerror(errno));
+				goto on_error;
 			}
 		}
 
@@ -1001,7 +1004,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		*attached_process = attached_pid;
 		return 0;
 
-	cleanup_error:
+	on_error:
 		/* first shut down the socket, then wait for the pid,
 		 * otherwise the pid we're waiting for may never exit
 		 */
@@ -1025,7 +1028,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 	status = -1;
 	ret = lxc_read_nointr_expect(ipc_sockets[1], &status, sizeof(status), &expected);
 	if (ret <= 0) {
-		ERROR("error communicating with child process");
+		ERROR("Expected to receive sequence number 0: %s.", strerror(errno));
 		shutdown(ipc_sockets[1], SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1038,7 +1041,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 	 */
 	ret = lxc_attach_to_ns(init_pid, options->namespaces);
 	if (ret < 0) {
-		ERROR("failed to enter the namespace");
+		ERROR("Failed to enter namespaces.");
 		shutdown(ipc_sockets[1], SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1050,7 +1053,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		new_cwd = cwd;
 	ret = chdir(new_cwd);
 	if (ret < 0)
-		WARN("could not change directory to '%s'", new_cwd);
+		WARN("Could not change directory to \"%s\".", new_cwd);
 	free(cwd);
 
 	/* now create the real child process */
@@ -1071,7 +1074,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 
 	/* shouldn't happen, clone() should always return positive pid */
 	if (pid <= 0) {
-		SYSERROR("failed to create subprocess");
+		SYSERROR("Failed to create subprocess.");
 		shutdown(ipc_sockets[1], SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1085,7 +1088,7 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		 * so the parent won't be able to reap it and the attached process
 		 * will remain a zombie
 		 */
-		ERROR("error using IPC to notify main process of pid of the attached process");
+		ERROR("Intended to send pid %d: %s.", pid, strerror(errno));
 		shutdown(ipc_sockets[1], SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1119,7 +1122,7 @@ static int attach_child_main(void* data)
 	status = -1;
 	ret = lxc_read_nointr_expect(ipc_socket, &status, sizeof(status), &expected);
 	if (ret <= 0) {
-		ERROR("Error using IPC to receive notification from initial process (0): %s.", strerror(errno));
+		ERROR("Expected to receive sequence number 0: %s.", strerror(errno));
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1147,7 +1150,7 @@ static int attach_child_main(void* data)
 	if (options->attach_flags & LXC_ATTACH_SET_PERSONALITY) {
 		ret = personality(new_personality);
 		if (ret < 0) {
-			SYSERROR("could not ensure correct architecture");
+			SYSERROR("Could not ensure correct architecture.");
 			shutdown(ipc_socket, SHUT_RDWR);
 			rexit(-1);
 		}
@@ -1157,7 +1160,7 @@ static int attach_child_main(void* data)
 	if (options->attach_flags & LXC_ATTACH_DROP_CAPABILITIES) {
 		ret = lxc_attach_drop_privs(init_ctx);
 		if (ret < 0) {
-			ERROR("could not drop privileges");
+			ERROR("Could not drop privileges.");
 			shutdown(ipc_socket, SHUT_RDWR);
 			rexit(-1);
 		}
@@ -1166,7 +1169,7 @@ static int attach_child_main(void* data)
 	/* always set the environment (specify (LXC_ATTACH_KEEP_ENV, NULL, NULL) if you want this to be a no-op) */
 	ret = lxc_attach_set_environment(options->env_policy, options->extra_env_vars, options->extra_keep_env);
 	if (ret < 0) {
-		ERROR("could not set initial environment for attached process");
+		ERROR("Could not set initial environment for attached process.");
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1188,13 +1191,13 @@ static int attach_child_main(void* data)
 	/* setup the control tty */
 	if (options->stdin_fd && isatty(options->stdin_fd)) {
 		if (setsid() < 0) {
-			SYSERROR("unable to setsid");
+			SYSERROR("Unable to setsid.");
 			shutdown(ipc_socket, SHUT_RDWR);
 			rexit(-1);
 		}
 
 		if (ioctl(options->stdin_fd, TIOCSCTTY, (char *)NULL) < 0) {
-			SYSERROR("unable to TIOCSTTY");
+			SYSERROR("Unable to set TIOCSTTY.");
 			shutdown(ipc_socket, SHUT_RDWR);
 			rexit(-1);
 		}
@@ -1203,13 +1206,13 @@ static int attach_child_main(void* data)
 	/* try to set the uid/gid combination */
 	if ((new_gid != 0 || options->namespaces & CLONE_NEWUSER)) {
 		if (setgid(new_gid) || setgroups(0, NULL)) {
-			SYSERROR("switching to container gid");
+			SYSERROR("Switching to container gid.");
 			shutdown(ipc_socket, SHUT_RDWR);
 			rexit(-1);
 		}
 	}
 	if ((new_uid != 0 || options->namespaces & CLONE_NEWUSER) && setuid(new_uid)) {
-		SYSERROR("switching to container uid");
+		SYSERROR("Switching to container uid.");
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1218,7 +1221,7 @@ static int attach_child_main(void* data)
 	status = 1;
 	ret = lxc_write_nointr(ipc_socket, &status, sizeof(status));
 	if (ret != sizeof(status)) {
-		ERROR("Error using IPC to notify initial process for initialization (1): %s.", strerror(errno));
+		ERROR("Intended to send sequence number 1: %s.", strerror(errno));
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1230,9 +1233,7 @@ static int attach_child_main(void* data)
 	status = -1;
 	ret = lxc_read_nointr_expect(ipc_socket, &status, sizeof(status), &expected);
 	if (ret <= 0) {
-		ERROR("Error using IPC to receive message from initial process "
-		      "that it is done pre-initializing (2): %s",
-		      strerror(errno));
+		ERROR("Expected to receive sequence number 2: %s", strerror(errno));
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1255,7 +1256,7 @@ static int attach_child_main(void* data)
 	status = 3;
 	ret = lxc_write_nointr(ipc_socket, &status, sizeof(status));
 	if (ret <= 0) {
-		ERROR("Error using IPC to tell parent to set up LSM labels (3): %s.", strerror(errno));
+		ERROR("Intended to send sequence number 3: %s.", strerror(errno));
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1265,7 +1266,7 @@ static int attach_child_main(void* data)
 		/* Receive fd for LSM security module. */
 		ret = lxc_abstract_unix_recv_fd(ipc_socket, &lsm_labelfd, NULL, 0);
 		if (ret <= 0) {
-			ERROR("Error using IPC for parent to tell us LSM label fd (4): %s.", strerror(errno));
+			ERROR("Expected to receive file descriptor: %s.", strerror(errno));
 			shutdown(ipc_socket, SHUT_RDWR);
 			rexit(-1);
 		}
@@ -1284,7 +1285,7 @@ static int attach_child_main(void* data)
 	if (init_ctx->container && init_ctx->container->lxc_conf &&
 	    init_ctx->container->lxc_conf->seccomp &&
 	    (lxc_seccomp_load(init_ctx->container->lxc_conf) != 0)) {
-		ERROR("Loading seccomp policy");
+		ERROR("Failed to load seccomp policy.");
 		shutdown(ipc_socket, SHUT_RDWR);
 		rexit(-1);
 	}
@@ -1326,7 +1327,7 @@ static int attach_child_main(void* data)
 			continue;
 		if (flags & FD_CLOEXEC)
 			if (fcntl(fd, F_SETFL, flags & ~FD_CLOEXEC) < 0)
-				SYSERROR("Unable to clear CLOEXEC from fd");
+				SYSERROR("Unable to clear FD_CLOEXEC from file descriptor.");
 	}
 
 	/* we're done, so we can now do whatever the user intended us to do */
@@ -1338,7 +1339,7 @@ int lxc_attach_run_command(void* payload)
 	lxc_attach_command_t* cmd = (lxc_attach_command_t*)payload;
 
 	execvp(cmd->program, cmd->argv);
-	SYSERROR("failed to exec '%s'", cmd->program);
+	SYSERROR("Failed to exec \"%s\".", cmd->program);
 	return -1;
 }
 
@@ -1373,6 +1374,6 @@ int lxc_attach_run_shell(void* payload)
 	 * we will fall back on /bin/sh as a default shell
 	 */
 	execlp("/bin/sh", "/bin/sh", (char *)NULL);
-	SYSERROR("failed to exec shell");
+	SYSERROR("Failed to exec shell.");
 	return -1;
 }


More information about the lxc-devel mailing list