mds: enforce usage of host error in cephfs, use errorcode32_t in MClientReply message by salieri11 · Pull Request #60286 · ceph/ceph

salieri11 · 2024-10-13T09:20:10Z

Use errorcode32_t struct in MClientReply message

This struct enforces converting error code from host to ceph and back, while encoding and decoding messages, respectelvly.

Enforce usage of E* error codes in the cephfs code instead of CEPHFS_E* error codes

cephfs code now use host error codes instead of CEPHFS_* error codes

Fixes https://tracker.ceph.com/issues/64611

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

salieri11 · 2024-10-13T17:51:42Z

jenkins test make check

salieri11 · 2024-10-15T07:46:18Z

jenkins test make check arm64

dparmar18 · 2024-10-15T07:49:28Z

(just an extreme nit) s/cephhfs/cephfs in the commit title

src/messages/MClientReply.h

github-actions · 2024-10-16T13:56:25Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

salieri11 · 2024-10-19T14:59:41Z

jenkins test make check

salieri11 · 2024-10-19T19:18:45Z

jenkins test make check

github-actions · 2024-11-04T09:36:22Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

dparmar18 · 2025-02-03T17:11:08Z

LGTM, just need the above two comments to be addressed.

batrick

Please annotate the commit message which fixes/resolves a Ceph tracker issue with:

Fixes: http://tracker.ceph.com/issues/...

This is essential when examining the history of the repository (this commit fixes what) and helps merge scripts identify issues that have been resolved by a merge. See this article on GitHub on how to amend commits and update your pull request.

batrick

Template saved-reply: Please prefix the title of your commit message with the sub-component you are changing. This is required for your PR to be merged.

See this article on GitHub on how to amend commits and update your pull request.

Specific-comment: Your PR would be best split into multiple commits with fixes by each component. Large commits are hard to navigate/review. I also asked for explanations for some changes in previous reviews to be put into commit messages. I saw you left a github comment but what I would have preferred is that you put that response in the commit message and told me to read it there.

