conf, start: be smarter when deleting networks

- So far we blindly called lxc_delete_network() to make sure that we deleted
  all network interfaces. This resulted in pointless netlink calls, especially
  when a container had multiple networks defined. Let's be smarter and have
  lxc_delete_network() return a boolean that indicates whether *all* configured
  networks have been deleted. If so, don't needlessly try to delete them again
  in start.c. This also decreases confusing error messages a user might see.

- When we receive -ENODEV from one of our lxc_netdev_delete_*() functions,
  let's assume that either the network device already got deleted or that it
  got moved to a different network namespace. Inform the user about this but do
  not report an error in this case.

- When we have explicitly deleted the host side of a veth pair let's
  immediately free(priv.veth_attr.pair) and NULL it, or
  memset(priv.veth_attr.pair, ...) the corresponding member so we don't
  needlessly try to destroy them again when we have to call
  lxc_delete_network() again in start.c

Signed-off-by: Christian Brauner <christian.brauner@canonical.com>

src/lxc/conf.c

@@ -2880,20 +2880,22 @@ 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;
 		}
 
@@ -2907,35 +2909,54 @@ void lxc_delete_network(struct lxc_handler *handler)
 		 */
 		if (netdev->ifindex != 0) {
 			ret = lxc_netdev_delete_by_index(netdev->ifindex);
-			if (ret < 0)
-				WARN("Failed to remove interface %d '%s': %s.",
-						netdev->ifindex,
-						netdev->name ? netdev->name : "(null)", strerror(-ret));
-			else
-				INFO("Removed interface %d '%s'.",
-						netdev->ifindex,
-						netdev->name ? netdev->name : "(null)");
+			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 = NULL;
-			if (netdev->priv.veth_attr.pair)
+			char *hostveth;
+			if (netdev->priv.veth_attr.pair) {
 				hostveth = netdev->priv.veth_attr.pair;
-			else if (strlen(netdev->priv.veth_attr.veth1) > 0)
+				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;
-
-			if (hostveth) {
 				ret = lxc_netdev_delete_by_name(hostveth);
-				if (ret < 0)
-					WARN("Failed to remove '%s' from host.", hostveth);
-				else
-					INFO("Deleted '%s' from host.", 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"

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/start.c

@@ -1343,6 +1343,7 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
 	struct lxc_handler *handler;
 	int err = -1;
 	int status;
+	bool removed_all_netdevs = true;
 
 	handler = lxc_init(name, conf, lxcpath);
 	if (!handler) {
@@ -1437,7 +1438,7 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
 	lxc_restore_phys_nics_to_netns(handler->netnsfd, handler->conf);
 
 	DEBUG("Tearing down virtual network devices used by container");
-	lxc_delete_network(handler);
+	removed_all_netdevs = lxc_delete_network(handler);
 
 	if (handler->pinfd >= 0) {
 		close(handler->pinfd);
@@ -1447,7 +1448,12 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
 	lxc_monitor_send_exit_code(name, status, handler->lxcpath);
 	err =  lxc_error_set_and_log(handler->pid, status);
 out_fini:
-	lxc_delete_network(handler);
+	if (!removed_all_netdevs) {
+		DEBUG("Failed tearing down all network devices used by container. Trying again!");
+		removed_all_netdevs = lxc_delete_network(handler);
+		if (!removed_all_netdevs)
+			DEBUG("Failed tearing down network devices used by container. Not trying again!");
+	}
 
 out_detach_blockdev:
 	detach_block_device(handler->conf);