On Mon, Nov 04, 2024 at 03:34:26PM -0800, Darrick J. Wong wrote: > On Mon, Nov 04, 2024 at 09:04:37PM +0800, Zorro Lang wrote: > > On Sun, Nov 03, 2024 at 11:50:32PM -0800, Christoph Hellwig wrote: > > > On Fri, Nov 01, 2024 at 02:49:26PM -0700, Darrick J. Wong wrote: > > > > > How about unset the MKFS_OPTIONS for this test? As it already tests rtdev > > > > > and logdev by itself. Or call _notrun if MKFS_OPTIONS has "rmapbt=1"? > > > > > > > > That will exclude quite a few configurations. Also, how many people > > > > actually turn on rmapbt explicitly now? > > > > > > > > > Any better idea? > > > > > > > > I'm afraid not. Maybe I should restructure the test to force the rt > > > > device to be 500MB even when we're not using the fake rtdev? > > > > > > All of this is really just bandaids or the fundamental problem that: > > > > > > - we try to abitrarily mix config and test provided options without > > > checking that they are compatible in general, and with what the test > > > is trying to specifically > > > - some combination of options and devices (size, block size, sequential > > > required zoned) fundamentally can't work > > > > > > I haven't really found an easy solution for them. In the long run I > > > suspect we need to split tests between those that just take the options > > > from the config and are supposed to work with all options (maybe a few > > > notruns that fundamentally can't work). And those that want to test > > > specific mkfs/mount options and hard code them but don't take options > > > from the input. > > > > So how about unset extra MKFS_OPTIONS in this case ? This test has its own > > mkfs options (-L label and logdev and rtdev and fssize). > > The trouble with clearing MKFS_OPTIONS is that you then have to adjust > the other _scratch_* calls in check_label(), and then all you're doing > is reducing fs configuration test coverage. If (say) there was a bug > when changing the fs label on a rtgroups filesystem with a rt section, > you'd never see it. Nobody need to overload MKFS_OPTIONS or unset it. Local test configs are supposed to be passed as function parameters, whilst MKFS_OPTIONS defines the global defaults. When the two conflict, _scratch_mkfs drops the global MKFS_OPTIONS and uses only the local parameters so the filesystem is set up with the configuration the test expects. In this case, MKFS_OPTIONS="-m rmapbt=1" which conflicts with the local RTDEV/USE_EXTERNAL test setup. Because the test icurrently overloads the global MKFS_OPTIONS with local test options, the local test parameters are dropped along with the global paramters when there is a conflict. Hence the mkfs_scratch call fails to set the filesystem up the way the test expects. IOWs, Zorro's fix is correct and the right one to make - it fixes the failures here on all the config sections I have that have configs that aren't compatible with RT devices. -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
