On Wed, May 13, 2020 at 5:38 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Wed, May 13, 2020 at 6:17 AM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote: > > > > ---- 在 星期三, 2020-05-13 09:10:19 Eryu Guan <eguan@xxxxxxxxxxxxxxxxx> 撰写 ---- > > > On Tue, May 12, 2020 at 07:56:35PM +0300, Amir Goldstein wrote: > > > > > > > I see no feature detection logic, so test just fails on old kernels > > > > > > > without this feature? I tried with v5.7-r4 kernel, test fails because > > > > > > > each whiteout file has only one hardlink. > > > > > > > > > > > > That's true. > > > > > > > > > > I'd like to see it _notrun on old kernels where the feature is not > > > > > available. But that seems hard to do.. Do you have any better ideas? > > > > > > > > > > > > > I've got a few. > > > > 1. LTP has the concept of require minimum kernel version. > > > > This would mean that functionality will be not be tested if feature > > > > is backported to old kernels. > > > > 2. We could add to overlayfs advertising of supported features, like > > > > /sys/fs/ext4/features/, but it already does "advertise" the configurable > > > > features at /sys/module/overlay/parameters/, and we were already > > > > asking the question during patch review: > > > > /* Is there a reason anyone would want not to share whiteouts? */ > > > > ofs->share_whiteout = true; > > > > and we left the answer to "later" time. > > > > > > > > So a simple solution would be to add the module parameter (without adding > > > > a mount option), because: > > > > - It doesn't hurt (?) > > > > - Somebody may end up using it, for some reason we did not think of > > > > - We can use it in test to require the feature > > > > > > Yeah, I think that works. And I see that ext4 and btrfs both have a > > > /sys/fs/<fs>/features directory and list supported features there, is > > > this something overlay could do? Or is this basically the same thing as > > > what you proposed? > > > > > > > IMO, for those features which don't need to change module param, maybe feature list > > is more suitable. > > > > I suppose it is more suitable, but since at the moment there is only one(?) > such feature and there is an open question whether it should or should not > be configurable, I myself would have taken the easy path, but Miklos > often has a different perspective on these sort of things... What exactly are we testing? Hard linked whiteouts are an optimization, not something to be relied on in any case. The test should succeed even if overlayfs decides for some reason not to share the inode. Thanks, Miklos
