On Tue, Jul 27, 2021 at 10:15:27AM +0530, Chandan Babu R wrote:
> On 26 Jul 2021 at 22:49, Darrick J. Wong wrote:
> > On Mon, Jul 26, 2021 at 12:13:13PM +0530, Chandan Babu R wrote:
> >> _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?
--D
diff --git a/tests/xfs/530 b/tests/xfs/530
index 4d168ac5..9c6f44d7 100755
--- a/tests/xfs/530
+++ b/tests/xfs/530
@@ -60,10 +60,22 @@ echo "Format and mount rt volume"
export USE_EXTERNAL=yes
export SCRATCH_RTDEV=$rtdev
-_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
+_scratch_mkfs -d size=$((1024 * 1024 * 1024)) \
-r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
_try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
+# If we didn't get the desired realtime volume and the same blocksize as the
+# first format (which we used to compute a specific rt geometry), skip the
+# test. This can happen if the MKFS_OPTIONS conflict with the ones we passed
+# to _scratch_mkfs or do not result in a valid rt fs geometry. In this case,
+# _scratch_mkfs will try to "succeed" at formatting by dropping MKFS_OPTIONS,
+# giving us the wrong geometry.
+formatted_blksz="$(_get_block_size $SCRATCH_MNT)"
+test "$formatted_blksz" -ne "$dbsize" && \
+ _notrun "Tried to format with $dbsize blocksize, got $formatted_blksz."
+$XFS_INFO_PROG $SCRATCH_MNT | egrep -q 'realtime.*blocks=0' && \
+ _notrun "Filesystem should have a realtime volume"
+
echo "Consume free space"
fillerdir=$SCRATCH_MNT/fillerdir
nr_free_blks=$(stat -f -c '%f' $SCRATCH_MNT)
> >
> > Eventually, the plan is to support rmap[1] and reflink[2] on the
> > realtime device, at which point this will have to be torn out and a
> > better solution found.
> >
> > --D
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=realtime-rmap
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=realtime-reflink
> >
>
> --
> chandan
