Re: [PATCH v2] generic: test i_mode recovery after power failure

[Eryu Guan <guaneryu@gmail.com>]

On Wed, Mar 06, 2019 at 07:53:44AM +1100, Dave Chinner wrote:
> On Tue, Mar 05, 2019 at 07:47:44PM +0800, Chao Yu wrote:
> > After fsync, filesystem should guarantee inode metadata including
> > permission info being persisted, so even after sudden power-cut,
> > during mount, we should recover i_mode fields correctly, in order
> > to not loss those meta info.
> > 
> > So adding this testcase to check whether generic filesystem can
> > guarantee that.
> 
> Can I make a suggestion here?
> 
> I've noticed that we are getting a lot of these one-off, random
> "fsync persists one specific piece of metadata in one specific case"
> tests, mainly in response to some fsync bug that was found in btrfs.
> This is reactive, and does not find new bugs in this area.
> 
> We are also beyond the point where the number of tests is
> maintainable (e.g. to be able to make infrastructure changes), so we
> really should be looking to consolidate largely similar tests into
> single tests where possible.

This sounds reasonable, and could reduce the test time a bit as well.

> 
> I'd strongly suggest that a robust fsync tester should be written
> for all the cases that are currently being addressed in an ad hoc
> fashion. Then they can be consolidated into a single test run, and
> we can get rid of all the one-off "fsync failed on this <specific
> thing>" tests from the harness.
> 
> Oh, wait, we *already have that infrastructure*: src/fsync-tester.c
> and generic/311.
> 
> Can we please consider rolling all of these "do something, fsync,
> drop-writes, remount check" into fsync-tester.c and do the same for
> all future one-off "did fsync persist X" tests?

I'd like to take this patch and the one from Filipe for now, and look
into folding them into fsync-tester later, in a separate patch.

Thanks,
Eryu