On Fri, Oct 19, 2018 at 3:36 PM zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: > > On 2018/10/18 12:44, Amir Goldstein Wrote: > > On Thu, Oct 18, 2018 at 6:43 AM zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: > >> > >> 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? > >> > > > > I think the only sane choice is for this test to relax the expectation of 0 > > exit code to "correct" exit code (i.e. _overlay_repair_dirs()) for the "Valid" > > test cases. > > > The meaning of the "valid" test cases is to make sure fsck.overlay will never > change the on-disk filesystem if the feature(xattr) we want to test is valid, > so the FSCK_OK and FSCK_NONDESTRUCT is totally different. > > If we relax the expectation of 0(FSCK_OK) exit code, we couldn't distinguish > the fsck was changed the file system or not, if so, we also couldn't distinguish > it's caused by some bugs of fsck or the base dirs were not valid enough. Then > the "valid" test cases cannot catch fsck's fault accurately. So I think make > a valid enough overlay image manually now is still the best way. > > I think maybe after we introduce "feature set" xattr, it will becomes much easier, > fsck.overlay will fix things according to feature set, and we create overlay image > through mkfs.overlay. So we could disable some irrelevant features to avoid > disturbing our tests. Is it fine? > Its fine by me to re-open this for discussion that next time fsck.overlay changes and breaks the test. Thanks, Amir.
