On Wed, Jun 16, 2021 at 06:55:31PM -0400, Theodore Ts'o wrote: > On Thu, Jun 17, 2021 at 01:51:04AM +0800, Gao Xiang wrote: > > > > Considering the original XFS regression report [1], I think > > underlayfs blksize may still be needed. And binary search to get the > > maximum attr value may be another new case for this as well. Or > > alternatively just add by block size to do a trip test. > > > > Although I have no idea if we can just skip the case when testing with > > NFS. If getting underlayfs blksize is unfeasible, I think we might > > skip such case for now since nfs blksize is not useful for generic/486. > > > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=199119 > > I've looked at the original XFS regression size, and I don't see why > using the underlaying blocksize matters at all. This is especially > true if you look at the comment in the test, and the commit which > fixed the bug. All that is needed for the xfs regression test is to > start with a small xattr, and replace it with a large xattr. The > blocksize is really irrelevant. What I said was the original testcase strictly addressing the original regression report, which converts from shortform to single-block leaf format, see: https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/attr_replace_test.c#n40 > /* > * The value should be 3/4 the size of a fs block to ensure that we > * get to extents format. > */ > ret = fstat(fd, &sbuf); > if (ret < 0) die(); > size = sbuf.st_blksize * 3 / 4; and https://lore.kernel.org/fstests/20180425054826.GB1661@magnolia/ > > And I found another problem in the test, it fails on 1k/2k block size > > extN filesystems, because 2k xattr doesn't fit in single block.. e.g. > > > > -Attribute "world" has a 2048 byte value for SCRATCH_MNT/hello > > +No space left on device > > +error=22 at line 46 > > +Attribute "world" has a 1 byte value for > > > > We probably need to check the block size of SCRATCH_DEV and _notrun if > > it's smaller than 4k. > > > > Or require ea_inode feature when block size < 4k? Note that this test > > does fail on ext4 with ea_inode feature enabled (so add ext4 list to > > cc). e.g. > I was about to say "Eh, this is a regression test for XFS so that's > probably fine, but then... Of course, the testcase itself may have room to improve, I'll look at it when I have extra time. Thanks, Gao Xiang > > - Ted
