diff mbox series

[12/12] xfs/157: fix test failures when MKFS_OPTIONS has -L options

Message ID 173197064609.904310.7896567442225446738.stgit@frogsfrogsfrogs (mailing list archive)
State New, archived
Headers show
Series [01/12] generic/757: fix various bugs in this test | expand

Commit Message

Darrick J. Wong Nov. 18, 2024, 11:04 p.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

Zorro reports that this test fails if the test runner set an -L (label)
option in MKFS_OPTIONS.  Fix the test to work around this with a bunch
of horrid sed filtering magic.  It's probably not *critical* to make
this test test work with random labels, but it'd be nice not to lose
them.

Cc: <fstests@vger.kernel.org> # v2024.10.14
Fixes: 2f7e1b8a6f09b6 ("xfs/157,xfs/547,xfs/548: switch to using _scratch_mkfs_sized")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/157 |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

Comments

Zorro Lang Nov. 21, 2024, 10:17 a.m. UTC | #1
On Mon, Nov 18, 2024 at 03:04:31PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Zorro reports that this test fails if the test runner set an -L (label)
> option in MKFS_OPTIONS.  Fix the test to work around this with a bunch

I didn't hit the xfs/157 failure by setting "-L label" in MKFS_OPTIONS,
I set MKFS_OPTIONS="-m rmapbt=1" in local.config, then "-m rmapbt=1" is
conflict with rtdev, that cause the "-L oldlabel" be dropped by
_scratch_mkfs_sized.

I don't mind having this "xfs/157 enhancement" patch. But as we've talked,
I don't think any testers would like to write MKFS_OPTIONS="-L label" in
local.config. So this patch might not be necessary. What do you think?

Thanks,
Zorro

> of horrid sed filtering magic.  It's probably not *critical* to make
> this test test work with random labels, but it'd be nice not to lose
> them.
> 
> Cc: <fstests@vger.kernel.org> # v2024.10.14
> Fixes: 2f7e1b8a6f09b6 ("xfs/157,xfs/547,xfs/548: switch to using _scratch_mkfs_sized")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/157 |   29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/tests/xfs/157 b/tests/xfs/157
> index e102a5a10abe4b..0c21786e389695 100755
> --- a/tests/xfs/157
> +++ b/tests/xfs/157
> @@ -65,9 +65,34 @@ scenario() {
>  	SCRATCH_RTDEV=$orig_rtdev
>  }
>  
> +extract_mkfs_label() {
> +	set -- $MKFS_OPTIONS
> +	local in_l
> +
> +	for arg in "$@"; do
> +		if [ "$in_l" = "1" ]; then
> +			echo "$arg"
> +			return 0
> +		elif [ "$arg" = "-L" ]; then
> +			in_l=1
> +		fi
> +	done
> +	return 1
> +}
> +
>  check_label() {
> -	_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
> -	_scratch_xfs_db -c label
> +	local existing_label
> +	local filter
> +
> +	# Handle -L somelabel being set in MKFS_OPTIONS
> +	if existing_label="$(extract_mkfs_label)"; then
> +		filter=(sed -e "s|$existing_label|oldlabel|g")
> +		_scratch_mkfs_sized $fs_size >> $seqres.full
> +	else
> +		filter=(cat)
> +		_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
> +	fi
> +	_scratch_xfs_db -c label | "${filter[@]}"
>  	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
>  	_scratch_xfs_db -c label
>  	_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
>
Darrick J. Wong Nov. 21, 2024, 5:19 p.m. UTC | #2
On Thu, Nov 21, 2024 at 06:17:12PM +0800, Zorro Lang wrote:
> On Mon, Nov 18, 2024 at 03:04:31PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Zorro reports that this test fails if the test runner set an -L (label)
> > option in MKFS_OPTIONS.  Fix the test to work around this with a bunch
> 
> I didn't hit the xfs/157 failure by setting "-L label" in MKFS_OPTIONS,
> I set MKFS_OPTIONS="-m rmapbt=1" in local.config, then "-m rmapbt=1" is
> conflict with rtdev, that cause the "-L oldlabel" be dropped by
> _scratch_mkfs_sized.
> 
> I don't mind having this "xfs/157 enhancement" patch. But as we've talked,
> I don't think any testers would like to write MKFS_OPTIONS="-L label" in
> local.config. So this patch might not be necessary. What do you think?

Yeah, I guess I will drop it then.

--D

> Thanks,
> Zorro
> 
> > of horrid sed filtering magic.  It's probably not *critical* to make
> > this test test work with random labels, but it'd be nice not to lose
> > them.
> > 
> > Cc: <fstests@vger.kernel.org> # v2024.10.14
> > Fixes: 2f7e1b8a6f09b6 ("xfs/157,xfs/547,xfs/548: switch to using _scratch_mkfs_sized")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/157 |   29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/tests/xfs/157 b/tests/xfs/157
> > index e102a5a10abe4b..0c21786e389695 100755
> > --- a/tests/xfs/157
> > +++ b/tests/xfs/157
> > @@ -65,9 +65,34 @@ scenario() {
> >  	SCRATCH_RTDEV=$orig_rtdev
> >  }
> >  
> > +extract_mkfs_label() {
> > +	set -- $MKFS_OPTIONS
> > +	local in_l
> > +
> > +	for arg in "$@"; do
> > +		if [ "$in_l" = "1" ]; then
> > +			echo "$arg"
> > +			return 0
> > +		elif [ "$arg" = "-L" ]; then
> > +			in_l=1
> > +		fi
> > +	done
> > +	return 1
> > +}
> > +
> >  check_label() {
> > -	_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
> > -	_scratch_xfs_db -c label
> > +	local existing_label
> > +	local filter
> > +
> > +	# Handle -L somelabel being set in MKFS_OPTIONS
> > +	if existing_label="$(extract_mkfs_label)"; then
> > +		filter=(sed -e "s|$existing_label|oldlabel|g")
> > +		_scratch_mkfs_sized $fs_size >> $seqres.full
> > +	else
> > +		filter=(cat)
> > +		_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
> > +	fi
> > +	_scratch_xfs_db -c label | "${filter[@]}"
> >  	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
> >  	_scratch_xfs_db -c label
> >  	_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"
> > 
> 
>
diff mbox series

Patch

diff --git a/tests/xfs/157 b/tests/xfs/157
index e102a5a10abe4b..0c21786e389695 100755
--- a/tests/xfs/157
+++ b/tests/xfs/157
@@ -65,9 +65,34 @@  scenario() {
 	SCRATCH_RTDEV=$orig_rtdev
 }
 
+extract_mkfs_label() {
+	set -- $MKFS_OPTIONS
+	local in_l
+
+	for arg in "$@"; do
+		if [ "$in_l" = "1" ]; then
+			echo "$arg"
+			return 0
+		elif [ "$arg" = "-L" ]; then
+			in_l=1
+		fi
+	done
+	return 1
+}
+
 check_label() {
-	_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
-	_scratch_xfs_db -c label
+	local existing_label
+	local filter
+
+	# Handle -L somelabel being set in MKFS_OPTIONS
+	if existing_label="$(extract_mkfs_label)"; then
+		filter=(sed -e "s|$existing_label|oldlabel|g")
+		_scratch_mkfs_sized $fs_size >> $seqres.full
+	else
+		filter=(cat)
+		_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
+	fi
+	_scratch_xfs_db -c label | "${filter[@]}"
 	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
 	_scratch_xfs_db -c label
 	_scratch_xfs_repair -n &>> $seqres.full || echo "Check failed?"