On Tue, Oct 31, 2017 at 12:25:51PM +0200, Omer Zilberberg wrote: > > > On 10/31/2017 06:37 AM, Eryu Guan wrote: > > On Tue, Oct 31, 2017 at 07:36:58AM +1100, Dave Chinner wrote: > >> On Mon, Oct 30, 2017 at 10:08:31AM +0200, Omer Zilberberg wrote: > >>> These tests locally change the TEST_FS_MOUNT_OPTS/MOUNT_OPTIONS > >>> environment variables, and run _test_cycle_mount. As a result, following > >>> tests using the TEST mount point may start with different mount options, > >>> depending on run order. > >> I don't think that's the case. The change of the environment > >> variable should only affect the current test process and it's > >> children. When the test exits, we go back to the environment of the > >> check process, where the TEST_FS_MOUNT_OPTS environment variable is > >> still correctly set, and all future tests inherit from that. i.e.: > >> > >> $ export FOO=foo > >> $ echo $FOO > >> foo > >> $ bash > >> $ echo $FOO > >> foo > >> $ export FOO=bar > >> $ echo $FOO > >> bar > >> $ exit > >> $ echo $FOO > >> foo > >> $ > >> > >> And after each test, check runs _check_filesystems(), which cycles > >> the test mount, so for each new test process that is run they should > >> already start in the correct state... > > I agreed, the changing of variables in a sub-shell won't affect the > > parent's copy, and check will restore the mounts with the untouched > > options. > > > > But the problem is that _check_test_fs() will cycle mount TEST_DEV with > > MOUNT_OPTIONS not TEST_FS_MOUNT_OPTS, so if you have different mount > > options set for TEST_DEV and SCRATCH_DEV, you'll see mount options > > changed for TEST_DEV. e.g. > > > > MOUNT_OPTIONS="-o dax" TEST_FS_MOUNT_OPTS="" ./check generic/413 generic/445 > > generic/445 mount TEST_DEV with "-o dax" too > > > > MOUNT_OPTIONS="" TEST_FS_MOUNT_OPTS="-o dax" ./check generic/413 generic/445 > > generic/445 mount TEST_DEV without "-o dax" > > > > MOUNT_OPTIONS="-o dax" TEST_FS_MOUNT_OPTS="-o dax" ./check generic/413 generic/445 > > both tests and both devices mount with "-o dax" > > > > That's been discussed in this thread: > > https://patchwork.kernel.org/patch/9742039/ > > > > Omer, can you please confirm if you're hitting this issue? > I'm not 100% that's the case, so I better describe my settings more clearly: > I have a debug mount option on my system to recover the FS from a backup. > When that flag is set, umount writes everything to the backup. > Mount restores from it, overwriting everything. If you're testing with setting your debug mount option to both TEST_FS_MOUNT_OPTS and MOUNT_OPTIONS, and you still see the failure you were seeing, then that's a different problem. > > As long as generic/413 is not involved, everything works well. > All _test_cycle_mount() calls first back everything up on umount, > then restore upon mount. So I get the same FS contents. > > But, consider generic/118 running after generic/413: > - generic/413 finishes with a mount point with no mount options > - generic/118 begins with restored TEST_FS_MOUNT_OPTS, as you've pointed out. > - some writes are performed to the FS > - next _test_cycle_mount: > calls umount w/o backing up (debug flag previously unset by generic/413). Does this clear the backup too? If so, I suspect TEST_DEV got cleared on first mount with the debug option in generic/118, because the backup has been cleared in the _test_cycle_mount call in generic/413. > calls mount WITH the debug flag, and recovers from an empty backup, > deleting the earlier writes. > - subsequent md5sum fails on "No such file or directory", as FS is now empty. > > > I think fixing _check_<fs>_filesystem() is the correct way. And I guess > > we can refactor out a common function and call it in > > _check_[xfs|btrfs|generic]_filesystem, pass the correct mount options > > based on what device we're working on. > If indeed we're talking about the same problem, > please let me know if you'd like me to prepare a different patch. Sure, really appreciated if you can prepare a different patch, even if it's not the same problem :) Thanks, Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
