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>

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 651f1f33cba1715e156ae6819fda8c627c588e59..6bf41013c2c1114956bf92982f396989349d8b94 100644 (file)
--- a/src/lxc/conf.c
+++ b/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"

diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index 842e4dcff71b143837f479820a78ed1d8875c4a6..a0f0c3ebc487acd49ed7ce173e6a2bd20c548ea6 100644 (file)
--- a/src/lxc/conf.h
+++ b/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);

diff --git a/src/lxc/start.c b/src/lxc/start.c
index cc48962449c7bb94346c6d9f22dc3c5871f50974..9144050dbb47ffc68c0ff3fcddad103b5df3e565 100644 (file)
--- a/src/lxc/start.c
+++ b/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);