quincy: MClientRequest: properly handle ceph_mds_request_head_legacy for ext_num_retry, ext_num_fwd, owner_uid, owner_gid#54411
Merged
yuriw merged 3 commits intoceph:quincyfrom Jan 23, 2024
Conversation
This patch adds separate fields to pass inode owner's UID/GID
for operations which create new inodes:
CEPH_MDS_OP_CREATE, CEPH_MDS_OP_MKNOD, CEPH_MDS_OP_RENAME,
CEPH_MDS_OP_MKDIR, CEPH_MDS_OP_SYMLINK.
Before we have used caller_{u,g}id fields for two purposes:
- for MDS permission checks
- to set UID/GID for new inodes
but during a long discussion around adding idmapped mounts
support to cephfs kernel client we decided to extend wire protocol [1].
Originally, Christian Brauner pointed to this problem [2], but
a few initial versions of "ceph: support idmapped mounts" patchset
were based on a compromise between bringing idmapped mounts support
and changing on-wire data structures. The only problem with this approach
is that if we have UID/GID-based permission checks on the MDS side they
won't work as intended. Xiubo Li pointed to this issue and said that
it's critical enough to not being ignored.
This patch does not change behavior for client or MDS, because
in the current client implementation we do:
req->set_inode_owner_uid_gid(perms.uid(), perms.gid());
so, these new fields contain the same value as caller_{u,g}id.
This new extension will be used in a non-trivial way in the Linux kernel client.
[1] https://lore.kernel.org/lkml/CAEivzxcipdTQ9-b2zk-7-AMDfwPk2Brtp=X6H8xXsHMEtQJKFQ@mail.gmail.com/
[2] https://lore.kernel.org/all/20220104140414.155198-3-brauner@kernel.org/
Fixes: https://tracker.ceph.com/issues/62217
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
(cherry picked from commit 46cb244)
…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> (cherry picked from commit 43f32a4)
…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> (cherry picked from commit a70a70f)
Contributor
Author
|
cc @vshankar |
Contributor
Author
|
jenkins test make check |
1 similar comment
Contributor
Author
|
jenkins test make check |
Contributor
Author
vshankar
approved these changes
Dec 13, 2023
mchangir
approved these changes
Jan 23, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR also restores another backport #53139 because it was reverted #54108
backport tracker: https://tracker.ceph.com/issues/63477
backport of #54149
parent tracker: https://tracker.ceph.com/issues/63288