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

rbd: Add locking rbd APIs #852

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

Nikhil-Ladha
Copy link
Contributor

@Nikhil-Ladha Nikhil-Ladha commented Mar 30, 2023

Added new locking APIs for the rbd component named rbd_lock_acquire, rbd_lock_break, rbd_lock_release, rbd_lock_get_owners, rbd_is_exclusive_lock_owner along with supporting TCs.

Fixes: #274
Fixes: #275

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
  • Ran make api-update to record new APIs

rbd/locks.go Outdated Show resolved Hide resolved
rbd/locks_test.go Show resolved Hide resolved
rbd/locks.go Outdated Show resolved Hide resolved
@mergify
Copy link

mergify bot commented Apr 4, 2023

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@phlogistonjohn
Copy link
Collaborator

Thanks for the PR, just a heads up that we are going to release tomorrow but this PR will not be included, it should be no problem to land it in the following release.

rbd/locks.go Outdated Show resolved Hide resolved
rbd/locks_test.go Show resolved Hide resolved
rbd/locks_test.go Show resolved Hide resolved
Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

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

Also while rebasing with current master you may have to resolve the conflicts manually.

Edit: Looking at the recent changes, I think there won't be any conflict to be resolved manually.

Added new locking APIs for the rbd component named
`rbd_lock_acquire`, `rbd_lock_break`, `rbd_lock_release`, `rbd_lock_get_owners`,
`rbd_is_exclusive_lock_owner` along with supporting TCs.

Fixes: ceph#275

Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>
Copy link
Collaborator

@anoopcs9 anoopcs9 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 for all clarifications.

@anoopcs9 anoopcs9 added the API This PR includes a change to the public API of a go-ceph package label Apr 13, 2023
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.

I have exactly one thing to discuss, everything else looks good. Before I approve I want to understand the for loop in LockGetOwners. It may be fine, but I want to understand the thought process behind it first. If you do think it can be improved by using WithSizes, go ahead with the change :-)

rbd/locks.go Show resolved Hide resolved
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

@mergify mergify bot merged commit 9041276 into ceph:master Apr 13, 2023
12 checks passed
@Nikhil-Ladha Nikhil-Ladha deleted the issue275/rbd_locking_api branch April 13, 2023 14:37
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 rbd API components: locking Missing rbd API components: function rbd_is_exclusive_lock_owner
3 participants