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
librados: extend C API for so it accepts keys with NUL chars #20314
Merged
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
branch-predictor
force-pushed
the
bp-librados-nntk
branch
3 times, most recently
from
February 5, 2018 13:46
aa1d991
to
56e4f47
Compare
liewegas
reviewed
Mar 12, 2018
src/librados/librados.cc
Outdated
size_t val_len, | ||
int *prval) | ||
{ | ||
bufferlist bl; | ||
bl.append(val, val_len); | ||
std::map<std::string, pair<bufferlist, int> > assertions; | ||
string lkey = string(key, key_len); | ||
|
||
assertions[key] = std::make_pair(bl, comparison_operator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mea to change key to lkey here?
Can you add tests to test/librados/omap.cc for these? Otherwise looks good! Is this fixing a user in-tree that is passing 0's in the keys, or is it to support a new out-of-tree omap user? |
On 18-03-12 04:27 PM, Sage Weil wrote:
Can you add tests to test/librados/omap.cc for these?
Otherwise looks good! Is this fixing a user in-tree that is passing 0's in
the keys, or is it to support a new out-of-tree omap user?
The latter. We had cases of broken rbd images and rbd alone doesn't provide
many tools to fix it - for example, broken rbd_children can't be easily
fixed. While writing our own tool for that we had to be creative in what
apis to use to make it work -- librados C api is really lacking in this
regard (C++ api works fine, but we prefer C one because it's more stable).
This PR will make things easier for us in the future and I believe for
others too.
|
Librados C API functions that operate on OMAP accept key values that are NUL-terminated. This makes them unsuitable for operation on keys that have NULs embedded in them (like those produced by librbd). This commit adds new API functions: - rados_omap_get_next2 - rados_write_op_omap_cmp2 - rados_write_op_omap_set2 - rados_write_op_omap_rm_keys2 - rados_read_op_omap_cmp2 - rados_read_op_omap_get_vals_by_keys2 that accept or provide actual key length in bytes and therefore are not limiting key values to first NUL encountered. Signed-off-by: Piotr Dałek <piotr.dalek@corp.ovh.com>
branch-predictor
force-pushed
the
bp-librados-nntk
branch
3 times, most recently
from
March 15, 2018 13:12
f96c041
to
3d19dfb
Compare
Unit tests for new apis: * rados_omap_get_next2 * rados_write_op_omap_cmp2 * rados_write_op_omap_set2 * rados_write_op_omap_rm_keys2 * rados_read_op_omap_cmp2 * rados_read_op_omap_get_vals_by_keys2 Signed-off-by: Piotr Dałek <piotr.dalek@corp.ovh.com>
branch-predictor
force-pushed
the
bp-librados-nntk
branch
from
March 15, 2018 15:38
3d19dfb
to
4900398
Compare
@liewegas done:
|
liewegas
approved these changes
Mar 15, 2018
retest this please |
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.
Librados C API functions that operate on OMAP accept key values that are NUL-terminated. This makes them unsuitable for operation on keys that have NULs embedded in them (like those produced by librbd). This PR adds new API functions:
that accept or provide actual key length in bytes and therefore are not limiting key values to first NUL encountered.
Signed-off-by: Piotr Dałek piotr.dalek@corp.ovh.com