Hi,
Thank you for your comment. I will send fixed patch.
>>> On Wed, 29 Oct 2014 13:56:45 +0000
in message "Re: [lxc-devel] [PATCH RFC] overlayfs: overlayfs.v22 or
higher needs workdir option"
Serge Hallyn-san wrote:
> 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
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
