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