Merge pull request #1255 from brauner/2016-10-27/cleanup_netdevs_lxd#2439_lxc#1253

src/lxc/attach.c

@@ -220,9 +220,8 @@ static void lxc_proc_put_context_info(struct lxc_proc_context_info *ctx)
 
 static int lxc_attach_to_ns(pid_t pid, int which)
 {
-	char path[MAXPATHLEN];
 	/* according to <http://article.gmane.org/gmane.linux.kernel.containers.lxc.devel/1429>,
-	 * the file for user namepsaces in /proc/$pid/ns will be called
+	 * the file for user namespaces in /proc/$pid/ns will be called
 	 * 'user' once the kernel supports it
 	 */
 	static char *ns[] = { "user", "mnt", "pid", "uts", "ipc", "net", "cgroup" };
@@ -235,8 +234,7 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 	int i, j, saved_errno;
 
 
-	snprintf(path, MAXPATHLEN, "/proc/%d/ns", pid);
-	if (access(path, X_OK)) {
+	if (access("/proc/self/ns", X_OK)) {
 		ERROR("Does this kernel version support 'attach' ?");
 		return -1;
 	}
@@ -250,8 +248,7 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 			continue;
 		}
 
-		snprintf(path, MAXPATHLEN, "/proc/%d/ns/%s", pid, ns[i]);
-		fd[i] = open(path, O_RDONLY | O_CLOEXEC);
+		fd[i] = lxc_preserve_ns(pid, ns[i]);
 		if (fd[i] < 0) {
 			saved_errno = errno;
 
@@ -262,7 +259,7 @@ static int lxc_attach_to_ns(pid_t pid, int which)
 				close(fd[j]);
 
 			errno = saved_errno;
-			SYSERROR("failed to open '%s'", path);
+			SYSERROR("failed to open namespace: '%s'.", ns[i]);
 			return -1;
 		}
 	}

src/lxc/conf.c

@@ -2399,24 +2399,20 @@ static int setup_network(struct lxc_list *network)
 /* try to move physical nics to the init netns */
 void lxc_restore_phys_nics_to_netns(int netnsfd, struct lxc_conf *conf)
 {
-	int i, ret, oldfd;
-	char path[MAXPATHLEN];
+	int i, oldfd;
 	char ifname[IFNAMSIZ];
 
 	if (netnsfd < 0 || conf->num_savednics == 0)
 		return;
 
-	INFO("running to reset %d nic names", conf->num_savednics);
+	INFO("Running to reset %d nic names.", conf->num_savednics);
 
-	ret = snprintf(path, MAXPATHLEN, "/proc/self/ns/net");
-	if (ret < 0 || ret >= MAXPATHLEN) {
-		WARN("Failed to open monitor netns fd");
-		return;
-	}
-	if ((oldfd = open(path, O_RDONLY)) < 0) {
-		SYSERROR("Failed to open monitor netns fd");
+	oldfd = lxc_preserve_ns(getpid(), "net");
+	if (oldfd < 0) {
+		SYSERROR("Failed to open monitor netns fd.");
 		return;
 	}
+
 	if (setns(netnsfd, 0) != 0) {
 		SYSERROR("Failed to enter container netns to reset nics");
 		close(oldfd);
@@ -2591,6 +2587,7 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 				      veth1, netdev->link, strerror(-err));
 			goto out_delete;
 		}
+		INFO("Attached '%s': to the bridge '%s': ", veth1, netdev->link);
 	}
 
 	err = lxc_netdev_up(veth1);
@@ -2883,36 +2880,83 @@ int lxc_create_network(struct lxc_handler *handler)
 	return 0;
 }
 
