[PATCH v3 0/2] ceph: add vxattrs to get ids and status debug file support

[PATCH v3 0/2] ceph: add vxattrs to get ids and status debug file support

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

V3:
- switch ioctl to vxattr.

V2:
- some typo fixings
- switch to use ceph_client_gid() and ceph_client_addr() helpers
- for ioctl cmd will return in text for cluster/client ids

Xiubo Li (2):
  ceph: add status debug file support
  ceph: add ceph.{clusterid/clientid} vxattrs suppport

 fs/ceph/debugfs.c | 20 ++++++++++++++++++++
 fs/ceph/super.h   |  1 +
 fs/ceph/xattr.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)

-- 
2.27.0

[PATCH v3 1/2] ceph: add status debug file support

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

This will help list some useful client side info, like the client
entity address/name and bloclisted status, etc.

URL: https://tracker.ceph.com/issues/48057
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 fs/ceph/debugfs.c | 20 ++++++++++++++++++++
 fs/ceph/super.h   |  1 +
 2 files changed, 21 insertions(+)

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 7a8fbe3e4751..4e498a492de4 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -304,11 +304,25 @@ static int mds_sessions_show(struct seq_file *s, void *ptr)
 	return 0;
 }
 
+static int status_show(struct seq_file *s, void *p)
+{
+	struct ceph_fs_client *fsc = s->private;
+	struct ceph_entity_inst *inst = &fsc->client->msgr.inst;
+	struct ceph_entity_addr *client_addr = ceph_client_addr(fsc->client);
+
+	seq_printf(s, "inst_str: %s.%lld %s/%u\n", ENTITY_NAME(inst->name),
+		   ceph_pr_addr(client_addr), le32_to_cpu(client_addr->nonce));
+	seq_printf(s, "blocklisted: %s\n", fsc->blocklisted ? "true" : "false");
+
+	return 0;
+}
+
 DEFINE_SHOW_ATTRIBUTE(mdsmap);
 DEFINE_SHOW_ATTRIBUTE(mdsc);
 DEFINE_SHOW_ATTRIBUTE(caps);
 DEFINE_SHOW_ATTRIBUTE(mds_sessions);
 DEFINE_SHOW_ATTRIBUTE(metric);
