On Mon, Jun 24, 2024 at 09:24:34AM GMT, Darrick J. Wong wrote:
> On Mon, Jun 24, 2024 at 02:02:33PM +0000, Daniel Gomez wrote:
> > On Fri, Jun 14, 2024 at 08:55:55AM GMT, Darrick J. Wong wrote:
> > > On Fri, Jun 14, 2024 at 06:17:26AM +0000, Daniel Gomez wrote:
> > > > Mount options for a SCRATCH device might not be the same in the TEST
> > > > device if RECREATE_TEST_DEV is not enabled. So, print mount options for
> > > > each device to clarify this.
> > > >
> > > > Add mount and mkfs info for TEST devices so we get the same information
> > > > being printed for both devices.
> > > >
> > > > Signed-off-by: Daniel Gomez <da.gomez@xxxxxxxxxxx>
> > > > ---
> > > > check | 4 ++++
> > > > common/rc | 20 ++++++++++++++++++--
> > > > 2 files changed, 22 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/check b/check
> > > > index 723a52e30..e02d28b39 100755
> > > > --- a/check
> > > > +++ b/check
> > > > @@ -807,7 +807,11 @@ function run_section()
> > > > # print out our test configuration
> > > > echo "FSTYP -- `_full_fstyp_details`"
> > > > echo "PLATFORM -- `_full_platform_details`"
> > > > + echo "TEST device:"
> > > > + echo "MKFS_OPTIONS -- `_test_mkfs_options`"
> > > > + echo "MOUNT_OPTIONS -- `_test_mount_options`"
> > > > if [ ! -z "$SCRATCH_DEV" ]; then
> > > > + echo "SCRATCH device:"
> > > > echo "MKFS_OPTIONS -- `_scratch_mkfs_options`"
> > > > echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
> > >
> > > Now there are two lines that start with "MKFS_OPTIONS"; will this break
> > > anyone's parsing scripts? Or should these be prefixed:
> > >
> > > echo "TEST_MKFS_OPTIONS -- `_test_mkfs_options`"
> > > ...
> > > echo "SCRATCH_MKFS_OPTIONS -- `_scratch_mkfs_options`"
> > >
> > > ?
> >
> > This looks like my initial version, but I prefer the 'sub-menu style' because
> > we did not have these variables before. However, I think it makes sense to
> > introduce them so we can differentiate between them.
>
> The trouble is, if your parsing script were doing something like
> splitting on "-- " then the "TEST device:" string would not split
> properly and you'd have to add context retention of some sort to know
> which "MKFS_OPTIONS" this was for. Better just to namespace the new
> lines from the start. I guess for compatibility's sake then we'd have
> to have:
>
> TEST_MKFS_OPTIONS -- -o foo=bar
> MKFS_OPTIONS -- -o foo=baz
>
> in the output, along with a comment somewhere in the source that the
> non-prefixed scratch mkfs options is that way for hyst[eo]rical output
> parsing reasons.
We are also reporting as MOUNT_OPTIONS the scratch mount options. But
$MOUNT_OPTIONS are just the 'common' options, a subset of the scratch mount
options.
But I agree with only adding information for TEST device.
>
> --D
>
> > I guess introducing this change would break anyone's parsing scripts regardless?
> > However, I do think is necessary to specify the mount differences.
> > >
> > > Also should these four variables be captured explicitly by the reports
> > > that are generated by common/report ?
> >
> > I guess the report only includes the scratch mount options. I will update it to
> > include the new specific test and scratch mount/mkfs options.
> >
> > >
> > > --D
> > >
> > > > fi
> > > > diff --git a/common/rc b/common/rc
> > > > index a42792c37..b351a82eb 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -235,6 +235,17 @@ _scratch_mount_options()
> > > > $SCRATCH_DEV $SCRATCH_MNT
> > > > }
> > > >
> > > > +_test_mount_options()
> > > > +{
> > > > + _test_options mount
> > > > +
> > > > + if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
> > > > + echo $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $TMPFS_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
> > > > + else
> > > > + echo $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
> > > > + fi
> > > > +}
> > > > +
> > > > _supports_filetype()
> > > > {
> > > > local dir=$1
> > > > @@ -456,8 +467,7 @@ _test_mount()
> > > > return $?
> > > > fi
> > > >
> > > > - _test_options mount
> > > > - _mount -t $FSTYP$FUSE_SUBTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $TMPFS_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
> > > > + _mount -t $FSTYP$FUSE_SUBTYP `_test_mount_options $*`
> > > > mount_ret=$?
> > > > [ $mount_ret -ne 0 ] && return $mount_ret
> > > > _idmapped_mount $TEST_DEV $TEST_DIR
> > > > @@ -571,6 +581,12 @@ _metadump_dev() {
> > > > esac
> > > > }
> > > >
> > > > +_test_mkfs_options()
> > > > +{
> > > > + _test_options mkfs
> > > > + echo $TEST_OPTIONS $MKFS_OPTIONS $* $TEST_DEV
> > > > +}
> > > > +
> > > > _test_mkfs()
> > > > {
> > > > case $FSTYP in
> > > > --
> > > > 2.43.0
> > > >
