Re: [PATCH v2 2/4] overlay: prepare for new lowerdir+,datadir+ tests

Amir Goldstein
2023-12-06T10:30:07Z

On Wed, Dec 6, 2023 at 10:37 AM Zorro Lang <zlang@redhat.com> 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@gmail.com>
> > ---
> > 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.

Thanks,
Amir.