Re: [PATCH v4 2/2] ceph: add ceph.{cluster_fsid/client_id} vxattrs suppport

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Nov 13, 2020 at 8:45 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Fri, 2020-11-13 at 11:40 -0800, Patrick Donnelly wrote:
> > On Tue, Nov 10, 2020 at 5:29 PM <xiubli@xxxxxxxxxx> wrote:
> > >
> > > From: Xiubo Li <xiubli@xxxxxxxxxx>
> > >
> > > These two vxattrs will only exist in local client side, with which
> > > we can easily know which mountpoint the file belongs to and also
> > > they can help locate the debugfs path quickly.
> > >
> > > URL: https://tracker.ceph.com/issues/48057
> > > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> > > ---
> > >  fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >
> > > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> > > index 0fd05d3d4399..e89750a1f039 100644
> > > --- a/fs/ceph/xattr.c
> > > +++ b/fs/ceph/xattr.c
> > > @@ -304,6 +304,23 @@ static ssize_t ceph_vxattrcb_snap_btime(struct ceph_inode_info *ci, char *val,
> > >                                 ci->i_snap_btime.tv_nsec);
> > >  }
> > >
> > > +static ssize_t ceph_vxattrcb_cluster_fsid(struct ceph_inode_info *ci,
> > > +                                         char *val, size_t size)
> > > +{
> > > +       struct ceph_fs_client *fsc = ceph_sb_to_client(ci->vfs_inode.i_sb);
> > > +
> > > +       return ceph_fmt_xattr(val, size, "%pU", &fsc->client->fsid);
> > > +}
> > > +
> > > +static ssize_t ceph_vxattrcb_client_id(struct ceph_inode_info *ci,
> > > +                                      char *val, size_t size)
> > > +{
> > > +       struct ceph_fs_client *fsc = ceph_sb_to_client(ci->vfs_inode.i_sb);
> > > +
> > > +       return ceph_fmt_xattr(val, size, "client%lld",
> > > +                             ceph_client_gid(fsc->client));
> > > +}
> >
> > Let's just have this return the id number. The caller can concatenate
> > that with "client" however they require. Otherwise, parsing it out is
> > needlessly troublesome.
> >
>
> That does seem nicer, but it would be inconsistent with the existing rbd
> attributes. Ilya, would you object?

I'm not married to it, but staying consistent would be good.  Yes,
it concatenates with "client" and yes, without a dot, but the kernel
client has been doing that for a very long time.  This behaviour
actually predates the rbd attributes I mentioned.

>
> > >  #define CEPH_XATTR_NAME(_type, _name)  XATTR_CEPH_PREFIX #_type "." #_name
> > >  #define CEPH_XATTR_NAME2(_type, _name, _name2) \
> > >         XATTR_CEPH_PREFIX #_type "." #_name "." #_name2
> > > @@ -407,6 +424,24 @@ static struct ceph_vxattr ceph_file_vxattrs[] = {
> > >         { .name = NULL, 0 }     /* Required table terminator */
> > >  };
> > >
> > > +static struct ceph_vxattr ceph_common_vxattrs[] = {
> > > +       {
> > > +               .name = "ceph.cluster_fsid",
> > > +               .name_size = sizeof("ceph.cluster_fsid"),
> > > +               .getxattr_cb = ceph_vxattrcb_cluster_fsid,
> > > +               .exists_cb = NULL,
> > > +               .flags = VXATTR_FLAG_READONLY,
> > > +       },
> >
> > I would prefer just "ceph.fsid".
> >
>
> Ilya suggested this name to match existing rbd attributes.

I think "cluster" is particularly important here because people
might mistake it for a filesystem identifier.

Thanks,

                Ilya



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux