On Mon, Jan 22, 2024 at 1:52 PM Alexander Larsson <alexl@xxxxxxxxxx> wrote:
>
> On Mon, 2024-01-22 at 13:09 +0200, Amir Goldstein wrote:
> > On Mon, Jan 22, 2024 at 12:14 PM Alexander Larsson <alexl@xxxxxxxxxx>
> > wrote:
> > >
> > > On Sun, 2024-01-21 at 17:05 +0200, Amir Goldstein wrote:
> > > > An opaque directory cannot have xwhiteouts, so instead of marking
> > > > an
> > > > xwhiteouts directory with a new xattr, overload overlay.opaque
> > > > xattr
> > > > for marking both opaque dir ('y') and xwhiteouts dir ('x').
> > > >
> > > > This is more efficient as the overlay.opaque xattr is checked
> > > > during
> > > > lookup of directory anyway.
> > > >
> > > > This also prevents unnecessary checking the xattr when reading a
> > > > directory without xwhiteouts, i.e. most of the time.
> > > >
> > > > Note that the xwhiteouts marker is not checked on the upper layer
> > > > and
> > > > on the last layer in lowerstack, where xwhiteouts are not
> > > > expected.
> > > >
> > > > Fixes: bc8df7a3dc03 ("ovl: Add an alternative type of whiteout")
> > > > Cc: <stable@xxxxxxxxxxxxxxx> # v6.7
> > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > > ---
> > > >
> > > > Miklos,
> > > >
> > > > Alex has reported a problem with your suggested approach of
> > > > requiring
> > > > xwhiteouts xattr on layers root dir [1].
> > > >
> > > > Following counter proposal, amortizes the cost of checking opaque
> > > > xattr
> > > > on directories during lookup to also check for xwhiteouts.
> > > >
> > > > This change requires the following change to test overlay/084:
> > > >
> > > > --- a/tests/overlay/084
> > > > +++ b/tests/overlay/084
> > > > @@ -115,7 +115,8 @@ do_test_xwhiteout()
> > > >
> > > > mkdir -p $basedir/lower $basedir/upper $basedir/work
> > > > touch $basedir/lower/regular $basedir/lower/hidden
> > > > $basedir/upper/hidden
> > > > - setfattr -n $prefix.overlay.whiteouts -v "y"
> > > > $basedir/upper
> > > > + # overlay.opaque="x" means directory has xwhiteout
> > > > children
> > > > + setfattr -n $prefix.overlay.opaque -v "x" $basedir/upper
> > > > setfattr -n $prefix.overlay.whiteout -v "y"
> > > > $basedir/upper/hidden
> > > >
> > > >
> > > > Alex,
> > > >
> > > > Please let us know if this change is acceptable for composefs.
> > >
> > > Yes, this looks very good to me. (Minor comments below)
> > > I'll do some testing on this.
> > >
> >
> > Excellent, I'll be expecting your RVB/Tested-by.
> > >
>
> Yes
> Reviewed-by: Alexander Larsson <alexl@xxxxxxxxxx>
> Tested-by: Alexander Larsson <alexl@xxxxxxxxxx>
>
> for the patch in the ovl-fixes branch.
Thanks. pushed.
>
> I tested it manually, and with xfstest (with change), and also
> with this composefs change:
>
> https://github.com/alexlarsson/composefs/tree/new-format-version
>
> I created a lowerdir with a regular whiteout in, and after running that
> though the changed mkcomposefs I was able to mount the composefs image,
> and then mount the lowerdirs from the composefs mount, and they
> correctly handled the whiteout both when mounted normally and with
> userxattr.
>
I noticed you comment in composefs:
* 1 - Mark xwhitouts using the new opaque=x format as needed by Linux 6.8
Note that this "fix" is aimed to be backported to v6.7.y, so there is no kernel
version that is expected to retain support for the old format.
Thanks,
Amir.
