Quoting KATOH Yasufumi (ka...@jazz.email.ne.jp):
> Hi, Sorry for my slow response.
> 
> >>> On Wed, 8 Oct 2014 18:21:19 +0000
>     in message   "Re: [lxc-devel] lxc-clone do not work when overlayfs.v22 or 
> higher is used"
>                   Serge Hallyn-san wrote:
> 
> > Quoting KATOH Yasufumi (ka...@jazz.email.ne.jp):
> > > Hi,
> > > 
> > > Now, lxc-clone with overlayfs (lxc-clone -s -B overlayfs) runs internally:
> > >   mount -t overlayfs -oupperdir=${upper},lowerdir=${lower} lower dest
> > > 
> > > But, overlayfs.v22(kernel 3.15) or higher need the option "workdir=",
> > > so lxc-clone do not work.
> > > 
> > > On 3.17 with overlayfs.v24, without workdir option:
> > >   # mount -n -t overlayfs -o lowerdir=1,upperdir=2 overlayfs 3
> > >   mount: wrong fs type, bad option, bad superblock on overlayfs,
> > >          missing codepage or helper program, or other error
> > > 
> > >          In some cases useful info is found in syslog - try
> > >          dmesg | tail or so.
> > >   # dmesg | tail
> > >   [  683.456473] overlayfs: missing upperdir or lowerdir or workdir
> > > 
> > > On 3.17 with overlayfs.v24, with workdir option:
> > >   # mount -n -t overlayfs -o lowerdir=1,upperdir=2,workdir=3 overlayfs 4
> > >   # cat /proc/mounts | grep overlayfs
> > >   overlayfs /root/overlayfs/4 overlayfs 
> > > rw,relatime,lowerdir=1,upperdir=2,workdir=3 0 0
> > > 
> > > See
> > >   
> > > https://git.kernel.org/cgit/linux/kernel/git/mszeredi/vfs.git/tree/Documentation/filesystems/overlayfs.txt?h=overlayfs.v24
> 
> > The older version (which is still in use in the 3.16 kernel in Ubuntu 14.10)
> > fails with that option, so it looks like there's not much to do but try
> > the workdir=, then if that fails try without workdir=.
> 
> > Can you send a patch to do that?
> 
> I try it. But I don't know well the details of the clone process, I
> don't know whether this is right.

Hi,

Mostly ok, just a few cmments below.  If you like I'll make the
changes as I apply, or you can send a new version.

thanks,
-serge

> This patch is
> * the workdir is LXCPATH/ct/olwork.
> * mkdir workdir when clone
> * mount without workdir is failed, then retry mount with workdir
> 
> I've tested on:
>   * 3.14.4 with overlayfs.v21 (not need workdir=) (priv and unpriv)
>   * 3.17.0 with overlayfs.v24 (need workdir=) (priv and unpriv)
> 
> Signed-off-by: KATOH Yasufumi <ka...@jazz.email.ne.jp>
> ---
>  src/lxc/bdev.c | 77 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 70 insertions(+), 7 deletions(-)
> 
> diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
> index 4fc10f2..69c2e7f 100644
> --- a/src/lxc/bdev.c
> +++ b/src/lxc/bdev.c
> @@ -2154,10 +2154,11 @@ static int overlayfs_detect(const char *path)
>  static int overlayfs_mount(struct bdev *bdev)
>  {
>       char *options, *dup, *lower, *upper;
> -     int len;
> +     char *options_work, *work, *worktmp;
> +     int len, len2;
>       unsigned long mntflags;
>       char *mntdata;
> -     int ret;
> +     int ret, ret2;
>  
>       if (strcmp(bdev->type, "overlayfs"))
>               return -22;
> @@ -2175,6 +2176,14 @@ static int overlayfs_mount(struct bdev *bdev)
>       *upper = '\0';
>       upper++;
>  
> +     // workdir option that is needed overlayfs.v22 or higher
> +     // workdir=$LXCDIR/ct/olwork
> +     work = alloca(strlen(upper)+1);
> +     strcpy(work, upper);
> +     if (!(worktmp = rindex(work, '/')))
> +             return -22;
> +     strncpy(worktmp+1, "olwork", 6);

can't do this exactly, because olwork my be longer than the upper
filename specified by the container.  Just liberally adding 6 to
the size you alloca should be ok.

(because upper does not have to be 'rootfs')

> +
>       if (parse_mntopts(bdev->mntopts, &mntflags, &mntdata) < 0) {
>               free(mntdata);
>               return -22;
> @@ -2187,21 +2196,42 @@ static int overlayfs_mount(struct bdev *bdev)
>               len = strlen(lower) + strlen(upper) + 
> strlen("upperdir=,lowerdir=,") + strlen(mntdata) + 1;
>               options = alloca(len);
>               ret = snprintf(options, len, "upperdir=%s,lowerdir=%s,%s", 
> upper, lower, mntdata);
> +
> +             len2 = strlen(lower) + strlen(upper) + strlen(work)
> +                     + strlen("upperdir=,lowerdir=,workdir=") + 
> strlen(mntdata) + 1;
> +             options_work = alloca(len2);
> +             ret2 = snprintf(options, len2, 
> "upperdir=%s,lowerdir=%s,workdir=%s,%s",
> +                     upper, lower, work, mntdata);
>       }
>       else {
>               len = strlen(lower) + strlen(upper) + 
> strlen("upperdir=,lowerdir=") + 1;
>               options = alloca(len);
>               ret = snprintf(options, len, "upperdir=%s,lowerdir=%s", upper, 
> lower);
> +
> +             len2 = strlen(lower) + strlen(upper) + strlen(work)
> +                     + strlen("upperdir=,lowerdir=,workdir=") + 1;
> +             options_work = alloca(len2);
> +             ret2 = snprintf(options_work, len2, 
> "upperdir=%s,lowerdir=%s,workdir=%s",
> +                     upper, lower, work);
>       }
> -     if (ret < 0 || ret >= len) {
> +     if (ret < 0 || ret >= len || ret2 < 0 || ret2 >= len2) {
>               free(mntdata);
>               return -1;
>       }
>  
>       ret = mount(lower, bdev->dest, "overlayfs", MS_MGC_VAL | mntflags, 
> options);
> -     if (ret < 0)
> -             SYSERROR("overlayfs: error mounting %s onto %s options %s",
> +     if (ret < 0) {
> +             WARN("overlayfs: error mounting %s onto %s options %s",
>                       lower, bdev->dest, options);

Let's actually make this an INFO.  Or actually it'll be clear from the INFO 
below
so we can just drop the message altogether.

> +             // retry with workdir option
> +             ret = mount(lower, bdev->dest, "overlayfs", MS_MGC_VAL | 
> mntflags, options_work);
> +             if (ret < 0)
> +                     SYSERROR("overlayfs: error mounting %s onto %s options 
> %s",
> +                             lower, bdev->dest, options_work);
> +             else
> +                     INFO("overlayfs: mounted %s onto %s options %s",
> +                             lower, bdev->dest, options_work);
> +     }
>       else
>               INFO("overlayfs: mounted %s onto %s options %s",
>                       lower, bdev->dest, options);
> @@ -2265,7 +2295,7 @@ static int overlayfs_clonepaths(struct bdev *orig, 
> struct bdev *new, const char
>               WARN("Failed to update ownership of %s", new->dest);
>  
>       if (strcmp(orig->type, "dir") == 0) {
> -             char *delta;
> +             char *delta, *work;
>               int ret, len;
>  
>               // if we have /var/lib/lxc/c2/rootfs, then delta will be
> @@ -2287,6 +2317,23 @@ static int overlayfs_clonepaths(struct bdev *orig, 
> struct bdev *new, const char
>               if (am_unpriv() && chown_mapped_root(delta, conf) < 0)
>                       WARN("Failed to update ownership of %s", delta);
>  
> +             // mkdir workdir that is needed on overlayfs.v22 or higher
> +             // if we have /var/lib/lxc/c2/rootfs, then workdir will be
> +             //            /var/lib/lxc/c2/olwork
> +             work = strdup(new->dest);

I realize code above this is doing the same thing , and it actually
works out because we always create the new container with bdev->dest NULL
and then in bdev_copy fill it in ending in rootfs.  But I'd prefer we
change that in both places so that if/when that flow changes we dont
get mysterious occasional segvs.

> +             if (!work)
> +                     return -1;
> +
> +             strcpy(&work[strlen(work)-6], "olwork");
> +             if (mkdir(work, 0755) < 0) {
> +                     SYSERROR("error: mkdir %s", work);
> +                     free(work);
> +                     return -1;
> +             }
> +             if (am_unpriv() && chown_mapped_root(work, conf) < 0)
> +                     WARN("Failed to update ownership of %s", work);
> +             free(work);
> +
>               // the src will be 'overlayfs:lowerdir:upperdir'
>               len = strlen(delta) + strlen(orig->src) + 12;
>               new->src = malloc(len);
> @@ -2303,7 +2350,7 @@ static int overlayfs_clonepaths(struct bdev *orig, 
> struct bdev *new, const char
>               // I think we want to use the original lowerdir, with a
>               // private delta which is originally rsynced from the
>               // original delta
> -             char *osrc, *odelta, *nsrc, *ndelta;
> +             char *osrc, *odelta, *nsrc, *ndelta, *work;
>               int len, ret;
>               if (!(osrc = strdup(orig->src)))
>                       return -22;
> @@ -2327,6 +2374,22 @@ static int overlayfs_clonepaths(struct bdev *orig, 
> struct bdev *new, const char
>               }
>               if (am_unpriv() && chown_mapped_root(ndelta, conf) < 0)
>                       WARN("Failed to update ownership of %s", ndelta);
> +
> +             // mkdir workdir for overlayfs.v22 or later

Workdir temporary holds files before atomically switching with
the destination, so it's ok for it to be empty.  Let's a comment
to that effect so future reviewers can feel sure about that
without looking at the overlayfs details.

> +             work = strdup(ndelta);
> +             if (!work)
> +                     return -1;
> +
> +             strcpy(&work[strlen(work)-6], "olwork");
> +             if (mkdir(work, 0755) < 0) {
> +                     SYSERROR("error: mkdir %s", work);
> +                     free(work);
> +                     return -1;
> +             }
> +             if (am_unpriv() && chown_mapped_root(work, conf) < 0)
> +                     WARN("Failed to update ownership of %s", work);
> +             free(work);
> +
>               struct rsync_data_char rdata;
>               rdata.src = odelta;
>               rdata.dest = ndelta;
> -- 
> 2.1.1
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to