On Sun, Aug 01, 2021 at 07:41:09PM +0800, Eryu Guan wrote:

On Tue, Jul 27, 2021 at 11:37:34AM -0700, Darrick J. Wong wrote:

quoted

On Tue, Jul 27, 2021 at 10:15:27AM +0530, Chandan Babu R wrote:

quoted

On 26 Jul 2021 at 22:49, Darrick J. Wong wrote:

quoted

On Mon, Jul 26, 2021 at 12:13:13PM +0530, Chandan Babu R wrote:

quoted

_scratch_do_mkfs constructs a mkfs command line by concatenating the values of
1. $mkfs_cmd
2. $MKFS_OPTIONS
3. $extra_mkfs_options

The corresponding mkfs command line fails if $MKFS_OPTIONS enables either
reflink or rmapbt feature. The failure occurs because the test tries to create
a filesystem with realtime device enabled. In such a case, _scratch_do_mkfs()
will construct and invoke an mkfs command line without including the value of
$MKFS_OPTIONS.

To prevent such silent failures, this commit causes the test to exit if it
detects either reflink or rmapbt feature being enabled.

Er, what combinations of mkfs.xfs and MKFS_OPTIONS cause this result?
What kind of fs configuration comes out of that?

With MKFS_OPTIONS set as shown below,

export MKFS_OPTIONS="-m reflink=1 -b size=1k"

_scratch_do_mkfs() invokes mkfs.xfs with both realtime and reflink options
enabled. Such an invocation of mkfs.xfs fails causing _scratch_do_mkfs() to
ignore the contents of $MKFS_OPTIONS while constructing and invoking mkfs.xfs
once again.

This time, the fs block size will however be set to 4k (the default block
size). At the beginning of the test we would have obtained the block size of
the filesystem as 1k and used it to compute the size of the realtime device
required to overflow realtime bitmap inode's max pseudo extent count.

Invocation of xfs_growfs (made later in the test) ends up succeeding since a
4k fs block can accommodate more bits than a 1k fs block.

OK, now I think I've finally put all the pieces together.  Both of these
patches are fixing weirdness when MKFS_OPTIONS="-m reflink=1 -b size=1k".

With current HEAD, we try to mkfs.xfs with double "-b size" arguments.
That fails with 'option respecified', so fstests tries again without
MKFS_OPTIONS, which means you don't get the filesystem that you want.
If, say, MKFS_OPTIONS also contained bigtime=1, you won't get a bigtime
filesystem.

So the first patch removes the double -bsize arguments.  But you still
have the problem that the reflink=1 in MKFS_OPTIONS still causes
mkfs.xfs to fail (because we don't do rt and reflink yet), so fstests
again drops MKFS_OPTIONS, and now you're testing the fs without a block
size option at all.  The test still regresses because the special rt
geometry depends on the blocksize, and we didn't get all the geometry
elements that we need to trip the growfs failure.

Does the following patch fix all that for you?

Do you have plan to post formal patch? I think both problems could be
fixed in one patch like you did. I'll leave patch 1 for now.

Ah, I saw the patch now, thanks!

Eryu