Skip to content

rados: Add support for rados_checksum API#1262

Merged
mergify[bot] merged 1 commit into
ceph:masterfrom
naylorpmax-joyent:rados-checksum
May 6, 2026
Merged

rados: Add support for rados_checksum API#1262
mergify[bot] merged 1 commit into
ceph:masterfrom
naylorpmax-joyent:rados-checksum

Conversation

@naylorpmax-joyent
Copy link
Copy Markdown
Contributor

@naylorpmax-joyent naylorpmax-joyent commented Apr 29, 2026 •
edited
Loading

These changes add the Checksum method to rados.IOContext that implements the rados_checksum method for all Ceph-supported checksum types (XXHash32, XXHash64, and CRC32).

RADOS API: https://github.com/ceph/ceph/blob/226cf55c23404ad4784c6bb3f8a7c85e02bcca08/src/include/rados/librados.h#L1615-L1619

Reference IOCtxImpl.checksum method: https://github.com/ceph/ceph/blob/c6ea09b123c45a19190f2b65e4c9f1fbc4282ca6/src/librados/IoCtxImpl.h#L142-L143

Notes:

  • Since there isn't currently an implementation of XXHash in the Go standard library, these changes include a new import of https://pkg.go.dev/github.com/pierrec/xxHash in the testing package, in order to demonstrate compatibility and usage, and assert result accuracy on arbitrary data.
  • I opted to use sensible defaults and an options struct for non-required API parameters to make the API more expressive.

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

phlogistonjohn
phlogistonjohn reviewed Apr 29, 2026
View reviewed changes
Comment thread rados/ioctx_checksum.go Outdated
phlogistonjohn
phlogistonjohn reviewed Apr 29, 2026
View reviewed changes
Comment thread rados/ioctx_checksum.go Outdated
@phlogistonjohn phlogistonjohn added the API This PR includes a change to the public API of a go-ceph package label Apr 29, 2026
@phlogistonjohn
Copy link
Copy Markdown
Collaborator

phlogistonjohn commented Apr 29, 2026

Thanks for being so accepting of my suggestions. If you don't mind I like to ask another: please squash the commits into one. We generally prefer a linear history in go-ceph and would prefer the final versions of the code not reflect evolution of each change. (FWIW >1 commit is fine, but that should reflect steps to add fixes/features not intra-PR steps - if that makes any sense)

@naylorpmax-joyent
Copy link
Copy Markdown
Contributor Author

naylorpmax-joyent commented Apr 30, 2026

No problem! Updated (and sorry, I'd just assumed squash-on-merge)

phlogistonjohn
phlogistonjohn previously approved these changes Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown
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

@phlogistonjohn phlogistonjohn requested a review from anoopcs9 April 30, 2026 18:24
nixpanic
nixpanic reviewed May 1, 2026
View reviewed changes
Comment thread rados/ioctx_checksum.go Outdated
@mergify mergify Bot dismissed phlogistonjohn’s stale review May 1, 2026 10:13

Pull request has been modified.

anoopcs9
anoopcs9 reviewed May 1, 2026
View reviewed changes
Comment thread rados/ioctx_checksum.go Outdated
anoopcs9
anoopcs9 reviewed May 2, 2026
View reviewed changes
Copy link
Copy Markdown
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.

nitpick

I see mixed usage of assert.Nil() and assert.NoError() in the test files. Can we converge on a single style, preferably assert.NoError(), throughout the file?

Comment thread rados/ioctx_checksum_test.go Outdated
@naylorpmax-joyent naylorpmax-joyent force-pushed the rados-checksum branch 2 times, most recently from 332d2e1 to 2c41b01 Compare May 4, 2026 07:30
@naylorpmax-joyent naylorpmax-joyent requested a review from anoopcs9 May 4, 2026 16:47
anoopcs9
anoopcs9 previously approved these changes May 5, 2026
View reviewed changes
Copy link
Copy Markdown
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 with a minor comment.

Thanks for the swift updates.

Comment thread rados/ioctx_checksum_test.go
@mergify mergify Bot dismissed anoopcs9’s stale review May 5, 2026 15:44

Pull request has been modified.

@naylorpmax-joyent naylorpmax-joyent requested a review from anoopcs9 May 5, 2026 15:46
@phlogistonjohn
Copy link
Copy Markdown
Collaborator

phlogistonjohn commented May 5, 2026

@Mergifyio update

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 5, 2026

update

❌ Mergify doesn't have permission to update

Details

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/main.yml without workflows permission

@phlogistonjohn
Copy link
Copy Markdown
Collaborator

phlogistonjohn commented May 5, 2026

@Mergifyio rebase

@naylorpmax-joyent @phlogistonjohn
rados: Add support for rados_checksum API
616180b 
Add support for in-OSD calculations of object checksums using one of the supported algorithms:
* XXHash32
* XXHash64
* CRC32

Signed-off-by: Max Naylor <maxnaylor09@gmail.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 5, 2026

Deprecation notice: This pull request comes from a fork and was rebased using bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the rebase action will no longer be able to rebase fork pull requests with this configuration. Please switch to the update action/command to ensure compatibility going forward.

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 5, 2026

rebase

✅ Branch has been successfully rebased

@mergify mergify Bot added the queued label May 6, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 6, 2026 •
edited
Loading

Merge Queue Status

  • ✅ Entered queue — 2026-05-06 11:42 UTC · Rule: default
  • ✅ Checks skipped · PR is already up-to-date
  • ✅ Merged — 2026-05-06 11:42 UTC · at 616180bd1aaefbbcfe74c8dd11f1f893e051fb37 · rebase

This pull request spent 10 seconds in the queue, including 2 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = check
    • check-neutral = check
    • check-skipped = check
  • any of [🛡 GitHub branch protection]:
    • check-success = test-suite (pacific)
    • check-neutral = test-suite (pacific)
    • check-skipped = test-suite (pacific)
  • any of [🛡 GitHub branch protection]:
    • check-success = test-suite (quincy)
    • check-neutral = test-suite (quincy)
    • check-skipped = test-suite (quincy)
  • any of [🛡 GitHub branch protection]:
    • check-success = test-suite (reef)
    • check-neutral = test-suite (reef)
    • check-skipped = test-suite (reef)
  • any of [🛡 GitHub branch protection]:
    • check-success = test-suite (squid)
    • check-neutral = test-suite (squid)
    • check-skipped = test-suite (squid)

@mergify mergify Bot merged commit 6e969b9 into ceph:master May 6, 2026
14 of 15 checks passed
@mergify mergify Bot removed the queued label May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anoopcs9 anoopcs9 anoopcs9 approved these changes

@phlogistonjohn phlogistonjohn phlogistonjohn approved these changes

@nixpanic nixpanic Awaiting requested review from nixpanic

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@naylorpmax-joyent @phlogistonjohn @nixpanic @anoopcs9