+DEFINE_SHOW_ATTRIBUTE(status);
 
 
 /*
@@ -394,6 +408,12 @@ void ceph_fs_debugfs_init(struct ceph_fs_client *fsc)
 						fsc->client->debugfs_dir,
 						fsc,
 						&caps_fops);
+
+	fsc->debugfs_status = debugfs_create_file("status",
+						  0400,
+						  fsc->client->debugfs_dir,
+						  fsc,
+						  &status_fops);
 }
 
 
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index f097237a5ad3..5138b75923f9 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -128,6 +128,7 @@ struct ceph_fs_client {
 	struct dentry *debugfs_bdi;
 	struct dentry *debugfs_mdsc, *debugfs_mdsmap;
 	struct dentry *debugfs_metric;
+	struct dentry *debugfs_status;
 	struct dentry *debugfs_mds_sessions;
 #endif
 
-- 
2.27.0

Re: [PATCH v3 1/2] ceph: add status debug file support

[Jeff Layton]

On Tue, 2020-11-10 at 22:17 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> This will help list some useful client side info, like the client
> entity address/name and bloclisted status, etc.
> 

"blocklisted"

> URL: https://tracker.ceph.com/issues/48057
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/debugfs.c | 20 ++++++++++++++++++++
>  fs/ceph/super.h   |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
> index 7a8fbe3e4751..4e498a492de4 100644
> --- a/fs/ceph/debugfs.c
> +++ b/fs/ceph/debugfs.c
> @@ -304,11 +304,25 @@ static int mds_sessions_show(struct seq_file *s, void *ptr)
>  	return 0;
>  }
>  
> 
> 
> 
> 
> 
> 
> 
> +static int status_show(struct seq_file *s, void *p)
> +{
> +	struct ceph_fs_client *fsc = s->private;
> +	struct ceph_entity_inst *inst = &fsc->client->msgr.inst;
> +	struct ceph_entity_addr *client_addr = ceph_client_addr(fsc->client);
> +
> +	seq_printf(s, "inst_str: %s.%lld %s/%u\n", ENTITY_NAME(inst->name),

nit: maybe we should call the first field "instance:"

I'll go ahead and fix this up as I merge it. You don't need to resend.

> +		   ceph_pr_addr(client_addr), le32_to_cpu(client_addr->nonce));
> +	seq_printf(s, "blocklisted: %s\n", fsc->blocklisted ? "true" : "false");
> +
> +	return 0;
> +}
> +
>  DEFINE_SHOW_ATTRIBUTE(mdsmap);
>  DEFINE_SHOW_ATTRIBUTE(mdsc);
>  DEFINE_SHOW_ATTRIBUTE(caps);
>  DEFINE_SHOW_ATTRIBUTE(mds_sessions);
>  DEFINE_SHOW_ATTRIBUTE(metric);
> +DEFINE_SHOW_ATTRIBUTE(status);
>  
> 
> 
> 
>  
> 
> 
> 
>  /*
> @@ -394,6 +408,12 @@ void ceph_fs_debugfs_init(struct ceph_fs_client *fsc)
>  						fsc->client->debugfs_dir,
>  						fsc,
>  						&caps_fops);
> +
> +	fsc->debugfs_status = debugfs_create_file("status",
> +						  0400,
> +						  fsc->client->debugfs_dir,
> +						  fsc,
> +						  &status_fops);
>  }
>  
> 
> 
> 
>  
> 
> 
> 
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index f097237a5ad3..5138b75923f9 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -128,6 +128,7 @@ struct ceph_fs_client {
>  	struct dentry *debugfs_bdi;
>  	struct dentry *debugfs_mdsc, *debugfs_mdsmap;
>  	struct dentry *debugfs_metric;
> +	struct dentry *debugfs_status;
>  	struct dentry *debugfs_mds_sessions;
>  #endif
>  
> 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3 1/2] ceph: add status debug file support

[Patrick Donnelly]

On Tue, Nov 10, 2020 at 6:17 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> This will help list some useful client side info, like the client
> entity address/name and bloclisted status, etc.
>
> URL: https://tracker.ceph.com/issues/48057

Thanks for working on this Xiubo. Do we have an Ceph PR for updating
the QA bits to use this?

-- 
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D

Re: [PATCH v3 1/2] ceph: add status debug file support

[Xiubo Li]

On 2020/11/14 3:42, Patrick Donnelly wrote:
> On Tue, Nov 10, 2020 at 6:17 AM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> This will help list some useful client side info, like the client
>> entity address/name and bloclisted status, etc.
>>
>> URL: https://tracker.ceph.com/issues/48057
> Thanks for working on this Xiubo. Do we have an Ceph PR for updating
> the QA bits to use this?
>
Not yet. I will raise one.

Thanks

[PATCH v3 2/2] ceph: add ceph.{clusterid/clientid} vxattrs suppport

[xiubli]

From: Xiubo Li <xiubli@redhat.com>

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@redhat.com>
---
 fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 0fd05d3d4399..4a41db46e191 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_clusterid(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_clientid(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));
+}
+
 #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_vxattrs[] = {
+	{
+		.name = "ceph.clusterid",
+		.name_size = sizeof("ceph.clusterid"),
+		.getxattr_cb = ceph_vxattrcb_clusterid,
+		.exists_cb = NULL,
+		.flags = VXATTR_FLAG_READONLY,
+	},
+	{
+		.name = "ceph.clientid",
+		.name_size = sizeof("ceph.clientid"),
+		.getxattr_cb = ceph_vxattrcb_clientid,
+		.exists_cb = NULL,
+		.flags = VXATTR_FLAG_READONLY,
+	},
+	{ .name = NULL, 0 }	/* Required table terminator */
+};
+
 static struct ceph_vxattr *ceph_inode_vxattrs(struct inode *inode)
 {
 	if (S_ISDIR(inode->i_mode))
@@ -429,6 +464,13 @@ static struct ceph_vxattr *ceph_match_vxattr(struct inode *inode,
 		}
 	}
 
