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

kvs: reconsider use of EPERM in lookup/commit API #1228

Closed
chu11 opened this issue Oct 10, 2017 · 4 comments
Closed

kvs: reconsider use of EPERM in lookup/commit API #1228

chu11 opened this issue Oct 10, 2017 · 4 comments
Assignees

Comments

@chu11
Copy link
Member

chu11 commented Oct 10, 2017

in the internal KVS lookup & commit APIs, the errno EPERM is used when returning an error to user for an internally inconsistent state. For example, if a directory reference points to a non-directory tree object.

This is perhaps not the best errno to describe this and should be reconsidered to something else.

@chu11
Copy link
Member Author

chu11 commented Oct 17, 2017

thinking about this a bit, EFAULT might be a good replacement. For anything related to a bad blobref or invalid references, this makes perfect sense, as the blobref is a "bad address".

There may not be any errno that is perfect for anything that is just "internally inconsistent". ENOTRECOVERABLE seems like a pretty good one.

@chu11
Copy link
Member Author

chu11 commented Oct 17, 2017

Perhaps EUCLEAN "structure needs cleaning", it's not bad and sort of similar to the idea of "file system corruption".

@garlick
Copy link
Member

garlick commented Oct 17, 2017

I kind of like ENOTRECOVERABLE (State not recoverable). EFAULT and EPERM are both too common to me and tend to point to specific problems that might throw someone chasing an issue off course.

If I saw ENOTRECOVERABLE pop up, I'd probably think "WTF?", then grep source. If I saw EPERM or EFAULT I might jump to conclusions about security or bad pointers and follow a false lead.

For reference here is a list of POSIX.2008 error numbers and descriptions

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

@chu11
Copy link
Member Author

chu11 commented Oct 17, 2017

ok, lets go with ENOTRECOVERABLE

chu11 added a commit to chu11/flux-core that referenced this issue Oct 17, 2017
Previously EPERM was generally used as the errno to signify that
the state without the KVS was inconsistent/illogical (e.g. a
dirref points to a val object).  To remove confusion that a
potential security issue is in play, change the errno from EPERM
to ENOTRECOVERABLE.

Fixes flux-framework#1228
chu11 added a commit to chu11/flux-core that referenced this issue Oct 17, 2017
Previously EPERM was generally used as the errno to signify that
the state without the KVS was inconsistent/illogical (e.g. a
dirref points to a val object).  To remove confusion that a
potential security issue is in play, change the errno from EPERM
to ENOTRECOVERABLE.

Update unit tests appropriately.

Fixes flux-framework#1228
@chu11 chu11 self-assigned this Oct 18, 2017
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

No branches or pull requests

2 participants