Skip to content

MClientRequest: properly handle ceph_mds_request_head_legacy for ext_num_retry, ext_num_fwd, owner_uid, owner_gid#54149

Merged
rishabh-d-dave merged 2 commits intoceph:mainfrom
mihalicyn:ceph_mds_request_head_legacy_fallback_fix
Nov 7, 2023
Merged

MClientRequest: properly handle ceph_mds_request_head_legacy for ext_num_retry, ext_num_fwd, owner_uid, owner_gid#54149
rishabh-d-dave merged 2 commits intoceph:mainfrom
mihalicyn:ceph_mds_request_head_legacy_fallback_fix

Conversation

@mihalicyn
Copy link
Contributor

@mihalicyn mihalicyn commented Oct 23, 2023 •
edited
Loading

Fix for #53139 (comment)

Reproducer:

  1. use old enough Linux kernel ceph client (Linux kernel v5.4 works fine for this purpose)
  2. mount ceph on /mnt/ceph
  3. touch /mnt/ceph/123
  4. MDS will crash with assertion on ceph_assert(_inode->gid != (unsigned)-1).

https://tracker.ceph.com/issues/63288

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)
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

@mihalicyn mihalicyn force-pushed the ceph_mds_request_head_legacy_fallback_fix branch from 4e6310b to 2a183e6 Compare October 23, 2023 13:47
@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 23, 2023

cc @lxbsz

@mihalicyn mihalicyn mentioned this pull request Oct 23, 2023
Merged
@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 23, 2023

jenkins retest this please

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 23, 2023

I'm not sure if we want to incorporate fix from #54121 to this PR.

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 23, 2023

jenkins test make check arm64

@mihalicyn mihalicyn mentioned this pull request Oct 23, 2023
Closed
14 tasks
lxbsz
lxbsz approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@lxbsz lxbsz left a comment

Choose a reason for hiding this comment

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

Good catch. LDTM.

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 24, 2023

jenkins test make check arm64

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 24, 2023

jenkins test api

@mihalicyn
MClientRequest: handle ext_num_retry and ext_num_fwd from ceph_mds_re…
43f32a4 
…quest_head_legacy

When a client is too old and uses struct ceph_mds_request_head_legacy we must
fill new ext_num_retry and ext_num_fwd fields from an old num_retry and num_fwd.

Fixes: ceph#45669
Fixes: https://tracker.ceph.com/issues/63288
Fixes: commit cbd7e30 ("ceph_fs.h: add 32 bits extended num_retry and num_fwd support")

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
@mihalicyn
MClientRequest: handle owner_uid and owner_gid from ceph_mds_request_…
a70a70f 
…head_legacy

When a client is too old and uses struct ceph_mds_request_head_legacy we must
fill new owner_uid and owner_gid fields from an old client_uid and client_gid.

Fixes: ceph#52575
Fixes: https://tracker.ceph.com/issues/63288
Fixes: commit 46cb244 ("ceph_fs.h: add separate owner_{u,g}id fields")

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
@mihalicyn mihalicyn force-pushed the ceph_mds_request_head_legacy_fallback_fix branch from 2a183e6 to a70a70f Compare October 24, 2023 14:08
@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 24, 2023

Rebased and added "Fixes" tags to the commit descriptions.

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 24, 2023 •
edited
Loading

jenkins test make check arm64

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 24, 2023

jenkins test api

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 25, 2023

jenkins test make check arm64

2 similar comments
@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 25, 2023

jenkins test make check arm64

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 25, 2023

jenkins test make check arm64

@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 25, 2023

cc @batrick

@batrick batrick added cephfs Ceph File System needs-qa labels Oct 25, 2023
@mihalicyn
Copy link
Contributor Author

mihalicyn commented Oct 30, 2023

cc @vshankar

@rishabh-d-dave rishabh-d-dave added the wip-rishabh-testing Rishabh's testing label label Nov 1, 2023
vshankar
vshankar approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

LGTM

rishabh-d-dave
rishabh-d-dave approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave left a comment

Choose a reason for hiding this comment

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

CephFS integration tests were successful - https://tracker.ceph.com/projects/cephfs/wiki/Main#7-Nov-2023

@rishabh-d-dave rishabh-d-dave merged commit 665e692 into ceph:main Nov 7, 2023
This was referenced Nov 8, 2023
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vshankar vshankar vshankar approved these changes

@batrick batrick batrick approved these changes

@rishabh-d-dave rishabh-d-dave rishabh-d-dave approved these changes

@lxbsz lxbsz lxbsz approved these changes

Assignees

No one assigned

Labels

cephfs Ceph File System wip-rishabh-testing Rishabh's testing label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mihalicyn @vshankar @batrick @rishabh-d-dave @lxbsz