When refactoring large change-sets into multiple commits, use git reset -p and git rebase -i to help you restructure the commits. (If you're not already familiar...)

batrick

The rest of the changes look good -- thanks!

PendingReleaseNotes

src/include/ceph_fs.h

src/messages/MClientReply.h

salieri11 · 2025-02-12T14:27:48Z

jenkins test make check

salieri11 · 2025-02-16T08:09:15Z

jenkins test make check

salieri11 · 2025-02-16T11:44:36Z

jenkins test make check

batrick

please rebase + squash mds: fix merge error and release notes: explain that old CEPHFS_EXXX code were removed

release notes,python bindings: add note, change pybindings

split into two commits:

PendingReleaseNotes: add note on client/mds error codes
pybind/cephfs: switch CEPHFS_E error codes to system

ceph-build-deps_15.2.0-1_amd64.buildinfo

src/messages/MClientReply.h

Signed-off-by: Igor Golikov <[email protected]> Fixes: https://tracker.ceph.com/issues/64611

salieri11 · 2025-02-18T19:24:42Z

jenkins test api

github-actions bot added cephfs Ceph File System common pybind tests labels Oct 13, 2024

salieri11 force-pushed the igolikov-wip-bug-64611 branch from a6637a5 to 7fc4437 Compare October 13, 2024 16:28

salieri11 changed the title ~~enforce usage of errorcode_32t in cephhfs~~ mds: enforce usage of host error in cephhfs, use errorcode32_t in MClientReply message Oct 13, 2024

salieri11 force-pushed the igolikov-wip-bug-64611 branch from 7fc4437 to a57df10 Compare October 14, 2024 10:14

vshankar requested a review from a team October 14, 2024 15:59

salieri11 force-pushed the igolikov-wip-bug-64611 branch from 2cfc3fe to bee80a4 Compare October 14, 2024 18:40

salieri11 changed the title ~~mds: enforce usage of host error in cephhfs, use errorcode32_t in MClientReply message~~ mds: enforce usage of host error in cephfs, use errorcode32_t in MClientReply message Oct 15, 2024

dparmar18 reviewed Oct 15, 2024

View reviewed changes

src/messages/MClientReply.h Outdated Show resolved Hide resolved

github-actions bot added the needs-rebase label Oct 16, 2024

salieri11 force-pushed the igolikov-wip-bug-64611 branch from bee80a4 to 1c253db Compare October 19, 2024 11:35

github-actions bot removed the needs-rebase label Oct 19, 2024

salieri11 added the needs-qa label Oct 19, 2024

salieri11 force-pushed the igolikov-wip-bug-64611 branch from 76d7f55 to 311f62b Compare October 29, 2024 09:03

vshankar assigned batrick, rishabh-d-dave and dparmar18 Oct 29, 2024

salieri11 force-pushed the igolikov-wip-bug-64611 branch 3 times, most recently from 9d6291b to c967986 Compare November 1, 2024 16:36

github-actions bot added the needs-rebase label Nov 4, 2024

salieri11 force-pushed the igolikov-wip-bug-64611 branch from fcbf45c to 3848992 Compare February 5, 2025 09:32

batrick requested changes Feb 5, 2025

View reviewed changes

PendingReleaseNotes Show resolved Hide resolved

src/include/ceph_fs.h Show resolved Hide resolved

batrick requested changes Feb 5, 2025

View reviewed changes

src/messages/MClientReply.h Outdated Show resolved Hide resolved

github-actions bot added the needs-rebase label Feb 11, 2025

salieri11 force-pushed the igolikov-wip-bug-64611 branch 2 times, most recently from 8eabaed to 8f551cb Compare February 12, 2025 09:15

github-actions bot removed the needs-rebase label Feb 12, 2025

salieri11 force-pushed the igolikov-wip-bug-64611 branch from 8f551cb to 86a66b0 Compare February 13, 2025 10:16

salieri11 requested a review from batrick February 16, 2025 17:45

ceph deleted a comment from github-actions bot Feb 17, 2025

batrick requested changes Feb 17, 2025

View reviewed changes

ceph-build-deps_15.2.0-1_amd64.buildinfo Outdated Show resolved Hide resolved

src/messages/MClientReply.h Show resolved Hide resolved

salieri11 added 4 commits February 18, 2025 11:36

PendingReleaseNotes: add note on client/mds error codes

54ce979

Signed-off-by: Igor Golikov <[email protected]> Fixes: https://tracker.ceph.com/issues/64611

pybind/cephfs: switch CEPHFS_E error codes to system error codes

b1b2716

Signed-off-by: Igor Golikov <[email protected]> Fixes: https://tracker.ceph.com/issues/64611

mds: prepare infra to replace CEPHFS_EXXX codes to system codes

4fe51a1

Signed-off-by: Igor Golikov <[email protected]> Fixes: https://tracker.ceph.com/issues/64611

mds,client: change result field handling in the MClientReply message

bd3b248

Signed-off-by: Igor Golikov <[email protected]> Fixes: https://tracker.ceph.com/issues/64611

salieri11 force-pushed the igolikov-wip-bug-64611 branch from 86a66b0 to 90a3ebb Compare February 18, 2025 11:50

mds: use system error codes instead of CEPHFS_EXX codes

7055fa5

Signed-off-by: Igor Golikov <[email protected]> Fixes: https://tracker.ceph.com/issues/64611

salieri11 force-pushed the igolikov-wip-bug-64611 branch from 90a3ebb to 7055fa5 Compare February 18, 2025 12:48

batrick approved these changes Feb 18, 2025

View reviewed changes

salieri11 merged commit 5b16367 into ceph:main Feb 19, 2025
12 checks passed

This was referenced Feb 20, 2025

reef: mds: enforce usage of host error in cephfs, use errorcode32_t in MClientReply message #61940

Open

squid: mds: enforce usage of host error in cephfs, use errorcode32_t in MClientReply message #61994

Merged

joscollin mentioned this pull request Sep 25, 2025

reef: mds: enforce usage of host error in cephfs, use errorcode32_t in MClientReply message #65668

Closed

Comments

Conversation

salieri11 commented Oct 13, 2024

Contribution Guidelines

Checklist

Uh oh!

salieri11 commented Oct 13, 2024

Uh oh!

salieri11 commented Oct 15, 2024

Uh oh!

dparmar18 commented Oct 15, 2024

Uh oh!

Uh oh!

github-actions bot commented Oct 16, 2024

Uh oh!

salieri11 commented Oct 19, 2024

Uh oh!

salieri11 commented Oct 19, 2024

Uh oh!

github-actions bot commented Nov 4, 2024

Uh oh!

dparmar18 commented Feb 3, 2025

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

salieri11 commented Feb 12, 2025

Uh oh!

salieri11 commented Feb 16, 2025

Uh oh!

salieri11 commented Feb 16, 2025

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

salieri11 commented Feb 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants