[PATCH] btrfs-progs: output sectorsize related warning message into stdout

[PATCH] btrfs-progs: output sectorsize related warning message into stdout

[Qu Wenruo]

Since commit 90020a760584 ("btrfs-progs: mkfs: refactor how we handle
sectorsize override") we have extra warning message if the sectorsize of
mkfs doesn't match page size.

But this warning is show as stderr, which makes a lot of fstests cases
failure due to golden output mismatch.

Fix this by manually output the warning message as stdout.

This is just a temporary fix, a proper fix would needs kernel
/sys/fs/btrfs/features/supported_rw_sectorsize interface to do proper
prompt.

Fixes: 90020a760584 ("btrfs-progs: mkfs: refactor how we handle sectorsize override")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/fsfeatures.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/common/fsfeatures.c b/common/fsfeatures.c
index 569208a9e5b1..f9492e30d57a 100644
--- a/common/fsfeatures.c
+++ b/common/fsfeatures.c
@@ -341,8 +341,16 @@ int btrfs_check_sectorsize(u32 sectorsize)
 		return -EINVAL;
 	}
 	if (page_size != sectorsize)
-		warning(
-"the filesystem may not be mountable, sectorsize %u doesn't match page size %u",
+		/*
+		 * warning() will output message into stderr, which will screw
+		 * up a lot of golden output of fstests. So here we use
+		 * printf().
+		 *
+		 * This will be replaced by proper supported rw/ro sector size
+		 * detection with kernel change in the future.
+		 */
+		printf(
+"WARNING: the filesystem may not be mountable, sectorsize %u doesn't match page size %u\n",
 			sectorsize, page_size);
 	return 0;
 }
-- 
2.30.1

Re: [PATCH] btrfs-progs: output sectorsize related warning message into stdout

[David Sterba]

On Tue, Mar 09, 2021 at 03:39:09PM +0800, Qu Wenruo wrote:
> Since commit 90020a760584 ("btrfs-progs: mkfs: refactor how we handle
> sectorsize override") we have extra warning message if the sectorsize of
> mkfs doesn't match page size.
> 
> But this warning is show as stderr, which makes a lot of fstests cases
> failure due to golden output mismatch.

Well, no. Using message helpers in progs is what we want to do
everywhere, working around fstests output matching design is fixing the
problem in the wrong place. That this is fragile has been is known and
I want to keep the liberty to adjust output in progs as users need, not
as fstests require.

I can compare two different approaches of testsuites, fstests vs
btrfs-progs and I can say that the number of false positives on fstests
side was much higher and actually making the whole testing experience
much worse, up to ignoring test failures because they were not failures
at all because the tests are not robust enoughh. In btrfs-progs there
have been like 1 or 2 silent test breakages (unexpected pass) but that
led to stronger checks of the expected or unexpected output.

Re: [PATCH] btrfs-progs: output sectorsize related warning message into stdout

[Qu Wenruo]

On 2021/3/9 下午9:33, David Sterba wrote:
> On Tue, Mar 09, 2021 at 03:39:09PM +0800, Qu Wenruo wrote:
>> Since commit 90020a760584 ("btrfs-progs: mkfs: refactor how we handle
>> sectorsize override") we have extra warning message if the sectorsize of
>> mkfs doesn't match page size.
>>
>> But this warning is show as stderr, which makes a lot of fstests cases
>> failure due to golden output mismatch.
>
> Well, no. Using message helpers in progs is what we want to do
> everywhere, working around fstests output matching design is fixing the
> problem in the wrong place. That this is fragile has been is known and
> I want to keep the liberty to adjust output in progs as users need, not
> as fstests require.

OK, then I guess the best way to fix the problem is to add sysfs
interface to export supported rw/ro sectorsize.

It shouldn't be that complex and would be small enough for next merge
window.

Thanks,
Qu
>
> I can compare two different approaches of testsuites, fstests vs
> btrfs-progs and I can say that the number of false positives on fstests
> side was much higher and actually making the whole testing experience
> much worse, up to ignoring test failures because they were not failures
> at all because the tests are not robust enoughh. In btrfs-progs there
> have been like 1 or 2 silent test breakages (unexpected pass) but that
> led to stronger checks of the expected or unexpected output.
>

Re: [PATCH] btrfs-progs: output sectorsize related warning message into stdout

[David Sterba]

On Wed, Mar 10, 2021 at 08:18:16AM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/3/9 下午9:33, David Sterba wrote:
> > On Tue, Mar 09, 2021 at 03:39:09PM +0800, Qu Wenruo wrote:
> >> Since commit 90020a760584 ("btrfs-progs: mkfs: refactor how we handle
> >> sectorsize override") we have extra warning message if the sectorsize of
> >> mkfs doesn't match page size.
> >>
> >> But this warning is show as stderr, which makes a lot of fstests cases
> >> failure due to golden output mismatch.
> >
> > Well, no. Using message helpers in progs is what we want to do
> > everywhere, working around fstests output matching design is fixing the
> > problem in the wrong place. That this is fragile has been is known and
> > I want to keep the liberty to adjust output in progs as users need, not
> > as fstests require.
> 
> OK, then I guess the best way to fix the problem is to add sysfs
> interface to export supported rw/ro sectorsize.
> 
> It shouldn't be that complex and would be small enough for next merge
> window.

The subpage support should be advertised somewhere in sysfs so the range
of supported sector sizes sounds like a good idea.