Quoting S.Çağlar Onur ([email protected]):
> changes since v1;
>       incorporated Serge's changes
> changes since v2;
>       added missing Signed-off-by
> 
> Signed-off-by: S.Çağlar Onur <[email protected]>

The only thing that would worry me if we were past 1.0 would be that
changing the values of the other copy hooks could cause trouble with
a newly compiled client talking to some long-running service based
on the API.  (I haven't heard of any such long-running service existing
yet)  So if we had released 1.0, I would want the other values to remain
unchanged, and the COPYHOOKS value could simply remain blank until
after some deprecation period it could be reused by new values.

Acked-by: Serge E. Hallyn <[email protected]>

> ---
>  src/lxc/lxc_clone.c            |  7 +------
>  src/lxc/lxccontainer.c         | 30 ++++++++++++++++++------------
>  src/lxc/lxccontainer.h         |  8 +++-----
>  src/python-lxc/lxc.c           |  1 -
>  src/python-lxc/lxc/__init__.py |  1 -
>  5 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/src/lxc/lxc_clone.c b/src/lxc/lxc_clone.c
> index 88a768d..7464dcd 100644
> --- a/src/lxc/lxc_clone.c
> +++ b/src/lxc/lxc_clone.c
> @@ -69,8 +69,6 @@ void usage(const char *me)
>       printf("  -L: for blockdev-backed backingstore, use specified size\n");
>       printf("  -K: Keep name - do not change the container name\n");
>       printf("  -M: Keep macaddr - do not choose a random new mac address\n");
> -     printf("  -H: copy Hooks - copy mount hooks into container 
> directory\n");
> -     printf("      and substitute container names and lxcpaths\n");
>       printf("  -p: use container orig from custom lxcpath\n");
>       printf("  -P: create container new in custom lxcpath\n");
>       exit(1);
> @@ -85,7 +83,6 @@ static struct option options[] = {
>       { "vgname", required_argument, 0, 'v'},
>       { "keepname", no_argument, 0, 'K'},
>       { "keepmac", no_argument, 0, 'M'},
> -     { "copyhooks", no_argument, 0, 'H'},  // should this be default?
>       { "lxcpath", required_argument, 0, 'p'},
>       { "newpath", required_argument, 0, 'P'},
>       { "fstype", required_argument, 0, 't'},
> @@ -96,7 +93,7 @@ static struct option options[] = {
>  int main(int argc, char *argv[])
>  {
>       struct lxc_container *c1 = NULL, *c2 = NULL;
> -     int snapshot = 0, keepname = 0, keepmac = 0, copyhooks = 0;
> +     int snapshot = 0, keepname = 0, keepmac = 0;
>       int flags = 0, option_index;
>       long newsize = 0;
>       char *bdevtype = NULL, *lxcpath = NULL, *newpath = NULL, *fstype = NULL;
> @@ -120,7 +117,6 @@ int main(int argc, char *argv[])
>               case 'v': vgname = optarg; break;
>               case 'K': keepname = 1; break;
>               case 'M': keepmac = 1; break;
> -             case 'H': copyhooks = 1; break;
>               case 'p': lxcpath = optarg; break;
>               case 'P': newpath = optarg; break;
>               case 't': fstype = optarg; break;
> @@ -143,7 +139,6 @@ int main(int argc, char *argv[])
>       if (snapshot)  flags |= LXC_CLONE_SNAPSHOT;
>       if (keepname)  flags |= LXC_CLONE_KEEPNAME;
>       if (keepmac)   flags |= LXC_CLONE_KEEPMACADDR;
> -     if (copyhooks) flags |= LXC_CLONE_COPYHOOKS;
>  
>       // vgname and fstype could be supported by sending them through the
>       // bdevdata.  However, they currently are not yet.  I'm not convinced
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 1af8d62..203f4a8 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2184,9 +2184,15 @@ err:
>  
>  static int copyhooks(struct lxc_container *oldc, struct lxc_container *c)
>  {
> -     int i;
> -     int ret;
> +     int i, len, ret;
>       struct lxc_list *it;
> +     char *cpath;
> +
> +     len = strlen(oldc->config_path) + strlen(oldc->name) + 3;
> +     cpath = alloca(len);
> +     ret = snprintf(cpath, len, "%s/%s/", oldc->config_path, oldc->name);
> +     if (ret < 0 || ret >= len)
> +             return -1;
>  
>       for (i=0; i<NUM_LXC_HOOKS; i++) {
>               lxc_list_for_each(it, &c->lxc_conf->hooks[i]) {
> @@ -2195,6 +2201,10 @@ static int copyhooks(struct lxc_container *oldc, 
> struct lxc_container *c)
>                       char tmppath[MAXPATHLEN];
>                       if (!fname) // relative path - we don't support, but 
> maybe we should
>                               return 0;
> +                     if (strncmp(hookname, cpath, len - 1) != 0) {
> +                             // this hook is public - ignore
> +                             continue;
> +                     }
>                       // copy the script, and change the entry in confile
>                       ret = snprintf(tmppath, MAXPATHLEN, "%s/%s/%s",
>                                       c->config_path, c->name, fname+1);
> @@ -2546,14 +2556,11 @@ struct lxc_container *lxcapi_clone(struct 
> lxc_container *c, const char *newname,
>               goto out;
>       }
>  
> -
> -     // copy hooks if requested
> -     if (flags & LXC_CLONE_COPYHOOKS) {
> -             ret = copyhooks(c, c2);
> -             if (ret < 0) {
> -                     ERROR("error copying hooks");
> -                     goto out;
> -             }
> +     // copy hooks
> +     ret = copyhooks(c, c2);
> +     if (ret < 0) {
> +             ERROR("error copying hooks");
> +             goto out;
>       }
>  
>       if (copy_fstab(c, c2) < 0) {
> @@ -2600,7 +2607,6 @@ static bool lxcapi_rename(struct lxc_container *c, 
> const char *newname)
>  {
>       struct bdev *bdev;
>       struct lxc_container *newc;
> -     int flags = LXC_CLONE_KEEPMACADDR | LXC_CLONE_COPYHOOKS;
>  
>       if (!c || !c->name || !c->config_path)
>               return false;
> @@ -2611,7 +2617,7 @@ static bool lxcapi_rename(struct lxc_container *c, 
> const char *newname)
>               return false;
>       }
>  
> -     newc = lxcapi_clone(c, newname, c->config_path, flags, NULL, 
> bdev->type, 0, NULL);
> +     newc = lxcapi_clone(c, newname, c->config_path, LXC_CLONE_KEEPMACADDR, 
> NULL, bdev->type, 0, NULL);
>       bdev_put(bdev);
>       if (!newc) {
>               lxc_container_put(newc);
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index b7c5313..797ad91 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -29,10 +29,9 @@
>  #include <stdlib.h>
>  
>  #define LXC_CLONE_KEEPNAME        (1 << 0) /*!< Do not edit the rootfs to 
> change the hostname */
> -#define LXC_CLONE_COPYHOOKS       (1 << 1) /*!< Copy all hooks into the 
> container directory */
> -#define LXC_CLONE_KEEPMACADDR     (1 << 2) /*!< Do not change the MAC 
> address on network interfaces */
> -#define LXC_CLONE_SNAPSHOT        (1 << 3) /*!< Snapshot the original 
> filesystem(s) */
> -#define LXC_CLONE_MAXFLAGS        (1 << 4) /*!< Number of \c LXC_CLONE_* 
> flags */
> +#define LXC_CLONE_KEEPMACADDR     (1 << 1) /*!< Do not change the MAC 
> address on network interfaces */
> +#define LXC_CLONE_SNAPSHOT        (1 << 2) /*!< Snapshot the original 
> filesystem(s) */
> +#define LXC_CLONE_MAXFLAGS        (1 << 3) /*!< Number of \c LXC_CLONE_* 
> flags */
>  #define LXC_CREATE_QUIET          (1 << 0) /*!< Redirect \c stdin to \c 
> /dev/zero and \c stdout and \c stderr to \c /dev/null */
>  #define LXC_CREATE_MAXFLAGS       (1 << 1) /*!< Number of \c LXC_CREATE* 
> flags */
>  
> @@ -516,7 +515,6 @@ struct lxc_container {
>        *  (XXX: should we use the default instead?)
>        * \param flags Additional \c LXC_CLONE* flags to change the cloning 
> behaviour:
>        *  - \ref LXC_CLONE_KEEPNAME
> -      *  - \ref LXC_CLONE_COPYHOOKS
>        *  - \ref LXC_CLONE_KEEPMACADDR
>        *  - \ref LXC_CLONE_SNAPSHOT
>        * \param bdevtype Optionally force the cloned bdevtype to a specified 
> plugin.
> diff --git a/src/python-lxc/lxc.c b/src/python-lxc/lxc.c
> index cd92949..0099b98 100644
> --- a/src/python-lxc/lxc.c
> +++ b/src/python-lxc/lxc.c
> @@ -1733,7 +1733,6 @@ PyInit__lxc(void)
>      PYLXC_EXPORT_CONST(LXC_ATTACH_SET_PERSONALITY);
>  
>      /* clone: clone flags */
> -    PYLXC_EXPORT_CONST(LXC_CLONE_COPYHOOKS);
>      PYLXC_EXPORT_CONST(LXC_CLONE_KEEPMACADDR);
>      PYLXC_EXPORT_CONST(LXC_CLONE_KEEPNAME);
>      PYLXC_EXPORT_CONST(LXC_CLONE_SNAPSHOT);
> diff --git a/src/python-lxc/lxc/__init__.py b/src/python-lxc/lxc/__init__.py
> index e708781..e389abb 100644
> --- a/src/python-lxc/lxc/__init__.py
> +++ b/src/python-lxc/lxc/__init__.py
> @@ -454,7 +454,6 @@ LXC_ATTACH_REMOUNT_PROC_SYS = 
> _lxc.LXC_ATTACH_REMOUNT_PROC_SYS
>  LXC_ATTACH_SET_PERSONALITY = _lxc.LXC_ATTACH_SET_PERSONALITY
>  
>  # clone: clone flags
> -LXC_CLONE_COPYHOOKS = _lxc.LXC_CLONE_COPYHOOKS
>  LXC_CLONE_KEEPMACADDR = _lxc.LXC_CLONE_KEEPMACADDR
>  LXC_CLONE_KEEPNAME = _lxc.LXC_CLONE_KEEPNAME
>  LXC_CLONE_SNAPSHOT = _lxc.LXC_CLONE_SNAPSHOT
> -- 
> 1.8.3.2
> 
> _______________________________________________
> lxc-devel mailing list
> [email protected]
> http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________
lxc-devel mailing list
[email protected]
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to