Xiubo Li <xiubli@xxxxxxxxxx> writes: > On 6/9/22 10:21 PM, David Disseldorp wrote: >> Hi Luís, >> >> On Thu, 9 Jun 2022 11:53:42 +0100, 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. >>> >>> This patch fixes the max_attrval_size so that it is slightly < 64K in >>> order to accommodate any already existing xattrs in the file. >>> >>> Signed-off-by: Luís Henriques <lhenriques@xxxxxxx> >>> --- >>> tests/generic/020 | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/tests/generic/020 b/tests/generic/020 >>> index d8648e96286e..76f13220fe85 100755 >>> --- a/tests/generic/020 >>> +++ b/tests/generic/020 >>> @@ -128,7 +128,7 @@ _attr_get_max() >>> pvfs2) >>> max_attrval_size=8192 >>> ;; >>> - xfs|udf|9p|ceph) >>> + xfs|udf|9p) >>> max_attrval_size=65536 >>> ;; >>> bcachefs) >>> @@ -139,6 +139,14 @@ _attr_get_max() >>> # the underlying filesystem, so just use the lowest value above. >>> max_attrval_size=1024 >>> ;; >>> + ceph) >>> + # CephFS does not have a maximum value for attributes. Instead, >>> + # it imposes a maximum size for the full set of xattrs >>> + # names+values, which by default is 64K. Set this to a value >>> + # that is slightly smaller than 64K so that it can accommodate >>> + # already existing xattrs. >>> + max_attrval_size=65000 >>> + ;; >> I take it a more exact calculation would be something like: >> (64K - $max_attrval_namelen - sizeof(user.snrub="fish2\012"))? > > Yeah, something like this looks better to me. Right, it could be hard-coded. But we'd need to take into account that the attribute value may not be ascii. That's why my initial attempt to fix this was to decode everything in hex. > I am afraid without reaching up to the real max size we couldn't test the real > bugs out from ceph. Such as the bug you fixed in ceph Locker.cc code. OK, I'll change this to use the exact value. Thanks, Xiubo. Cheers, -- Luis > >> Perhaps you could calculate this on the fly for CephFS by passing in the >> filename and subtracting the `getfattr -d $filename` results... That >> said, it'd probably get a bit ugly, expecially if encoding needs to be >> taken into account. >> >> Reviewed-by: David Disseldorp <ddiss@xxxxxxx> >> >> Cheers, David >> >
