Quoting KATOH Yasufumi (ka...@jazz.email.ne.jp):
> Hi,
> 
> I made changes.
> 
> >>> On Sat, 18 Oct 2014 01:24:55 +0900
>     in message   "Re: [lxc-devel] [PATCH RFC] overlayfs: overlayfs.v22 or     
> higher  needs workdir option"
>                  I wrote:
> 
> > > > 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 for your comment! I have some questions. 
> 
> > Sorry. I did not read your patch:
> >   
> > https://lists.linuxcontainers.org/pipermail/lxc-devel/2014-October/010687.html
> 
> > I will check it and resend this patch.
> 
> * I assume that upperdir is "delta0". Is this right?
> * This patch can apply after applying the patch:
>   
> https://lists.linuxcontainers.org/pipermail/lxc-devel/2014-October/010687.html
> * Add some comments.
> 
> ---
>  src/lxc/bdev.c | 88 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 82 insertions(+), 6 deletions(-)
> 
> diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
> index 8a819ab..2253a1b 100644
> --- a/src/lxc/bdev.c
> +++ b/src/lxc/bdev.c
> @@ -2154,10 +2154,12 @@ 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, *lastslash;
> +     int lastslashidx;
> +     int len, len2;
>       unsigned long mntflags;
>       char *mntdata;
> -     int ret;
> +     int ret, ret2;
>  
>       if (strcmp(bdev->type, "overlayfs"))
>               return -22;
> @@ -2175,6 +2177,21 @@ static int overlayfs_mount(struct bdev *bdev)
>       *upper = '\0';
>       upper++;
>  
> +     // overlayfs.v22 or higher needs workdir option
> +     // if upper is /var/lib/lxc/c2/delta0,
> +     // then workdir is /var/lib/lxc/c2/olwork
> +     lastslash = strrchr(upper, '/');
> +     if (!lastslash)
> +             return -22;
> +     if (strlen(lastslash) < 7)
> +             return -22;

I don't think you want to do this here.  We don't know for
sure that upper ends in /delta0.  And actually you don't need
to do it here, since you are allocating lastslashidx+7 below.

Or am i misreading?

> +     lastslash++;
> +     lastslashidx = lastslash - upper;
> +
> +     work = alloca(lastslashidx + 7);
> +     strncpy(work, upper, lastslashidx+7);
> +     strcpy(work+lastslashidx, "olwork");
> +
>       if (parse_mntopts(bdev->mntopts, &mntflags, &mntdata) < 0) {
>               free(mntdata);
>               return -22;
> @@ -2187,21 +2204,44 @@ 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;
>       }
>  
> +     // mount without workdir option for overlayfs before v21
>       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) {
> +             INFO("overlayfs: error mounting %s onto %s options %s. retry 
> with workdir",
>                       lower, bdev->dest, options);
> +
> +             // retry with workdir option for overlayfs v22 and higher
> +             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);
> @@ -2266,6 +2306,7 @@ static int overlayfs_clonepaths(struct bdev *orig, 
> struct bdev *new, const char
>  
>       if (strcmp(orig->type, "dir") == 0) {
>               char *delta, *lastslash;
> +             char *work;
>               int ret, len, lastslashidx;
>  
>               // if we have /var/lib/lxc/c2/rootfs, then delta will be
> @@ -2291,6 +2332,25 @@ 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);
>  
> +             // make workdir for overlayfs.v22 or higher
> +             // workdir is /var/lib/lxc/c2/olwork
> +             // it is used to prepare files before atomically swithing with 
> destination,
> +             // and needs to be on the same filesystem as upperdir,
> +             // so it's OK for it to be empty.
> +             work = malloc(lastslashidx + 7);
> +             if (!work)
> +                     return -1;
> +             strncpy(work, new->dest, lastslashidx+1);
> +             strcpy(work+lastslashidx, "olwork");
> +             if ((ret = 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);
> @@ -2307,7 +2367,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;
> @@ -2331,6 +2391,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);
> +
> +             // make workdir for overlayfs.v22 or higher
> +             // for details, see above.
> +             work = strdup(ndelta);
> +             if (!work)
> +                     return -1;
> +             strcpy(&work[strlen(work)-6], "olwork");

We can't be certain here that ndelta ends in 'delta0'.  It ends in whatever
odelta ended in.  So you need to do the strrchr('/') on ndelta and allocate
7 chars more than that.

> +             if (mkdir(work, 0755) < 0) {
> +                     SYSERROR("error: mkdir %s", work);
> +                     free(work);
> +                     return -1;
> +             }
> +             if (am_unpriv() && chown_mapped_root(ndelta, conf) < 0)

You're calling chown_mapped_root on ndelta, not on work

> +                     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