-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correctly return ERANGE in getxattr(2) #1409
Conversation
According to the getxattr(2) man page the ERANGE errno should be returned when the size of the value buffer is to small to hold the result. Prior to this patch the implementation would just truncate the value to size bytes. Signed-off-by: Brian Behlendorf <[email protected]> Issue openzfs#1408
wido: I've been doing some testing with Ceph wip-debug-xattr and am hitting asserts in both the SA and non-SA codepath. The SA issues may have been related to a new bug Brian fixed. What path were you using in your tests? |
I've been using Ceph 0.60 (f26f7a39021dbf440c28d6375222e21c94fe8e5c) from the master branch and that works for me. What do you mean with SA and non-SA? Not sure what those abbreviations mean.. I just ran a 'rados load-gen' and both my OSDS are still alive. root@wido-desktop: root@wido-desktop: And in my ceph.conf: [osd] |
Hi Wido, ZFS has a System Attribute based xattrs implementation that performs better than the default one. Here's some more info: If you get a chance, would you mind testing the wip-debug-xattr branch and seeing if you hit any asserts? |
I've been using an an xattr test utility I put together here, https://github.com/behlendorf/xattrtest, to check for correct behavior. So far I haven't seen any correctness issues at least with this workload. But perhaps ceph is doing something slightly different. |
@clusterfaq Possibly related, you mentioned you saw failures from sys_getfattr(). This would be because we haven't implemented it yet, #229, does ceph depend on file attribute support. And if so which attributes? |
@behlendorf I think it's sys_fgetxattr(). Is that what you were thinking of? |
specifically we use sys_fgetxattr() in chain_xattr.cc: https://github.com/ceph/ceph/blob/master/src/os/chain_xattr.cc#L207 |
@clusterfaq Ahh, never mind then. That's supported, I misremembered the call. |
I'm suspecting that may be at least partially what is failing on our end though. If I trace through our _fgetattr() implementation I ultimately end up looking at sys_fgetxattr() and seeing something like this in the logs: osd.1.log: -158> 2013-04-18 11:23:26.598801 7f6523fff700 -1 filestore(/srv/osd-device-1-data) _setattrs: _fgetattr returned (95) Operation not supported on read of attr _ for object c245dfe3/benchmark_data_mira097_22066_object12/head//3 |
Merged as, f706421 Correctly return ERANGE in getxattr(2) If we're still seeing problems running ceph let's open a new issue with those symptoms to track it. |
According to the getxattr(2) man page the ERANGE errno should be
returned when the size of the value buffer is to small to hold the
result. Prior to this patch the implementation would just truncate
the value to size bytes.
Signed-off-by: Brian Behlendorf [email protected]
Issue #1408