+	vxattr = ceph_vxattrs;
+	while (vxattr->name) {
+		if (!strcmp(vxattr->name, name))
+			return vxattr;
+		vxattr++;
+	}
+
 	return NULL;
 }
 
-- 
2.27.0

Re: [PATCH v3 2/2] ceph: add ceph.{clusterid/clientid} vxattrs suppport

[Jeff Layton]

On Tue, 2020-11-10 at 22:17 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> 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@redhat.com>
> ---
>  fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 0fd05d3d4399..4a41db46e191 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_clusterid(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_clientid(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));
> +}
> +
>  #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_vxattrs[] = {

I'm going to rename this to "ceph_common_vxattrs".

> +	{
> +		.name = "ceph.clusterid",
> +		.name_size = sizeof("ceph.clusterid"),
> +		.getxattr_cb = ceph_vxattrcb_clusterid,
> +		.exists_cb = NULL,
> +		.flags = VXATTR_FLAG_READONLY,
> +	},
> +	{
> +		.name = "ceph.clientid",
> +		.name_size = sizeof("ceph.clientid"),
> +		.getxattr_cb = ceph_vxattrcb_clientid,
> +		.exists_cb = NULL,
> +		.flags = VXATTR_FLAG_READONLY,
> +	},
> +	{ .name = NULL, 0 }	/* Required table terminator */
> +};
> +
>  static struct ceph_vxattr *ceph_inode_vxattrs(struct inode *inode)
>  {
>  	if (S_ISDIR(inode->i_mode))
> @@ -429,6 +464,13 @@ static struct ceph_vxattr *ceph_match_vxattr(struct inode *inode,
>  		}
>  	}
>  
> 
> 
> 
> +	vxattr = ceph_vxattrs;
> +	while (vxattr->name) {
> +		if (!strcmp(vxattr->name, name))
> +			return vxattr;
> +		vxattr++;
> +	}
> +
>  	return NULL;
>  }
>  
> 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3 2/2] ceph: add ceph.{clusterid/clientid} vxattrs suppport

[Ilya Dryomov]

On Tue, Nov 10, 2020 at 3:17 PM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> 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@redhat.com>
> ---
>  fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 0fd05d3d4399..4a41db46e191 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_clusterid(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_clientid(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));
> +}
> +
>  #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_vxattrs[] = {
> +       {
> +               .name = "ceph.clusterid",

I think this should be "ceph.cluster_fsid"

> +               .name_size = sizeof("ceph.clusterid"),
> +               .getxattr_cb = ceph_vxattrcb_clusterid,
> +               .exists_cb = NULL,
> +               .flags = VXATTR_FLAG_READONLY,
> +       },
> +       {
> +               .name = "ceph.clientid",

and this should be "ceph.client_id".  It's easier to read, consistent
with "ceph fsid" command and with existing rbd attributes:

  static DEVICE_ATTR(client_id, 0444, rbd_client_id_show, NULL);
  static DEVICE_ATTR(cluster_fsid, 0444, rbd_cluster_fsid_show, NULL);

Thanks,

                Ilya

Re: [PATCH v3 2/2] ceph: add ceph.{clusterid/clientid} vxattrs suppport

[Jeff Layton]

On Tue, 2020-11-10 at 16:59 +0100, Ilya Dryomov wrote:
> On Tue, Nov 10, 2020 at 3:17 PM <xiubli@redhat.com> wrote:
> > 
> > From: Xiubo Li <xiubli@redhat.com>
> > 
> > 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@redhat.com>
> > ---
> >  fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> > index 0fd05d3d4399..4a41db46e191 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_clusterid(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_clientid(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));
> > +}
> > +
> >  #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_vxattrs[] = {
> > +       {
> > +               .name = "ceph.clusterid",
> 
> I think this should be "ceph.cluster_fsid"
> 
> > +               .name_size = sizeof("ceph.clusterid"),
> > +               .getxattr_cb = ceph_vxattrcb_clusterid,
> > +               .exists_cb = NULL,
> > +               .flags = VXATTR_FLAG_READONLY,
> > +       },
> > +       {
> > +               .name = "ceph.clientid",
> 
> and this should be "ceph.client_id".  It's easier to read, consistent
> with "ceph fsid" command and with existing rbd attributes:
> 
>   static DEVICE_ATTR(client_id, 0444, rbd_client_id_show, NULL);
>   static DEVICE_ATTR(cluster_fsid, 0444, rbd_cluster_fsid_show, NULL);
> 

That sounds like a good idea. Xiubo, would you mind sending a v4?

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3 2/2] ceph: add ceph.{clusterid/clientid} vxattrs suppport

[Xiubo Li]

On 2020/11/11 0:26, Jeff Layton wrote:
> On Tue, 2020-11-10 at 16:59 +0100, Ilya Dryomov wrote:
>> On Tue, Nov 10, 2020 at 3:17 PM <xiubli@redhat.com> wrote:
>>> From: Xiubo Li <xiubli@redhat.com>
>>>
>>> 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@redhat.com>
>>> ---
>>>   fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 42 insertions(+)
>>>
>>> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
>>> index 0fd05d3d4399..4a41db46e191 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_clusterid(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_clientid(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));
>>> +}
>>> +
>>>   #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_vxattrs[] = {
>>> +       {
>>> +               .name = "ceph.clusterid",
>> I think this should be "ceph.cluster_fsid"
>>
>>> +               .name_size = sizeof("ceph.clusterid"),
>>> +               .getxattr_cb = ceph_vxattrcb_clusterid,
>>> +               .exists_cb = NULL,
>>> +               .flags = VXATTR_FLAG_READONLY,
>>> +       },
>>> +       {
>>> +               .name = "ceph.clientid",
>> and this should be "ceph.client_id".  It's easier to read, consistent
>> with "ceph fsid" command and with existing rbd attributes:
>>
>>    static DEVICE_ATTR(client_id, 0444, rbd_client_id_show, NULL);
>>    static DEVICE_ATTR(cluster_fsid, 0444, rbd_cluster_fsid_show, NULL);
>>
> That sounds like a good idea. Xiubo, would you mind sending a v4?

Sure, I will post the V4 for fix them all.

Thanks.

BRs

Xiubo


> Thanks,

Re: [PATCH v3 2/2] ceph: add ceph.{clusterid/clientid} vxattrs suppport

[Xiubo Li]

On 2020/11/10 23:59, Ilya Dryomov wrote:
> On Tue, Nov 10, 2020 at 3:17 PM <xiubli@redhat.com> wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> 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@redhat.com>
>> ---
>>   fs/ceph/xattr.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>
>> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
>> index 0fd05d3d4399..4a41db46e191 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_clusterid(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_clientid(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));
>> +}
>> +
>>   #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_vxattrs[] = {
>> +       {
>> +               .name = "ceph.clusterid",
> I think this should be "ceph.cluster_fsid"
>
>> +               .name_size = sizeof("ceph.clusterid"),
>> +               .getxattr_cb = ceph_vxattrcb_clusterid,
>> +               .exists_cb = NULL,
>> +               .flags = VXATTR_FLAG_READONLY,
>> +       },
>> +       {
>> +               .name = "ceph.clientid",
> and this should be "ceph.client_id".  It's easier to read, consistent
> with "ceph fsid" command and with existing rbd attributes:
>
>    static DEVICE_ATTR(client_id, 0444, rbd_client_id_show, NULL);
>    static DEVICE_ATTR(cluster_fsid, 0444, rbd_cluster_fsid_show, NULL);

Yeah, sounds nice.

Will fix them all.

Thanks.


> Thanks,
>
>                  Ilya
>