os/filestore: force lfn attrs to be written atomically, restructure name length limits #8496
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tests: ceph/ceph-qa-suite#944 |
| if (name_length > g_conf->osd_max_object_name_len) { | ||
| dout(4) << "do_op name+namespace+locator is longer than " | ||
| << g_conf->osd_max_object_name_len | ||
| << " bytes" << dendl; |
There was a problem hiding this comment.
I don't think this hunk is necessary if the ObjectStore has its own check. Also, I don't think it makes sense. If the max length is 16, then "0123456789abcdef" is a valid name. I should be able to specify an equally long key so that I can make "foo" stored next to it...
There was a problem hiding this comment.
Instead, I would separately enforce that key is <= max_object_name_len. And either use the same for namespace, or specify a new tunable that restricts it.
Enforce locator length vs the max name length and also introduce a namespace length limit. In addition to these checks, also pass the head object to the ObjectStore implementation to validate. This allows LFNIndex to account for the idiosyncracies of its filename escaping and for different xattr value max sizes. Signed-off-by: Samuel Just <sjust@redhat.com>
onechunk was a deceptive name since it didn't actually ensure that a single chunk would be used. Rename to ensure_single_attr. Also, add a parameter to ensure that we use a single attribute. We need these to be distinct since we have LFN attrs which have been split over 254 byte xattrs since after hammer which we need to correctly clean up. However, we need to ensure going forward that those attrs are never split over more than one chunk -- it's not atomic. Further, skip_chain_cleanup should imply ensure_single_attr, so make them template params and add a static check to make sure we don't mess this up in the future. Signed-off-by: Samuel Just <sjust@redhat.com>
…MAX_BLOCK_LEN Users of setxattr can now set attrs larger than that size. Signed-off-by: Samuel Just <sjust@redhat.com>
We leave skip_chain_cleanup = false since the object may have been written using code that split the lfn attr over multiple attrs. Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
…maller than max name Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
All current callers (except list_objects) already expect ret < 0 for error. Adjust list_objects to skip on EINVAL instead of 0. Signed-off-by: Samuel Just <sjust@redhat.com>
Otherwise callers need to implement buffer doubling in a lot of cases, which is error prone. Signed-off-by: Samuel Just <sjust@redhat.com>
LFN attrs can be longer than these hard coded limits, let's just do buffer doubling everywhere. Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
|
http://pulpito.ceph.com/samuelj-2016-04-10_21:23:13-rados-wip-sam-testing-distro-basic-smithi Only bug from that run, seems to be unrelated: Actually, that bug appears to be because I reverted part of the pgnls branch in the integration branch, so safe to ignore. Ready for review again. |
|
@liewegas ping |
liewegas
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested: samuelj-2016-04-06_16:46:34-rados-wip-sam-testing-distro-basic-smithi -- failures appear to be unrelated
ceph-qa-suite branch to follow