Skip to content

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
mihalicyn:wip-63477-quincy
Jan 23, 2024
Merged

quincy: MClientRequest: properly handle ceph_mds_request_head_legacy for ext_num_retry, ext_num_fwd, owner_uid, owner_gid#54411
yuriw merged 3 commits intoceph:quincyfrom
mihalicyn:wip-63477-quincy

Conversation

@mihalicyn
Copy link
Contributor

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

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)
@mihalicyn
Copy link
Contributor Author

cc @vshankar

@github-actions github-actions bot added the cephfs Ceph File System label Nov 8, 2023
@github-actions github-actions bot added this to the quincy milestone Nov 8, 2023
@mihalicyn
Copy link
Contributor Author

jenkins test make check

1 similar comment
@mihalicyn
Copy link
Contributor Author

jenkins test make check

@mihalicyn
Copy link
Contributor Author

cc @rishabh-d-dave @lxbsz

Copy link
Contributor

@mchangir mchangir left a comment

Choose a reason for hiding this comment

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

@yuriw yuriw merged commit 257a229 into ceph:quincy Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants