Re: [PATCH v2 2/2] generic/486: adjust the max xattr size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 10, 2022 at 05:25:45PM +1000, Dave Chinner wrote:
> On Fri, Jun 10, 2022 at 01:35:36PM +0800, Xiubo Li wrote:
> > 
> > On 6/9/22 6:53 PM, Luís Henriques wrote:
> > > CephFS doesn't have a maximum xattr size.  Instead, it imposes a maximum
> > > size for the full set of xattrs names+values, which by default is 64K.
> > > And since ceph reports 4M as the blocksize (the default ceph object size),
> > > generic/486 will fail in this filesystem because it will end up using
> > > XATTR_SIZE_MAX to set the size of the 2nd (big) xattr value.
> > > 
> > > The fix is to adjust the max size in attr_replace_test so that it takes
> > > into account the initial xattr name and value lengths.
> > > 
> > > Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
> > > ---
> > >   src/attr_replace_test.c | 7 ++++++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> > > index cca8dcf8ff60..1c8d1049a1d8 100644
> > > --- a/src/attr_replace_test.c
> > > +++ b/src/attr_replace_test.c
> > > @@ -29,6 +29,11 @@ int main(int argc, char *argv[])
> > >   	char *value;
> > >   	struct stat sbuf;
> > >   	size_t size = sizeof(value);
> > > +	/*
> > > +	 * Take into account the initial (small) xattr name and value sizes and
> > > +	 * subtract them from the XATTR_SIZE_MAX maximum.
> > > +	 */
> > > +	size_t maxsize = XATTR_SIZE_MAX - strlen(name) - 1;
> > 
> > Why not use the statfs to get the filesystem type first ? And then just
> > minus the strlen(name) for ceph only ?
> 
> No. The test mechanism has no business knowing what filesystem type
> it is running on - the test itself is supposed to get the limits for
> the filesystem type from the test infrastructure.
> 
> As I've already said: the right thing to do is to pass the maximum
> attr size for the test to use via the command line from the fstest
> itself. As per g/020, the fstests infrastructure is where we encode
> weird fs limit differences and behaviours based on $FSTYP.  Hacking
> around weird filesystem specific behaviours deep inside random bits
> of test source code is not maintainable.
> 
> AFAIA, only ceph is having a problem with this test, so it's trivial
> to encode into g/486 with:
> 
> # ceph has a weird dynamic maximum xattr size and block size that is
> # much, much larger than the maximum supported attr size. Hence the
> # replace test can't auto-probe a sane attr size and so we have
> # to provide it with a maximum size that will work.
> max_attr_size=65536
> [ "$FSTYP" = "ceph" ] && max_attr_size=64000
> attr_replace_test -m $max_attr_size .....
> .....

Agree. I'd recommend changing the attr_replace_test.c, make it have a
default max xattr size (keep using the XATTR_SIZE_MAX or define one if
it's not defined), then give it an optinal option which can specify a
customed max xattr size from outside.

Then the test case (e.g. g/486) which uses attr_replace_test can
specify a max xattr size if it needs. And it's easier to figure
out what attr size is better for a specified fs in test case.

Thanks,
Zorro

> 
> Cheers,
> 
> Dave.
> 
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx
> 




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux