Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230524153316.476973-1-aleksandr.mikhalitsyn@canonical.com>
 <20230524153316.476973-11-aleksandr.mikhalitsyn@canonical.com>
 <b3b1b8dc-9903-c4ff-0a63-9a31a311ff0b@redhat.com> <CAEivzxfxug8kb7_SzJGvEZMcYwGM8uW25gKa_osFqUCpF_+Lhg@mail.gmail.com>
 <20230602-vorzeichen-praktikum-f17931692301@brauner> <CAEivzxcwTbOUrT2ha8fR=wy-bU1+ZppapnMsqVXBXAc+C0gwhw@mail.gmail.com>
 <20230602-behoben-tauglich-b6ecd903f2a9@brauner>
In-Reply-To: <20230602-behoben-tauglich-b6ecd903f2a9@brauner>
From:   Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Date:   Fri, 2 Jun 2023 15:15:12 +0200
Message-ID: <CAEivzxfOgiQjXob+J1S5MsBFJjDGX_hApD_xR1s7q-S9eQh_bw@mail.gmail.com>
Subject: Re: [PATCH v2 10/13] ceph: allow idmapped setattr inode op
To:     Christian Brauner <brauner@kernel.org>
Cc:     Xiubo Li <xiubli@redhat.com>, stgraber@ubuntu.com,
        linux-fsdevel@vger.kernel.org, Jeff Layton <jlayton@kernel.org>,
        Ilya Dryomov <idryomov@gmail.com>, ceph-devel@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jun 2, 2023 at 3:08=E2=80=AFPM Christian Brauner <brauner@kernel.or=
g> wrote:
>
> On Fri, Jun 02, 2023 at 03:05:50PM +0200, Aleksandr Mikhalitsyn wrote:
> > On Fri, Jun 2, 2023 at 2:54=E2=80=AFPM Christian Brauner <brauner@kerne=
l.org> wrote:
> > >
> > > On Fri, Jun 02, 2023 at 02:45:30PM +0200, Aleksandr Mikhalitsyn wrote=
:
> > > > On Fri, Jun 2, 2023 at 3:30=E2=80=AFAM Xiubo Li <xiubli@redhat.com>=
 wrote:
> > > > >
> > > > >
> > > > > On 5/24/23 23:33, Alexander Mikhalitsyn wrote:
> > > > > > From: Christian Brauner <christian.brauner@ubuntu.com>
> > > > > >
> > > > > > Enable __ceph_setattr() to handle idmapped mounts. This is just=
 a matter
> > > > > > of passing down the mount's idmapping.
> > > > > >
> > > > > > Cc: Jeff Layton <jlayton@kernel.org>
> > > > > > Cc: Ilya Dryomov <idryomov@gmail.com>
> > > > > > Cc: ceph-devel@vger.kernel.org
> > > > > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > > > > > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@can=
onical.com>
> > > > > > ---
> > > > > >   fs/ceph/inode.c | 11 +++++++++--
> > > > > >   1 file changed, 9 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > > > > > index 37e1cbfc7c89..f1f934439be0 100644
> > > > > > --- a/fs/ceph/inode.c
> > > > > > +++ b/fs/ceph/inode.c
> > > > > > @@ -2050,6 +2050,13 @@ int __ceph_setattr(struct inode *inode, =
struct iattr *attr)
> > > > > >
> > > > > >       dout("setattr %p issued %s\n", inode, ceph_cap_string(iss=
ued));
> > > > > >
> > > > > > +     /*
> > > > > > +      * The attr->ia_{g,u}id members contain the target {g,u}i=
d we're
> > >
> > > This is now obsolete... In earlier imlementations attr->ia_{g,u}id wa=
s
> > > used and contained the filesystem wide value, not the idmapped mount
> > > value.
> > >
> > > However, this was misleading and we changed that in commit b27c82e129=
65
> > > ("attr: port attribute changes to new types") and introduced dedicate=
d
> > > new types into struct iattr->ia_vfs{g,u}id. So the you need to use
> > > attr->ia_vfs{g,u}id as documented in include/linux/fs.h and you need =
to
> > > transform them into filesystem wide values and then to raw values you
> > > send over the wire.
> > >
> > > Alex should be able to figure this out though.
> >
> > Hi Christian,
> >
> > Thanks for pointing this out. Unfortunately I wasn't able to notice
> > that. I'll take a look closer and fix that.
>
> Just to clarify: I wasn't trying to imply that you should've figured
> this out on your own. I was just trying to say that you should be able
> figure out the exact details how to implement this in ceph after I told
> you about the attr->ia_vfs{g,u}id change.

No problem, I've got your idea the same as you explained it ;-)
I'll rework that place and I will recheck that we pass xfstests after that.