-void lxc_delete_network(struct lxc_handler *handler)
+bool lxc_delete_network(struct lxc_handler *handler)
 {
+	int ret;
 	struct lxc_list *network = &handler->conf->network;
 	struct lxc_list *iterator;
 	struct lxc_netdev *netdev;
+	bool deleted_all = true;
 
 	lxc_list_for_each(iterator, network) {
 		netdev = iterator->elem;
 
 		if (netdev->ifindex != 0 && netdev->type == LXC_NET_PHYS) {
 			if (lxc_netdev_rename_by_index(netdev->ifindex, netdev->link))
-				WARN("failed to rename to the initial name the " \
-				     "netdev '%s'", netdev->link);
+				WARN("Failed to rename interface with index %d "
+				     "to its initial name \"%s\".",
+				     netdev->ifindex, netdev->link);
 			continue;
 		}
 
 		if (netdev_deconf[netdev->type](handler, netdev)) {
-			WARN("failed to destroy netdev");
+			WARN("Failed to destroy netdev");
 		}
 
 		/* Recent kernel remove the virtual interfaces when the network
 		 * namespace is destroyed but in case we did not moved the
 		 * interface to the network namespace, we have to destroy it
 		 */
-		if (netdev->ifindex != 0 &&
-		    lxc_netdev_delete_by_index(netdev->ifindex))
-			WARN("failed to remove interface %d '%s'",
-				netdev->ifindex,
-				netdev->name ? netdev->name : "(null)");
+		if (netdev->ifindex != 0) {
+			ret = lxc_netdev_delete_by_index(netdev->ifindex);
+			if (-ret == ENODEV) {
+				INFO("Interface \"%s\" with index %d already "
+				     "deleted or existing in different network "
+				     "namespace.",
+				     netdev->name ? netdev->name : "(null)",
+				     netdev->ifindex);
+			} else if (ret < 0) {
+				deleted_all = false;
+				WARN("Failed to remove interface \"%s\" with "
+				     "index %d: %s.",
+				     netdev->name ? netdev->name : "(null)",
+				     netdev->ifindex, strerror(-ret));
+			} else {
+				INFO("Removed interface \"%s\" with index %d.",
+				     netdev->name ? netdev->name : "(null)",
+				     netdev->ifindex);
+			}
+		}
+
+		/* Explicitly delete host veth device to prevent lingering
+		 * devices. We had issues in LXD around this.
+		 */
+		if (netdev->type == LXC_NET_VETH) {
+			char *hostveth;
+			if (netdev->priv.veth_attr.pair) {
+				hostveth = netdev->priv.veth_attr.pair;
+				ret = lxc_netdev_delete_by_name(hostveth);
+				if (ret < 0) {
+					WARN("Failed to remove interface \"%s\" from host: %s.", hostveth, strerror(-ret));
+				} else {
+					INFO("Removed interface \"%s\" from host.", hostveth);
+					free(netdev->priv.veth_attr.pair);
+					netdev->priv.veth_attr.pair = NULL;
+				}
+			} else if (strlen(netdev->priv.veth_attr.veth1) > 0) {
+				hostveth = netdev->priv.veth_attr.veth1;
+				ret = lxc_netdev_delete_by_name(hostveth);
+				if (ret < 0) {
+					WARN("Failed to remove \"%s\" from host: %s.", hostveth, strerror(-ret));
+				} else {
+					INFO("Removed interface \"%s\" from host.", hostveth);
+					memset((void *)&netdev->priv.veth_attr.veth1, 0, sizeof(netdev->priv.veth_attr.veth1));
+				}
+			}
+		}
 	}
+
+	return deleted_all;
 }
 
 #define LXC_USERNIC_PATH LIBEXECDIR "/lxc/lxc-user-nic"
@@ -3050,7 +3094,7 @@ int lxc_assign_network(const char *lxcpath, char *lxcname,
 			return -1;
 		}
 
-		DEBUG("move '%s' to '%d'", netdev->name, pid);
+		DEBUG("move '%s'/'%s' to '%d': .", ifname, netdev->name, pid);
 	}
 
 	return 0;

src/lxc/conf.h

@@ -408,7 +408,7 @@ extern int pin_rootfs(const char *rootfs);
 
 extern int lxc_requests_empty_network(struct lxc_handler *handler);
 extern int lxc_create_network(struct lxc_handler *handler);
-extern void lxc_delete_network(struct lxc_handler *handler);
+extern bool lxc_delete_network(struct lxc_handler *handler);
 extern int lxc_assign_network(const char *lxcpath, char *lxcname,
 			      struct lxc_list *networks, pid_t pid);
 extern int lxc_map_ids(struct lxc_list *idmap, pid_t pid);

src/lxc/error.c

