On 2018/10/16 17:26, Amir Goldstein Wrote: > On Tue, Oct 16, 2018 at 10:32 AM zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: >> >> Some valid test cases about fsck.overlay may be not valid enough now, >> they lose the impure xattr on the parent directory of the simluated >> redirect directory, and lose the whiteout which use to cover the origin >> lower object. Then fsck.overlay will fix these two inconsistency which >> are not those test cases want to cover, thus it will lead to >> fsck.overlay return FSCK_NONDESTRUCT instead of FSCK_OK. Fix these by >> complement the missing overlay related features. >> >> Signed-off-by: zhangyi (F) <yi.zhang@xxxxxxxxxx> >> --- > > Ok. I think it's fine if we merge this fix now, but this way it is going > to be quite hard to maintain this test. > > Imagine every time that you add another feature to fsck.overlay, > say "add overlay features xattr", fsck will start returning FSCK_NONDESTRUCT > and break this test. > > Perhaps it would have been better to construct the test cases by: > - mount overlay > - create some copied up/ redirected dirs and whiteouts > - umount overlay > - make minor modifications to upper/lower layer > - run fsck > > Then you wouldn't need to worry about things like impure parent dir > and future overlay features. > > I will leave it to you to decide if you want to fix this now or the > next time around... > Indeed, I thought about this choice. If we create overlay on-disk features (xattrs,whiteouts...) through overlayfs, the fsck tests results becomes non-independent. It will depends on the kernel (overlayfs module) user are running the test. Imaging if user want to test the latest fsck.overlay on the old kernel which contains a compatible feature xattr fsck.overlay know but the kernel don't, we will get the unexpected result. Or maybe we can add some _require_xxx_feature() helper to enforce user doing test on the kernel which supports the specified feature? Thanks, Yi.