From: Su Yue <l@damenly.org>
To: Brian Foster <bfoster@redhat.com>
Cc: Su Yue <glass.su@suse.com>,
fstests@vger.kernel.org, linux-bcachefs@vger.kernel.org,
l@damenly.org
Subject: Re: [PATCH] fstests: introduce MKFS_BCACHEFS_PROG for bcachefs
Date: Wed, 17 Jan 2024 10:46:01 +0800 [thread overview]
Message-ID: <le8o65d2.fsf@damenly.org> (raw)
In-Reply-To: <ZaaGkadipvNY02HM@bfoster>
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
>>
>>
prev parent reply other threads:[~2024-01-17 2:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-14 4:41 [PATCH] fstests: introduce MKFS_BCACHEFS_PROG for bcachefs Su Yue
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 [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=le8o65d2.fsf@damenly.org \
--to=l@damenly.org \
--cc=bfoster@redhat.com \
--cc=fstests@vger.kernel.org \
--cc=glass.su@suse.com \
--cc=linux-bcachefs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).