Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

behlendorf
Copy link
Contributor

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

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
@markhpc
Copy link

markhpc commented Apr 18, 2013

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?

@wido
Copy link

wido commented Apr 18, 2013

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:# ceph -s
health HEALTH_OK
monmap e1: 1 mons at {desktop=[2a00:f10:113:0:16da:XXX:XXX:XXX]:6789/0}, election epoch 1, quorum 0 desktop
osdmap e42: 2 osds: 2 up, 2 in
pgmap v903: 584 pgs: 584 active+clean; 8 bytes data, 368 MB used, 5232 GB / 5233 GB avail
mdsmap e1: 0/0/1 up
root@wido-desktop:
#

root@wido-desktop:# zfs list
NAME USED AVAIL REFER MOUNTPOINT
desktop 124G 2.56T 144K /desktop
desktop/ceph 425M 2.56T 152K /desktop/ceph
desktop/ceph/mon 17.7M 2.56T 144K /desktop/ceph/mon
desktop/ceph/mon/desktop 17.6M 2.56T 6.40M /desktop/ceph/mon/desktop
desktop/ceph/osd 407M 2.56T 152K /desktop/ceph/osd
desktop/ceph/osd/0 202M 2.56T 184M /desktop/ceph/osd/0
desktop/ceph/osd/1 205M 2.56T 185M /desktop/ceph/osd/1
desktop/home 119G 2.56T 107G /home
desktop/journal-0 2.21G 2.56T 154M -
desktop/journal-1 2.24G 2.56T 185M -
root@wido-desktop:
#

And in my ceph.conf:

[osd]
osd data = /desktop/ceph/$type/$id
osd journal = /dev/zvol/desktop/journal-$id
debug osd = 20
debug filestore = 20

@markhpc
Copy link

markhpc commented Apr 18, 2013

Hi Wido,

ZFS has a System Attribute based xattrs implementation that performs better than the default one. Here's some more info:

#443

If you get a chance, would you mind testing the wip-debug-xattr branch and seeing if you hit any asserts?

@behlendorf
Copy link
Contributor Author

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.

@behlendorf
Copy link
Contributor Author

@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?

@markhpc
Copy link

markhpc commented Apr 19, 2013

@behlendorf I think it's sys_fgetxattr(). Is that what you were thinking of?

@markhpc
Copy link

markhpc commented Apr 19, 2013

specifically we use sys_fgetxattr() in chain_xattr.cc:

https://github.com/ceph/ceph/blob/master/src/os/chain_xattr.cc#L207

@behlendorf
Copy link
Contributor Author

@clusterfaq Ahh, never mind then. That's supported, I misremembered the call.

@markhpc
Copy link

markhpc commented Apr 19, 2013

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

@behlendorf
Copy link
Contributor Author

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.

@behlendorf behlendorf closed this Apr 24, 2013
@behlendorf behlendorf mentioned this pull request Apr 30, 2013
@behlendorf behlendorf deleted the issue-1408 branch February 16, 2017 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants