Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Hans Holmberg <Hans.Holmberg@wdc.com>]

On 30/04/2025 16:52, Darrick J. Wong wrote:
> On Wed, Apr 30, 2025 at 08:26:00AM +0000, Hans Holmberg wrote:
>> On 28/04/2025 17:58, Darrick J. Wong wrote:
>>> On Mon, Apr 28, 2025 at 12:16:34PM +0000, Hans Holmberg wrote:
>>>> On 25/04/2025 17:05, Darrick J. Wong wrote:
>>>>> ps this test should check
>>>>> that a readonly log device results in a norecovery mount and that
>>>>> pending changes don't show up if the mount succeeds?
>>>>>
>>>>> Also, ext4 supports external log devices, should this be in
>>>>> tests/generic?
>>>>
>>>> Doh!, actually ext4 has a test for this already, ext4/002
>>>> (also based on generic/050)
>>>>
>>>> With my fix, ext4/002 passes for xfs Should/can we turn that into a
>>>> generic test?
>>>
>>> Yeah, it looks like ext4/002 already does most of what you want.  Though
>>> I'd amend it to check that SCRATCH_MNT/00-99 aren't visible in the
>>> norecovery mounts and only appear after recovery actually runs.
>>>
>>
>> So I added this check to ext4/002 and while this works for xfs - the
>> touched files are not visible until log recovery has completed, it does
>> not for ext3/4.
>>
>> For ext3/4 the files are visible after the first successful (norecovery)
>> mount, so even though we did a shutdown, a log recovery does not seem
>> required (dmesg tells me that the log recovery is done in the end after
>> the log device is set back to rw)
>>
>> ..and I presume this is fine - for a generic test can we really assume
>> that a log recovery is required to see the files?
> 
> Nope.  I guess that's an implementation dependent behavior.  TBH I'm not
> even sure we can 100% rely on it for xfs, since it's theoretically
> possible for the log to flush and checkpoint in the very small window
> between the creat and the shutdown call.
> 
> If hoisting ext4/002 to generic works for the three main filesystems
> then I'm fine with just doing that without the extra tests.

Cool, I've submitted a patch to turn ext4 generic.