On Wed, Dec 06, 2023 at 12:29:54PM +0200, Amir Goldstein wrote:
> On Wed, Dec 6, 2023 at 10:37 AM Zorro Lang <zlang@xxxxxxxxxx> wrote:
> >
> > On Mon, Dec 04, 2023 at 08:58:57PM +0200, Amir Goldstein wrote:
> > > In preparation to forking tests for new lowerdir+,datadir+ mount options,
> > > prepare a helper to test kernel support and pass datadirs into mount
> > > helpers in overlay/079 test.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > ---
> > > common/overlay | 15 +++++++++++++++
> > > tests/overlay/079 | 36 +++++++++++++++++++++---------------
> > > 2 files changed, 36 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/common/overlay b/common/overlay
> > > index 8f275228..ea1eb7b1 100644
> > > --- a/common/overlay
> > > +++ b/common/overlay
> > > @@ -247,6 +247,21 @@ _require_scratch_overlay_lowerdata_layers()
> > > _scratch_unmount
> > > }
> > >
> > > +# Check kernel support for lowerdir+=<lowerdir>,datadir+=<lowerdatadir> format
> > > +_require_scratch_overlay_lowerdir_add_layers()
> > > +{
> > > + local lowerdir="$OVL_BASE_SCRATCH_MNT/$OVL_UPPER"
> > > + local datadir="$OVL_BASE_SCRATCH_MNT/$OVL_LOWER"
> > > +
> > > + _scratch_mkfs > /dev/null 2>&1
> > > + $MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
> > > + -o"lowerdir+=$lowerdir,datadir+=$datadir" \
> > > + -o"redirect_dir=follow,metacopy=on" > /dev/null 2>&1 || \
> > > + _notrun "overlay lowerdir+,datadir+ not supported on ${SCRATCH_DEV}"
> >
> > Hi Amir,
> >
> > I found overlay cases don't use helpers in common/overlay recently, always
> > use raw $MOUNT_PROG directly (not only in this patchset). Although overlay
> > supports new mount format, can we improve the mount helpers in common/overlay
> > to support that? It would be to good to use common helpers to do common
> > operation.
> >
> > Anyway, that can be changed in another patch, if it takes too much time or
> > you don't want to do it at here. What do you think?
>
> I agree. I wouldn't improve the existing helpers to support the new
> lowerdir+,datadir+ options as positional argument like in
> _overlay_scratch_mount_dirs(), but there is an opportunity to reduce
> dedupe of this common line with a helper:
>
> # Mount with mnt/dev of scratch mount and custom mount options
> _overlay_scratch_mount_opts()
> {
> $MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT $*
> }
>
> I will work on this cleanup and post a patch when I get to it.
> No need to block this series for the cleanup.
Agree, thanks for doing this!
>
> Thanks,
> Amir.
>