@@ -46,13 +46,12 @@ extern int  lxc_error_set_and_log(int pid, int status)
 	if (WIFEXITED(status)) {
 		ret = WEXITSTATUS(status);
 		if (ret)
-			INFO("child <%d> ended on error (%d)", pid, ret);
+			INFO("Child <%d> ended on error (%d).", pid, ret);
 	}
 
 	if (WIFSIGNALED(status)) {
 		int signal = WTERMSIG(status);
-
-		INFO("child <%d> ended on signal (%d)", pid, signal);
+		INFO("Child <%d> ended on signal (%d).", pid, signal);
 	}
 
 	return ret;

src/lxc/lxc_user_nic.c

@@ -673,25 +673,21 @@ again:
 
 static int rename_in_ns(int pid, char *oldname, char **newnamep)
 {
-	char nspath[MAXPATHLEN];
 	int fd = -1, ofd = -1, ret, ifindex = -1;
 	bool grab_newname = false;
 
-	ret = snprintf(nspath, MAXPATHLEN, "/proc/%d/ns/net", getpid());
-	if (ret < 0 || ret >= MAXPATHLEN)
-		return -1;
-	if ((ofd = open(nspath, O_RDONLY)) < 0) {
-		fprintf(stderr, "Opening %s\n", nspath);
+	ofd = lxc_preserve_ns(getpid(), "net");
+	if (ofd < 0) {
+		fprintf(stderr, "Failed opening network namespace path for '%d'.", getpid());
 		return -1;
 	}
-	ret = snprintf(nspath, MAXPATHLEN, "/proc/%d/ns/net", pid);
-	if (ret < 0 || ret >= MAXPATHLEN)
-		goto out_err;
 
-	if ((fd = open(nspath, O_RDONLY)) < 0) {
-		fprintf(stderr, "Opening %s\n", nspath);
-		goto out_err;
+	fd = lxc_preserve_ns(pid, "net");
+	if (fd < 0) {
+		fprintf(stderr, "Failed opening network namespace path for '%d'.", pid);
+		return -1;
 	}
+
 	if (setns(fd, 0) < 0) {
 		fprintf(stderr, "setns to container network namespace\n");
 		goto out_err;

src/lxc/namespace.c

@@ -64,29 +64,29 @@ pid_t lxc_clone(int (*fn)(void *), void *arg, int flags)
 	ret = clone(do_clone, stack  + stack_size, flags | SIGCHLD, &clone_arg);
 #endif
 	if (ret < 0)
-		ERROR("failed to clone (%#x): %s", flags, strerror(errno));
+		ERROR("Failed to clone (%#x): %s.", flags, strerror(errno));
 
 	return ret;
 }
 
-static const char * const namespaces_list[] = {
-	"MOUNT", "PID", "UTSNAME", "IPC",
-	"USER", "NETWORK"
-};
-static const int cloneflags_list[] = {
-	CLONE_NEWNS, CLONE_NEWPID, CLONE_NEWUTS, CLONE_NEWIPC,
-	CLONE_NEWUSER, CLONE_NEWNET
+const struct ns_info ns_info[LXC_NS_MAX] = {
+	[LXC_NS_MNT] = {"mnt", CLONE_NEWNS, "CLONE_NEWNS"},
+	[LXC_NS_PID] = {"pid", CLONE_NEWPID, "CLONE_NEWPID"},
+	[LXC_NS_UTS] = {"uts", CLONE_NEWUTS, "CLONE_NEWUTS"},
+	[LXC_NS_IPC] = {"ipc", CLONE_NEWIPC, "CLONE_NEWIPC"},
+	[LXC_NS_USER] = {"user", CLONE_NEWUSER, "CLONE_NEWUSER"},
+	[LXC_NS_NET] = {"net", CLONE_NEWNET, "CLONE_NEWNET"},
+	[LXC_NS_CGROUP] = {"cgroup", CLONE_NEWCGROUP, "CLONE_NEWCGROUP"}
 };
 
 int lxc_namespace_2_cloneflag(char *namespace)
 {
-	int i, len;
-	len = sizeof(namespaces_list)/sizeof(namespaces_list[0]);
-	for (i = 0; i < len; i++)
-		if (!strcmp(namespaces_list[i], namespace))
-			return cloneflags_list[i];
+	int i;
+	for (i = 0; i < LXC_NS_MAX; i++)
+		if (!strcasecmp(ns_info[i].proc_name, namespace))
+			return ns_info[i].clone_flag;
 
-	ERROR("invalid namespace name %s", namespace);
+	ERROR("Invalid namespace name: %s.", namespace);
 	return -1;
 }
 
@@ -96,7 +96,7 @@ int lxc_fill_namespace_flags(char *flaglist, int *flags)
 	int aflag;
 
 	if (!flaglist) {
-		ERROR("need at least one namespace to unshare");
+		ERROR("At least one namespace is needed.");
 		return -1;
 	}
 

src/lxc/namespace.h

@@ -53,6 +53,23 @@
 #  define CLONE_NEWNET            0x40000000
 #endif
 
+enum {
+	LXC_NS_MNT,
+	LXC_NS_PID,
+	LXC_NS_UTS,
+	LXC_NS_IPC,
+	LXC_NS_USER,
+	LXC_NS_NET,
+	LXC_NS_CGROUP,
+	LXC_NS_MAX
+};
+
+extern const struct ns_info {
+	const char *proc_name;
+	int clone_flag;
+	const char *flag_name;
+} ns_info[LXC_NS_MAX];
+
 #if defined(__ia64__)
 int __clone2(int (*__fn) (void *__arg), void *__child_stack_base,
              size_t __child_stack_size, int __flags, void *__arg, ...);
@@ -62,7 +79,6 @@ int clone(int (*fn)(void *), void *child_stack,
 	/* pid_t *ptid, struct user_desc *tls, pid_t *ctid */ );
 #endif
 
-
 extern pid_t lxc_clone(int (*fn)(void *), void *arg, int flags);
 
 extern int lxc_namespace_2_cloneflag(char *namespace);

src/lxc/start.h

@@ -42,24 +42,6 @@ struct lxc_operations {
 
 struct cgroup_desc;
 
-enum {
-	LXC_NS_MNT,
-	LXC_NS_PID,
-	LXC_NS_UTS,
-	LXC_NS_IPC,
-	LXC_NS_USER,
-	LXC_NS_NET,
-	LXC_NS_CGROUP,
-	LXC_NS_MAX
-};
-
-struct ns_info {
-	const char *proc_name;
-	int clone_flag;
-};
-
-extern const struct ns_info ns_info[LXC_NS_MAX];
-
 struct lxc_handler {
 	pid_t pid;
 	char *name;
@@ -77,6 +59,7 @@ struct lxc_handler {
 	int ttysock[2]; // socketpair for child->parent tty fd passing
 	bool backgrounded; // indicates whether should we close std{in,out,err} on start
 	int nsfd[LXC_NS_MAX];
+	int netnsfd;
 };
 
 

src/lxc/utils.c

@@ -1968,3 +1968,23 @@ int lxc_append_string(char ***list, char *entry)
 
 	return 0;
 }
+
+int lxc_preserve_ns(const int pid, const char *ns)
+{
+	int ret;
+/* 5 /proc + 21 /int_as_str + 3 /ns + 20 /NS_NAME + 1 \0 */
+#define __NS_PATH_LEN 50
+	char path[__NS_PATH_LEN];
+
+	/* This way we can use this function to also check whether namespaces
+	 * are supported by the kernel by passing in the NULL or the empty
+	 * string.
+	 */
+	ret = snprintf(path, __NS_PATH_LEN, "/proc/%d/ns%s%s", pid,
+		       !ns || strcmp(ns, "") == 0 ? "" : "/",
+		       !ns || strcmp(ns, "") == 0 ? "" : ns);
+	if (ret < 0 || (size_t)ret >= __NS_PATH_LEN)
+		return -1;
+
+	return open(path, O_RDONLY | O_CLOEXEC);
+}

src/lxc/utils.h

@@ -312,6 +312,7 @@ int open_devnull(void);
 int set_stdfds(int fd);
 int null_stdfds(void);
 int lxc_count_file_lines(const char *fn);
+int lxc_preserve_ns(const int pid, const char *ns);
 
 /* Check whether a signal is blocked by a process. */
 bool task_blocking_signal(pid_t pid, int signal);