From: Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
To: Xiubo Li <xiubli@redhat.com>, Ilya Dryomov <idryomov@gmail.com>
Cc: brauner@kernel.org, stgraber@ubuntu.com,
linux-fsdevel@vger.kernel.org, Jeff Layton <jlayton@kernel.org>,
ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 00/12] ceph: support idmapped mounts
Date: Mon, 23 Oct 2023 09:57:41 +0200 [thread overview]
Message-ID: <CAEivzxf56EXhNToVZRNZ9HsS4NKYidXqE-89oT6L-XY=s0nPcQ@mail.gmail.com> (raw)
In-Reply-To: <772a6282-d690-b299-6cf4-c96dd20792fa@redhat.com>
On Thu, Oct 19, 2023 at 7:42 AM Xiubo Li <xiubli@redhat.com> wrote:
>
>
> On 10/17/23 17:20, Aleksandr Mikhalitsyn wrote:
> > On Tue, Aug 8, 2023 at 2:45 AM Xiubo Li <xiubli@redhat.com> wrote:
> >> LGTM.
> >>
> >> Reviewed-by: Xiubo Li <xiubli@redhat.com>
> >>
> >> I will queue this to the 'testing' branch and then we will run ceph qa
> >> tests.
> >>
> >> Thanks Alex.
> >>
> >> - Xiubo
> > Hi Xiubo,
> >
> > will this series be landed to 6.6?
> >
> > Userspace part was backported and merged to the Ceph Quincy release
> > (https://github.com/ceph/ceph/pull/53139)
> > And waiting to be tested and merged to the Ceph reef and pacific releases.
> > But the kernel part is still in the testing branch.
>
> This changes have been in the 'testing' branch for more than two mounts
> and well test, till now we haven't seen any issue.
>
> IMO it should be ready.
Thanks, Xiubo!
It would be awesome to have this in v.6.6.
Kind regards,
Alex
>
> Ilya ?
>
> Thanks
>
> - Xiubo
>
>
> > Kind regards,
> > Alex
> >
> >> On 8/7/23 21:26, Alexander Mikhalitsyn wrote:
> >>> Dear friends,
> >>>
> >>> This patchset was originally developed by Christian Brauner but I'll continue
> >>> to push it forward. Christian allowed me to do that :)
> >>>
> >>> This feature is already actively used/tested with LXD/LXC project.
> >>>
> >>> Git tree (based on https://github.com/ceph/ceph-client.git testing):
> >>> v10: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v10
> >>> current: https://github.com/mihalicyn/linux/tree/fs.idmapped.ceph
> >>>
> >>> In the version 3 I've changed only two commits:
> >>> - fs: export mnt_idmap_get/mnt_idmap_put
> >>> - ceph: allow idmapped setattr inode op
> >>> and added a new one:
> >>> - ceph: pass idmap to __ceph_setattr
> >>>
> >>> In the version 4 I've reworked the ("ceph: stash idmapping in mdsc request")
> >>> commit. Now we take idmap refcounter just in place where req->r_mnt_idmap
> >>> is filled. It's more safer approach and prevents possible refcounter underflow
> >>> on error paths where __register_request wasn't called but ceph_mdsc_release_request is
> >>> called.
> >>>
> >>> Changelog for version 5:
> >>> - a few commits were squashed into one (as suggested by Xiubo Li)
> >>> - started passing an idmapping everywhere (if possible), so a caller
> >>> UID/GID-s will be mapped almost everywhere (as suggested by Xiubo Li)
> >>>
> >>> Changelog for version 6:
> >>> - rebased on top of testing branch
> >>> - passed an idmapping in a few places (readdir, ceph_netfs_issue_op_inline)
> >>>
> >>> Changelog for version 7:
> >>> - rebased on top of testing branch
> >>> - this thing now requires a new cephfs protocol extension CEPHFS_FEATURE_HAS_OWNER_UIDGID
> >>> https://github.com/ceph/ceph/pull/52575
> >>>
> >>> Changelog for version 8:
> >>> - rebased on top of testing branch
> >>> - added enable_unsafe_idmap module parameter to make idmapped mounts
> >>> work with old MDS server versions
> >>> - properly handled case when old MDS used with new kernel client
> >>>
> >>> Changelog for version 9:
> >>> - added "struct_len" field in struct ceph_mds_request_head as requested by Xiubo Li
> >>>
> >>> Changelog for version 10:
> >>> - fill struct_len field properly (use cpu_to_le32)
> >>> - add extra checks IS_CEPH_MDS_OP_NEWINODE(..) as requested by Xiubo to match
> >>> userspace client behavior
> >>> - do not set req->r_mnt_idmap for MKSNAP operation
> >>> - atomic_open: set req->r_mnt_idmap only for CEPH_MDS_OP_CREATE as userspace client does
> >>>
> >>> I can confirm that this version passes xfstests and
> >>> tested with old MDS (without CEPHFS_FEATURE_HAS_OWNER_UIDGID)
> >>> and with recent MDS version.
> >>>
> >>> Links to previous versions:
> >>> v1: https://lore.kernel.org/all/20220104140414.155198-1-brauner@kernel.org/
> >>> v2: https://lore.kernel.org/lkml/20230524153316.476973-1-aleksandr.mikhalitsyn@canonical.com/
> >>> tree: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v2
> >>> v3: https://lore.kernel.org/lkml/20230607152038.469739-1-aleksandr.mikhalitsyn@canonical.com/#t
> >>> v4: https://lore.kernel.org/lkml/20230607180958.645115-1-aleksandr.mikhalitsyn@canonical.com/#t
> >>> tree: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v4
> >>> v5: https://lore.kernel.org/lkml/20230608154256.562906-1-aleksandr.mikhalitsyn@canonical.com/#t
> >>> tree: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v5
> >>> v6: https://lore.kernel.org/lkml/20230609093125.252186-1-aleksandr.mikhalitsyn@canonical.com/
> >>> tree: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v6
> >>> v7: https://lore.kernel.org/all/20230726141026.307690-1-aleksandr.mikhalitsyn@canonical.com/
> >>> tree: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v7
> >>> v8: https://lore.kernel.org/all/20230803135955.230449-1-aleksandr.mikhalitsyn@canonical.com/
> >>> tree: -
> >>> v9: https://lore.kernel.org/all/20230804084858.126104-1-aleksandr.mikhalitsyn@canonical.com/
> >>> tree: https://github.com/mihalicyn/linux/commits/fs.idmapped.ceph.v9
> >>>
> >>> Kind regards,
> >>> Alex
> >>>
> >>> Original description from Christian:
> >>> ========================================================================
> >>> This patch series enables cephfs to support idmapped mounts, i.e. the
> >>> ability to alter ownership information on a per-mount basis.
> >>>
> >>> Container managers such as LXD support sharaing data via cephfs between
> >>> the host and unprivileged containers and between unprivileged containers.
> >>> They may all use different idmappings. Idmapped mounts can be used to
> >>> create mounts with the idmapping used for the container (or a different
> >>> one specific to the use-case).
> >>>
> >>> There are in fact more use-cases such as remapping ownership for
> >>> mountpoints on the host itself to grant or restrict access to different
> >>> users or to make it possible to enforce that programs running as root
> >>> will write with a non-zero {g,u}id to disk.
> >>>
> >>> The patch series is simple overall and few changes are needed to cephfs.
> >>> There is one cephfs specific issue that I would like to discuss and
> >>> solve which I explain in detail in:
> >>>
> >>> [PATCH 02/12] ceph: handle idmapped mounts in create_request_message()
> >>>
> >>> It has to do with how to handle mds serves which have id-based access
> >>> restrictions configured. I would ask you to please take a look at the
> >>> explanation in the aforementioned patch.
> >>>
> >>> The patch series passes the vfs and idmapped mount testsuite as part of
> >>> xfstests. To run it you will need a config like:
> >>>
> >>> [ceph]
> >>> export FSTYP=ceph
> >>> export TEST_DIR=/mnt/test
> >>> export TEST_DEV=10.103.182.10:6789:/
> >>> export TEST_FS_MOUNT_OPTS="-o name=admin,secret=$password
> >>>
> >>> and then simply call
> >>>
> >>> sudo ./check -g idmapped
> >>>
> >>> ========================================================================
> >>>
> >>> Alexander Mikhalitsyn (3):
> >>> fs: export mnt_idmap_get/mnt_idmap_put
> >>> ceph: add enable_unsafe_idmap module parameter
> >>> ceph: pass idmap to __ceph_setattr
> >>>
> >>> Christian Brauner (9):
> >>> ceph: stash idmapping in mdsc request
> >>> ceph: handle idmapped mounts in create_request_message()
> >>> ceph: pass an idmapping to mknod/symlink/mkdir
> >>> ceph: allow idmapped getattr inode op
> >>> ceph: allow idmapped permission inode op
> >>> ceph: allow idmapped setattr inode op
> >>> ceph/acl: allow idmapped set_acl inode op
> >>> ceph/file: allow idmapped atomic_open inode op
> >>> ceph: allow idmapped mounts
> >>>
> >>> fs/ceph/acl.c | 6 +--
> >>> fs/ceph/crypto.c | 2 +-
> >>> fs/ceph/dir.c | 4 ++
> >>> fs/ceph/file.c | 11 ++++-
> >>> fs/ceph/inode.c | 29 +++++++------
> >>> fs/ceph/mds_client.c | 78 ++++++++++++++++++++++++++++++++---
> >>> fs/ceph/mds_client.h | 8 +++-
> >>> fs/ceph/super.c | 7 +++-
> >>> fs/ceph/super.h | 3 +-
> >>> fs/mnt_idmapping.c | 2 +
> >>> include/linux/ceph/ceph_fs.h | 10 ++++-
> >>> include/linux/mnt_idmapping.h | 3 ++
> >>> 12 files changed, 136 insertions(+), 27 deletions(-)
> >>>
>
prev parent reply other threads:[~2023-10-23 7:58 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-07 13:26 [PATCH v10 00/12] ceph: support idmapped mounts Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 01/12] fs: export mnt_idmap_get/mnt_idmap_put Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 02/12] ceph: stash idmapping in mdsc request Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 03/12] ceph: handle idmapped mounts in create_request_message() Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 04/12] ceph: add enable_unsafe_idmap module parameter Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 05/12] ceph: pass an idmapping to mknod/symlink/mkdir Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 06/12] ceph: allow idmapped getattr inode op Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 07/12] ceph: allow idmapped permission " Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 08/12] ceph: pass idmap to __ceph_setattr Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 09/12] ceph: allow idmapped setattr inode op Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 10/12] ceph/acl: allow idmapped set_acl " Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 11/12] ceph/file: allow idmapped atomic_open " Alexander Mikhalitsyn
2023-08-07 13:26 ` [PATCH v10 12/12] ceph: allow idmapped mounts Alexander Mikhalitsyn
2023-08-07 13:40 ` [PATCH v10 00/12] ceph: support " Christian Brauner
2023-08-08 0:45 ` Xiubo Li
2023-08-08 6:30 ` Aleksandr Mikhalitsyn
2023-08-08 7:50 ` Xiubo Li
2023-10-17 9:20 ` Aleksandr Mikhalitsyn
2023-10-19 5:41 ` Xiubo Li
2023-10-23 7:57 ` Aleksandr Mikhalitsyn [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEivzxf56EXhNToVZRNZ9HsS4NKYidXqE-89oT6L-XY=s0nPcQ@mail.gmail.com' \
--to=aleksandr.mikhalitsyn@canonical.com \
--cc=brauner@kernel.org \
--cc=ceph-devel@vger.kernel.org \
--cc=idryomov@gmail.com \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stgraber@ubuntu.com \
--cc=xiubli@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).