On Mon, Jun 24, 2024 at 01:33:55PM +0000, Daniel Gomez wrote:
> On Mon, Jun 17, 2024 at 03:06:54PM GMT, Zorro Lang wrote:
> > On Fri, Jun 14, 2024 at 06:17:25AM +0000, Daniel Gomez wrote:
> > > Add support for test device recreation (RECREATE_TEST_DEV=true) for
> > > tmpfs. This allows to inherit the tmpfs profile extra mount options
> > > (TMPFS_MOUNT_OPTIONS) for the test device.
> > >
> > > Signed-off-by: Daniel Gomez <da.gomez@xxxxxxxxxxx>
> > > ---
> > > common/rc | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index 163041fea..7f995b0fa 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -457,7 +457,7 @@ _test_mount()
> > > fi
> > >
> > > _test_options mount
> > > - _mount -t $FSTYP$FUSE_SUBTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
> > > + _mount -t $FSTYP$FUSE_SUBTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $TMPFS_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
> >
> > We shouldn't use a FSTYP specific parameter in a common mount line.
> > Better to set it in a "$FSTYP=tmpfs" branch. I notice that we set:
> >
> > tmpfs)
> > # We need to specify the size at mount, use 1G by default
> > echo "-o size=1G $TMPFS_MOUNT_OPTIONS"
> >
> > in _common_mount_opts(). Then it will be set in MOUNT_OPTIONS and
> > TEST_FS_MOUNT_OPTS, won't that help?
>
> No for the recreation path with specific section variables as we have not yet
> source '. common/rc'. Dropping changes in _test_mount() and force reading test
> mount options makes $TMPFS_MOUNT_OPTION available for the test in the recreation
> path.
>
> diff --git a/check b/check
> index 723a52e30..b3f7d7b14 100755
> --- a/check
> +++ b/check
> @@ -773,6 +773,7 @@ function run_section()
> status=1
> exit
> fi
> + _test_mount_opts
Actually the get_next_config function (from common/config) should have reloaded
the mount and mkfs paramters.
run_section()
{
...
get_next_config $section
...
if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
...
if ! _test_mkfs >$tmp.err 2>&1
...
if ! _test_mount
...
}
But why it doesn't?
Oh, due to the get_next_config does things as this:
get_next_config() {
...
unset MOUNT_OPTIONS
unset TEST_FS_MOUNT_OPTS
unset MKFS_OPTIONS
unset FSCK_OPTIONS
...
if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then
[ -z "$MOUNT_OPTIONS" ] && _mount_opts
[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
[ -z "$FSCK_OPTIONS" ] && _fsck_opts
...
else
[ -z "$MOUNT_OPTIONS" ] && export MOUNT_OPTIONS=$OLD_MOUNT_OPTIONS
[ -z "$TEST_FS_MOUNT_OPTS" ] && export TEST_FS_MOUNT_OPTS=$OLD_TEST_FS_MOUNT_OPTS
[ -z "$MKFS_OPTIONS" ] && export MKFS_OPTIONS=$OLD_MKFS_OPTIONS
[ -z "$FSCK_OPTIONS" ] && export FSCK_OPTIONS=$OLD_FSCK_OPTIONS
[ -z "$USE_EXTERNAL" ] && export USE_EXTERNAL=$OLD_USE_EXTERNAL
fi
...
}
Only if the later FSTYP is different with the former one, then get_next_config
reloads those OPTIONS.
1) That helps you don't need to set duplicated paramters in each section.
2) But that causes you can't reset/reload something likes TEST_FS_MOUNT_OPTS,
except you set MOUNT_OPTIONS, TEST_FS_MOUNT_OPTS, ... directly.
I'm not sure how many people depends on the 1)st behavior long time. And how
many people hope to remove the limitation of [ $OLD_FSTYP != $FSTYP ]. Any change
will be default behavior change.
But we might can bring the $RECREATE_TEST_DEV into get_next_config(), likes:
if $RECREATE_TEST_DEV || ([ -n "$OLD_FSTYP" ] && [ "$OLD_FSTYP" != "$FSTYP" ])
I didn't give it a test, any better ideas are welcome :)
Thanks,
Zorro
> if ! _test_mount
> then
> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>
> I understand from Darrick's comment that the change above in _test_mount() is
> wrong. Would this solution be okay? Not sure if this is a bug but I'd guess all
> specific mount options are now ignored in the recreation path (not only tmpfs).
>
> >
> > Thanks,
> > Zorro
> >
> >
> > > mount_ret=$?
> > > [ $mount_ret -ne 0 ] && return $mount_ret
> > > _idmapped_mount $TEST_DEV $TEST_DIR
> > > @@ -604,6 +604,9 @@ _test_mkfs()
> > > pvfs2)
> > > # do nothing for pvfs2
> > > ;;
> > > + tmpfs)
> > > + # do nothing for tmpfs
> > > + ;;
> > > udf)
> > > $MKFS_UDF_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null
> > > ;;
> > > --
> > > 2.43.0
> > >
> >
>
