On 12/04/2012 01:19 PM, Serge Hallyn wrote:
> When a physical nic is being set up, store its ifindex and original name
> in struct lxc_conf.  At reboot, reset the original name.
> We can't just go over the original network list in lxc_conf at shutdown
> because that may be tweaked in the meantime through the C api.  The
> saved_nics list is only setup during lxc_spawn(), and restored and
> freed after lxc_start.
> 
> Without this patch, if you take a container with physical nic eth1
> renamed to eth0, start it, shut it down, and restart it, the last
> restart will fail.
> 
> Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/1086244
> 
> Reported-by: Avijit Ghosh <avijit.gh...@aricent.com>
> Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>

Just one comment below, but looks good.

Acked-by: Stéphane Graber <stgra...@ubuntu.com>

> ---
>  src/lxc/conf.c    |   28 ++++++++++++++++++++++++++++
>  src/lxc/conf.h    |    9 +++++++++
>  src/lxc/execute.c |    6 ++++--
>  src/lxc/start.c   |   38 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 79d96d7..45e0b31 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1821,6 +1821,21 @@ static int setup_network(struct lxc_list *network)
>       return 0;
>  }
>  
> +void lxc_rename_phys_nics_on_shutdown(struct lxc_conf *conf)
> +{
> +     int i;
> +
> +     INFO("running to reset %d nic names", conf->num_savednics);
> +     for (i=0; i<conf->num_savednics; i++) {
> +             struct saved_nic *s = &conf->saved_nics[i];
> +             INFO("resetting nic %d to %s\n", s->ifindex, s->orig_name);
> +             lxc_netdev_rename_by_index(s->ifindex, s->orig_name);
> +             free(s->orig_name);
> +     }
> +     conf->num_savednics = 0;
> +     free(conf->saved_nics);
> +}
> +
>  static int setup_private_host_hw_addr(char *veth1)
>  {
>       struct ifreq ifr;
> @@ -2710,6 +2725,18 @@ int lxc_clear_hooks(struct lxc_conf *c, const char 
> *key)
>       return 0;
>  }
>  
> +void lxc_clear_saved_nics(struct lxc_conf *conf)
> +{
> +     int i;
> +
> +     if (!conf->num_savednics)
> +             return;
> +     for (i=0; i < conf->num_savednics; i++)
> +             free(conf->saved_nics[i].orig_name);
> +     conf->saved_nics = 0;
> +     free(conf->saved_nics);
> +}
> +
>  void lxc_conf_free(struct lxc_conf *conf)
>  {
>       if (!conf)
> @@ -2737,5 +2764,6 @@ void lxc_conf_free(struct lxc_conf *conf)
>       lxc_clear_cgroups(conf, "lxc.cgroup");
>       lxc_clear_hooks(conf, "lxc.hook");
>       lxc_clear_mount_entries(conf);
> +     lxc_clear_saved_nics(conf);
>       free(conf);
>  }
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index 694bce4..3f6181f 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -211,6 +211,11 @@ enum lxchooks {
>       LXCHOOK_POSTSTOP, NUM_LXC_HOOKS};
>  extern char *lxchook_names[NUM_LXC_HOOKS];
>  
> +struct saved_nic {
> +     int ifindex;
> +     char *orig_name;
> +};
> +
>  struct lxc_conf {
>       char *fstab;
>       int tty;
> @@ -221,6 +226,8 @@ struct lxc_conf {
>       struct utsname *utsname;
>       struct lxc_list cgroup;
>       struct lxc_list network;
> +     struct saved_nic *saved_nics;
> +     int num_savednics;
>       struct lxc_list mount_list;
>       struct lxc_list caps;
>       struct lxc_tty_info tty_info;
> @@ -273,4 +280,6 @@ extern int lxc_clear_hooks(struct lxc_conf *c, const char 
> *key);
>   */
>  
>  extern int lxc_setup(const char *name, struct lxc_conf *lxc_conf);
> +
> +extern void lxc_rename_phys_nics_on_shutdown(struct lxc_conf *conf);
>  #endif
> diff --git a/src/lxc/execute.c b/src/lxc/execute.c
> index 487765f..730b793 100644
> --- a/src/lxc/execute.c
> +++ b/src/lxc/execute.c
> @@ -27,7 +27,6 @@
>  #include <unistd.h>
>  #include <stdlib.h>
>  
> -
>  #include "log.h"
>  #include "start.h"
>  
> @@ -134,9 +133,12 @@ int lxc_execute(const char *name, char *const argv[], 
> int quiet,
>               .argv = argv,
>               .quiet = quiet
>       };
> +     int ret;
>  
>       if (lxc_check_inherited(conf, -1))
>               return -1;
>  
> -     return __lxc_start(name, conf, &execute_start_ops, &args);
> +     ret = __lxc_start(name, conf, &execute_start_ops, &args);
> +
> +     return ret;
>  }

What's the reason for that bit? Looks to me as functionally identical.

> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index 3e26b27..7320d74 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -579,6 +579,37 @@ out_warn_father:
>       return -1;
>  }
>  
> +int save_phys_nics(struct lxc_conf *conf)
> +{
> +     struct lxc_list *iterator;
> +
> +     lxc_list_for_each(iterator, &conf->network) {
> +             struct lxc_netdev *netdev = iterator->elem;
> +
> +             if (netdev->type != LXC_NET_PHYS)
> +                     continue;
> +             conf->saved_nics = realloc(conf->saved_nics,
> +                             (conf->num_savednics+1)*sizeof(struct 
> saved_nic));
> +             if (!conf->saved_nics) {
> +                     SYSERROR("failed to allocate memory");
> +                     return -1;
> +             }
> +             conf->saved_nics[conf->num_savednics].ifindex = netdev->ifindex;
> +             conf->saved_nics[conf->num_savednics].orig_name = 
> strdup(netdev->link);
> +             if (!conf->saved_nics[conf->num_savednics].orig_name) {
> +                     SYSERROR("failed to allocate memory");
> +                     return -1;
> +             }
> +             INFO("stored saved_nic #%d idx %d name %s\n", 
> conf->num_savednics,
> +                     conf->saved_nics[conf->num_savednics].ifindex,
> +                     conf->saved_nics[conf->num_savednics].orig_name);
> +             conf->num_savednics++;
> +     }
> +
> +     return 0;
> +}
> +
> +
>  int lxc_spawn(struct lxc_handler *handler)
>  {
>       int failed_before_rename = 0;
> @@ -613,6 +644,11 @@ int lxc_spawn(struct lxc_handler *handler)
>               }
>       }
>  
> +     if (save_phys_nics(handler->conf)) {
> +             ERROR("failed to save physical nic info");
> +             goto out_abort;
> +     }
> +
>       /*
>        * if the rootfs is not a blockdev, prevent the container from
>        * marking it readonly.
> @@ -739,6 +775,8 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
>               }
>          }
>  
> +     lxc_rename_phys_nics_on_shutdown(handler->conf);
> +
>       err =  lxc_error_set_and_log(handler->pid, status);
>  out_fini:
>       lxc_delete_network(handler);
> 


-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to