Skip to content

rados: Implement rados_set_alloc_hint2#710

Merged
mergify[bot] merged 1 commit intoceph:masterfrom
okhoshi:feat/set-alloc-hint
Jul 6, 2022
Merged

rados: Implement rados_set_alloc_hint2#710
mergify[bot] merged 1 commit intoceph:masterfrom
okhoshi:feat/set-alloc-hint

Conversation

@okhoshi
Copy link
Contributor

@okhoshi okhoshi commented Jun 17, 2022

Signed-off-by: Quentin Devos 4972091+Okhoshi@users.noreply.github.com

Add support in go-ceph:

  • rados_set_alloc_hint2
  • rados_write_op_set_alloc_hint2

Closes #697

Checklist

  • Added tests for features and functional changes
  • Public functions and types are documented
  • Standard formatting is applied to Go code
  • Is this a new API? Added a new file that begins with //go:build ceph_preview

I'm not sure how I'm supposed to test the Allocation Hint, especially since I'm not aware of a way to check the Allocation hints set on a file, and that the functions never return errors.

@phlogistonjohn
Copy link
Collaborator

I'm not sure how I'm supposed to test the Allocation Hint, especially since I'm not aware of a way to check the Allocation hints set on a file, and that the functions never return errors.

Heh. That's unfortunate. I suppose we can just add a few calls to the function with 2-3 of the various flags along with some other ops. This way we at least see that it doesn't segfault or anything like that. It's not the best test in the world but IMO better than nothing. Can you please do something like that? Once done, I can activate the CI and we'll be sure it compiles and executes even if we can't assert anything about it beyond that.

@okhoshi okhoshi force-pushed the feat/set-alloc-hint branch from 14a0e52 to 3a364f8 Compare June 17, 2022 17:36
@okhoshi
Copy link
Contributor Author

okhoshi commented Jun 17, 2022

Hey @phlogistonjohn, I added the tests you suggested 👍

phlogistonjohn
phlogistonjohn previously approved these changes Jun 21, 2022
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@phlogistonjohn phlogistonjohn requested a review from ansiwen June 21, 2022 14:18
@phlogistonjohn phlogistonjohn added the API This PR includes a change to the public API of a go-ceph package label Jun 21, 2022
@okhoshi okhoshi force-pushed the feat/set-alloc-hint branch from 3a364f8 to 4054294 Compare June 30, 2022 15:07
@mergify mergify bot dismissed phlogistonjohn’s stale review June 30, 2022 15:08

Pull request has been modified.

@okhoshi okhoshi requested review from ansiwen and phlogistonjohn July 1, 2022 07:47
@okhoshi okhoshi force-pushed the feat/set-alloc-hint branch from 4054294 to c057cb5 Compare July 5, 2022 15:20
@okhoshi okhoshi requested a review from phlogistonjohn July 5, 2022 15:22
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

Copy link
Collaborator

@ansiwen ansiwen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Quentin Devos <4972091+Okhoshi@users.noreply.github.com>
@ktdreyer ktdreyer force-pushed the feat/set-alloc-hint branch from c057cb5 to 55d0bd6 Compare July 6, 2022 17:39
@mergify mergify bot merged commit 5f38b54 into ceph:master Jul 6, 2022
@okhoshi okhoshi deleted the feat/set-alloc-hint branch July 7, 2022 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API This PR includes a change to the public API of a go-ceph package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing rados API components: function rados_set_alloc_hint

3 participants