On Tue, Oct 8, 2024 at 3:06 PM Mark Harmstone <maharmstone@xxxxxxxx> wrote: > > Thanks Filipe. > > On 8/10/24 14:35, Filipe Manana wrote: > >> +_begin_fstest auto quick metadata log volume > > > > Why the volume group? The test isn't doing any volume management stuff. > > > > However it should be in the "recoveryloop" group. > > No worries, I'll change that. > > >> +_log_writes_replay_log mkfs $SCRATCH_DEV > >> + > >> +_log_writes_fast_replay_check fua "$SCRATCH_DEV" "$BTRFS_UTIL_PROG check $SCRATCH_DEV" > > > > Why do we need to do the replays twice? Once with > > _log_writes_replay_log mkfs and then again with > > _log_writes_fast_replay_check fua. > > _log_writes_replay_log mkfs to put the FS back how it was after > mkfs.btrfs, _log_writes_fast_replay_check to replay it from there. Is > _log_writes_replay_log redundant here? No, I missed the mkfs mark passed to _log_writes_replay. Though it still seems redundant because _log_writes_fast_replay_check is called for each fua mark, and after mkfs we have a fua. > > > There's also nothing in this test that is btrfs specific, it could be > > made a generic test instead. > > Yes there is, it's running btrfs check every time. Right, but instead of calling it explicitly, it could pass "_check_scratch_fs" as an argument instead, and the test becomes generic: _log_writes_fast_replay_check fua "$SCRATCH_DEV" "_check_scratch_fs" Thanks. > > Mark
