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