| Message ID | 20240614061722.1080-6-da.gomez@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tmpfs fixes | expand |
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@samsung.com> > --- > 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`" ? Also should these four variables be captured explicitly by the reports that are generated by common/report ? --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 >
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@samsung.com> > > --- > > 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. 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 > >
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@samsung.com> > > > --- > > > 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. --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 > > >
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@samsung.com> > > > > --- > > > > 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 > > > >
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`" 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
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@samsung.com> --- check | 4 ++++ common/rc | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-)