* Re: [PATCH] fstests: introduce MKFS_BCACHEFS_PROG for bcachefs
2024-01-16 13:37 ` Brian Foster
@ 2024-01-16 15:20 ` Su Yue
2024-01-16 17:07 ` Kent Overstreet
2024-01-17 2:46 ` Su Yue
2 siblings, 0 replies; 5+ messages in thread
From: Su Yue @ 2024-01-16 15:20 UTC (permalink / raw)
To: Brian Foster; +Cc: Su Yue, fstests, linux-bcachefs, l
On Tue 16 Jan 2024 at 08:37, Brian Foster <bfoster@redhat.com>
wrote:
> On Sun, Jan 14, 2024 at 12:41:20PM +0800, Su Yue wrote:
>> mkfs.bcachefs supports force overwrite when option '-f' is
>> given:
>> $ mkfs.bcachefs --help | grep force
>> -f, --force
>> After this commit, MKFS_BCACHEFS_PROG will contains ' -f' so
>> we don't have to add '-f' to $MKFS_OPTIONS manually.
>>
>> It also fixes generic/466 which unsets MKFS_OPTIONS causing
>> that test hangs in mfks.bcachefs waiting for confirmation of
>> the force overwrite.
>>
>
> This seems mostly reasonable to me, but I'm kind of wondering
> why I
> haven't had to add -f to my MKFS_OPTIONS for bcachefs. Is there
> something unique to the test environment that affects this?
> I.e., I
> don't see any issue with generic/466 on a recent Fedora, even if
> I
> pre-format the scratch dev with XFS.
>
My bad. Something wrong happend during my test process and caused
incorrect memory. fstests call wipefs to erase scratch dev first.
I will remove the descirption about the
'have to add '-f' to $MKFS_OPTIONS in next version.
>
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> common/config | 3 ++-
>> common/rc | 12 +++++++++---
>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/common/config b/common/config
>> index c9771ff934cb..1f9edceec57a 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -105,7 +105,7 @@ set_mkfs_prog_path_with_opts()
>> # Note: mkfs.f2fs doesn't support the --help option yet,
>> but it doesn't
>> # matter since it also prints the help when an invalid
>> option is given.
>> if [ "$p" != "" ] && \
>> - $p --help |& grep -q "[[:space:]]-f[[:space:]|]"; then
>> + $p --help |& grep -q "[[:space:]]-f[[:space:]|,]";
>> then
>> echo "$p -f"
>> else
>> echo $p
>> @@ -313,6 +313,7 @@ export MKFS_REISER4_PROG=$(type -P
>> mkfs.reiser4)
>> export E2FSCK_PROG=$(type -P e2fsck)
>> export TUNE2FS_PROG=$(type -P tune2fs)
>> export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
>> +export MKFS_BCACHEFS_PROG=$(set_mkfs_prog_path_with_opts
>> bcachefs)
>>
>> # SELinux adds extra xattrs which can mess up our expected
>> output.
>> # So, mount with a context, and they won't be created.
>> diff --git a/common/rc b/common/rc
>> index cc92fe0681d6..f2e900bf1166 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -611,6 +611,9 @@ _test_mkfs()
>> xfs)
>> $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* $TEST_DEV
>> ;;
>> + bcachefs)
>> + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null
>> + ;;
>> *)
>> yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV
>> ;;
>> @@ -753,6 +756,10 @@ _scratch_mkfs()
>> mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
>> mkfs_filter="grep -v -e ^mkfs\.ocfs2"
>> ;;
>> + bcachefs)
>> + mkfs_cmd="$MKFS_BCACHEFS_PROG"
>> + mkfs_filter="cat"
>> + ;;
>> *)
>> mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
>> mkfs_filter="cat"
>> @@ -1044,7 +1051,7 @@ _scratch_mkfs_sized()
>> export MOUNT_OPTIONS="-o size=$fssize
>> $TMPFS_MOUNT_OPTIONS"
>> ;;
>> bcachefs)
>> - $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS
>> --fs_size=$fssize --block_size=$blocksize $SCRATCH_DEV
>> + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize
>> --block_size=$blocksize $SCRATCH_DEV
>
> Should this function have another bcachefs check at the top to
> set
> def_blksize in the case the config specifies a blocksize to use?
>
Yeah. Will add it.
> Brian
>
>> ;;
>> *)
>> _notrun "Filesystem $FSTYP not supported in
>> _scratch_mkfs_sized"
>> @@ -1128,8 +1135,7 @@ _scratch_mkfs_blocksized()
>> -C $blocksize $SCRATCH_DEV
>> ;;
>> bcachefs)
>> - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS
>> --block_size=$blocksize \
>> - $SCRATCH_DEV
>> + _scratch_mkfs --block_size=$blocksize
>> ;;
>> udf)
>> _scratch_mkfs -b $blocksize
>> --
>> 2.43.0
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] fstests: introduce MKFS_BCACHEFS_PROG for bcachefs
2024-01-16 13:37 ` Brian Foster
2024-01-16 15:20 ` Su Yue
@ 2024-01-16 17:07 ` Kent Overstreet
2024-01-17 2:46 ` Su Yue
2 siblings, 0 replies; 5+ messages in thread
From: Kent Overstreet @ 2024-01-16 17:07 UTC (permalink / raw)
To: Brian Foster; +Cc: Su Yue, fstests, linux-bcachefs, l
On Tue, Jan 16, 2024 at 08:37:21AM -0500, Brian Foster wrote:
> On Sun, Jan 14, 2024 at 12:41:20PM +0800, Su Yue wrote:
> > mkfs.bcachefs supports force overwrite when option '-f' is given:
> > $ mkfs.bcachefs --help | grep force
> > -f, --force
> > After this commit, MKFS_BCACHEFS_PROG will contains ' -f' so
> > we don't have to add '-f' to $MKFS_OPTIONS manually.
> >
> > It also fixes generic/466 which unsets MKFS_OPTIONS causing
> > that test hangs in mfks.bcachefs waiting for confirmation of
> > the force overwrite.
> >
>
> This seems mostly reasonable to me, but I'm kind of wondering why I
> haven't had to add -f to my MKFS_OPTIONS for bcachefs. Is there
> something unique to the test environment that affects this? I.e., I
> don't see any issue with generic/466 on a recent Fedora, even if I
> pre-format the scratch dev with XFS.
I think there's still multiple screwy things going on with blkid probing
and superblock wiping - I've been noticing that there's a bunch of ktest
tests that don't specify -f and work fine in the CI, but that fail when
I loop them locally.
It appears wipefs only zeroes out the magic number on our first
superblock - but it leaves the layout intact before the first
superblock, so we're still able to find our backup superblocks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fstests: introduce MKFS_BCACHEFS_PROG for bcachefs
2024-01-16 13:37 ` Brian Foster
2024-01-16 15:20 ` Su Yue
2024-01-16 17:07 ` Kent Overstreet
@ 2024-01-17 2:46 ` Su Yue
2 siblings, 0 replies; 5+ messages in thread
From: Su Yue @ 2024-01-17 2:46 UTC (permalink / raw)
To: Brian Foster; +Cc: Su Yue, fstests, linux-bcachefs, l
On Tue 16 Jan 2024 at 08:37, Brian Foster <bfoster@redhat.com>
wrote:
> On Sun, Jan 14, 2024 at 12:41:20PM +0800, Su Yue wrote:
>> mkfs.bcachefs supports force overwrite when option '-f' is
>> given:
>> $ mkfs.bcachefs --help | grep force
>> -f, --force
>> After this commit, MKFS_BCACHEFS_PROG will contains ' -f' so
>> we don't have to add '-f' to $MKFS_OPTIONS manually.
>>
>> It also fixes generic/466 which unsets MKFS_OPTIONS causing
>> that test hangs in mfks.bcachefs waiting for confirmation of
>> the force overwrite.
>>
>
> This seems mostly reasonable to me, but I'm kind of wondering
> why I
> haven't had to add -f to my MKFS_OPTIONS for bcachefs. Is there
> something unique to the test environment that affects this?
> I.e., I
> don't see any issue with generic/466 on a recent Fedora, even if
> I
> pre-format the scratch dev with XFS.
>
I was running tests of group auto. There are some tests which
call _scratch_mkfs multiple times e.g. tests/generic/171.
Without '-f' in MKFS_OPTIONS, thoese tests hangs.
I will make it clearer in commit message.
--
Su
>> Signed-off-by: Su Yue <glass.su@suse.com>
>> ---
>> common/config | 3 ++-
>> common/rc | 12 +++++++++---
>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/common/config b/common/config
>> index c9771ff934cb..1f9edceec57a 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -105,7 +105,7 @@ set_mkfs_prog_path_with_opts()
>> # Note: mkfs.f2fs doesn't support the --help option yet,
>> but it doesn't
>> # matter since it also prints the help when an invalid
>> option is given.
>> if [ "$p" != "" ] && \
>> - $p --help |& grep -q "[[:space:]]-f[[:space:]|]"; then
>> + $p --help |& grep -q "[[:space:]]-f[[:space:]|,]";
>> then
>> echo "$p -f"
>> else
>> echo $p
>> @@ -313,6 +313,7 @@ export MKFS_REISER4_PROG=$(type -P
>> mkfs.reiser4)
>> export E2FSCK_PROG=$(type -P e2fsck)
>> export TUNE2FS_PROG=$(type -P tune2fs)
>> export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
>> +export MKFS_BCACHEFS_PROG=$(set_mkfs_prog_path_with_opts
>> bcachefs)
>>
>> # SELinux adds extra xattrs which can mess up our expected
>> output.
>> # So, mount with a context, and they won't be created.
>> diff --git a/common/rc b/common/rc
>> index cc92fe0681d6..f2e900bf1166 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -611,6 +611,9 @@ _test_mkfs()
>> xfs)
>> $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* $TEST_DEV
>> ;;
>> + bcachefs)
>> + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null
>> + ;;
>> *)
>> yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV
>> ;;
>> @@ -753,6 +756,10 @@ _scratch_mkfs()
>> mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
>> mkfs_filter="grep -v -e ^mkfs\.ocfs2"
>> ;;
>> + bcachefs)
>> + mkfs_cmd="$MKFS_BCACHEFS_PROG"
>> + mkfs_filter="cat"
>> + ;;
>> *)
>> mkfs_cmd="yes | $MKFS_PROG -t $FSTYP --"
>> mkfs_filter="cat"
>> @@ -1044,7 +1051,7 @@ _scratch_mkfs_sized()
>> export MOUNT_OPTIONS="-o size=$fssize
>> $TMPFS_MOUNT_OPTIONS"
>> ;;
>> bcachefs)
>> - $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS
>> --fs_size=$fssize --block_size=$blocksize $SCRATCH_DEV
>> + $MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize
>> --block_size=$blocksize $SCRATCH_DEV
>
> Should this function have another bcachefs check at the top to
> set
> def_blksize in the case the config specifies a blocksize to use?
>
> Brian
>
>> ;;
>> *)
>> _notrun "Filesystem $FSTYP not supported in
>> _scratch_mkfs_sized"
>> @@ -1128,8 +1135,7 @@ _scratch_mkfs_blocksized()
>> -C $blocksize $SCRATCH_DEV
>> ;;
>> bcachefs)
>> - ${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS
>> --block_size=$blocksize \
>> - $SCRATCH_DEV
>> + _scratch_mkfs --block_size=$blocksize
>> ;;
>> udf)
>> _scratch_mkfs -b $blocksize
>> --
>> 2.43.0
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread