| Message ID | 20210309073909.74043-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: output sectorsize related warning message into stdout | expand |
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.
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. >
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.
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; }
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(-)