[RFC PATCH] ceph: add ceph.caps vxattr

[RFC PATCH] ceph: add ceph.caps vxattr

[Luis Henriques]

Add a new vxattr that allows userspace to list the caps for a specific
directory or file.

Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
 fs/ceph/xattr.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 197cb1234341..996512e05513 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -303,6 +303,18 @@ 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_caps(struct ceph_inode_info *ci, char *val,
+					size_t size)
+{
+	int issued;
+
+	spin_lock(&ci->i_ceph_lock);
+	issued = __ceph_caps_issued(ci, NULL);
+	spin_unlock(&ci->i_ceph_lock);
+
+	return ceph_fmt_xattr(val, size, "%s", ceph_cap_string(issued));
+}
+
 #define CEPH_XATTR_NAME(_type, _name)	XATTR_CEPH_PREFIX #_type "." #_name
 #define CEPH_XATTR_NAME2(_type, _name, _name2)	\
 	XATTR_CEPH_PREFIX #_type "." #_name "." #_name2
@@ -378,6 +390,13 @@ static struct ceph_vxattr ceph_dir_vxattrs[] = {
 		.exists_cb = ceph_vxattrcb_snap_btime_exists,
 		.flags = VXATTR_FLAG_READONLY,
 	},
+	{
+		.name = "ceph.caps",
+		.name_size = sizeof("ceph.caps"),
+		.getxattr_cb = ceph_vxattrcb_caps,
+		.exists_cb = NULL,
+		.flags = VXATTR_FLAG_HIDDEN,
+	},
 	{ .name = NULL, 0 }	/* Required table terminator */
 };
 
@@ -403,6 +422,13 @@ static struct ceph_vxattr ceph_file_vxattrs[] = {
 		.exists_cb = ceph_vxattrcb_snap_btime_exists,
 		.flags = VXATTR_FLAG_READONLY,
 	},
+	{
+		.name = "ceph.caps",
+		.name_size = sizeof("ceph.caps"),
+		.getxattr_cb = ceph_vxattrcb_caps,
+		.exists_cb = NULL,
+		.flags = VXATTR_FLAG_HIDDEN,
+	},
 	{ .name = NULL, 0 }	/* Required table terminator */
 };
 

Re: [RFC PATCH] ceph: add ceph.caps vxattr

[Jeff Layton]

On Mon, 2020-11-23 at 14:53 +0000, Luis Henriques wrote:
> Add a new vxattr that allows userspace to list the caps for a specific
> directory or file.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>
> ---
>  fs/ceph/xattr.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 197cb1234341..996512e05513 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -303,6 +303,18 @@ 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_caps(struct ceph_inode_info *ci, char *val,
> +					size_t size)
> +{
> +	int issued;
> +
> +	spin_lock(&ci->i_ceph_lock);
> +	issued = __ceph_caps_issued(ci, NULL);
> +	spin_unlock(&ci->i_ceph_lock);
> +
> +	return ceph_fmt_xattr(val, size, "%s", ceph_cap_string(issued));
> +}
> +
>  #define CEPH_XATTR_NAME(_type, _name)	XATTR_CEPH_PREFIX #_type "." #_name
>  #define CEPH_XATTR_NAME2(_type, _name, _name2)	\
>  	XATTR_CEPH_PREFIX #_type "." #_name "." #_name2
> @@ -378,6 +390,13 @@ static struct ceph_vxattr ceph_dir_vxattrs[] = {
>  		.exists_cb = ceph_vxattrcb_snap_btime_exists,
>  		.flags = VXATTR_FLAG_READONLY,
>  	},
> +	{
> +		.name = "ceph.caps",
> +		.name_size = sizeof("ceph.caps"),
> +		.getxattr_cb = ceph_vxattrcb_caps,
> +		.exists_cb = NULL,
> +		.flags = VXATTR_FLAG_HIDDEN,
> +	},
>  	{ .name = NULL, 0 }	/* Required table terminator */
>  };
>  
> 
> @@ -403,6 +422,13 @@ static struct ceph_vxattr ceph_file_vxattrs[] = {
>  		.exists_cb = ceph_vxattrcb_snap_btime_exists,
>  		.flags = VXATTR_FLAG_READONLY,
>  	},
> +	{
> +		.name = "ceph.caps",
> +		.name_size = sizeof("ceph.caps"),
> +		.getxattr_cb = ceph_vxattrcb_caps,
> +		.exists_cb = NULL,
> +		.flags = VXATTR_FLAG_HIDDEN,
> +	},
>  	{ .name = NULL, 0 }	/* Required table terminator */
>  };
>  
> 

Looks useful! I'll plan to merge this unless anyone has objections.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH] ceph: add ceph.caps vxattr

[Jeff Layton]

On Mon, 2020-11-23 at 14:53 +0000, Luis Henriques wrote:
> Add a new vxattr that allows userspace to list the caps for a specific
> directory or file.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>
> ---
>  fs/ceph/xattr.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
> index 197cb1234341..996512e05513 100644
> --- a/fs/ceph/xattr.c
> +++ b/fs/ceph/xattr.c
> @@ -303,6 +303,18 @@ 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_caps(struct ceph_inode_info *ci, char *val,
> +					size_t size)
> +{
> +	int issued;
> +
> +	spin_lock(&ci->i_ceph_lock);
> +	issued = __ceph_caps_issued(ci, NULL);
> +	spin_unlock(&ci->i_ceph_lock);
> +
> +	return ceph_fmt_xattr(val, size, "%s", ceph_cap_string(issued));
> +}

I wonder if we ought to print a numerical (hex) representation of the
cap set here in addition to the ceph_cap_string. A main use-case for
this will be in testcases, and determining whether (e.g.) Fs is in the
mask may be easier to do with a bitwise AND rather than having to parse
a string of text.

> +
>  #define CEPH_XATTR_NAME(_type, _name)	XATTR_CEPH_PREFIX #_type "." #_name
>  #define CEPH_XATTR_NAME2(_type, _name, _name2)	\
>  	XATTR_CEPH_PREFIX #_type "." #_name "." #_name2
> @@ -378,6 +390,13 @@ static struct ceph_vxattr ceph_dir_vxattrs[] = {
>  		.exists_cb = ceph_vxattrcb_snap_btime_exists,
>  		.flags = VXATTR_FLAG_READONLY,
>  	},
> +	{
> +		.name = "ceph.caps",
> +		.name_size = sizeof("ceph.caps"),
> +		.getxattr_cb = ceph_vxattrcb_caps,
> +		.exists_cb = NULL,
> +		.flags = VXATTR_FLAG_HIDDEN,
> +	},
>  	{ .name = NULL, 0 }	/* Required table terminator */
>  };
>  
> 
> 
> 
> @@ -403,6 +422,13 @@ static struct ceph_vxattr ceph_file_vxattrs[] = {
>  		.exists_cb = ceph_vxattrcb_snap_btime_exists,
>  		.flags = VXATTR_FLAG_READONLY,
>  	},
> +	{
> +		.name = "ceph.caps",
> +		.name_size = sizeof("ceph.caps"),
> +		.getxattr_cb = ceph_vxattrcb_caps,
> +		.exists_cb = NULL,
> +		.flags = VXATTR_FLAG_HIDDEN,
> +	},
>  	{ .name = NULL, 0 }	/* Required table terminator */
>  };
>  
> 
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

[PATCH v2] ceph: add ceph.caps vxattr

[Luis Henriques]

Add a new vxattr that allows userspace to list the caps for a specific
directory or file.

Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
Hi!

Here's a version that also shows the caps in hexadecimal format, as
suggested by Jeff.  IMO the parenthesis and the '0x' prefix help the
readability, but they may make it a bit harder for scripts to parsing the
output.  I'm OK dropping those.

Cheers,
-- 
Luis

fs/ceph/xattr.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 197cb1234341..aec9bd5c8e77 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -303,6 +303,19 @@ 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_caps(struct ceph_inode_info *ci, char *val,
+					size_t size)
+{
+	int issued;
+
+	spin_lock(&ci->i_ceph_lock);
+	issued = __ceph_caps_issued(ci, NULL);
+	spin_unlock(&ci->i_ceph_lock);
+
+	return ceph_fmt_xattr(val, size, "%s (0x%x)",
+			      ceph_cap_string(issued), issued);
+}
+
 #define CEPH_XATTR_NAME(_type, _name)	XATTR_CEPH_PREFIX #_type "." #_name
 #define CEPH_XATTR_NAME2(_type, _name, _name2)	\
 	XATTR_CEPH_PREFIX #_type "." #_name "." #_name2
@@ -378,6 +391,13 @@ static struct ceph_vxattr ceph_dir_vxattrs[] = {
 		.exists_cb = ceph_vxattrcb_snap_btime_exists,
 		.flags = VXATTR_FLAG_READONLY,
 	},
+	{
+		.name = "ceph.caps",
+		.name_size = sizeof("ceph.caps"),
+		.getxattr_cb = ceph_vxattrcb_caps,
+		.exists_cb = NULL,
+		.flags = VXATTR_FLAG_HIDDEN,
+	},
 	{ .name = NULL, 0 }	/* Required table terminator */
 };
 
@@ -403,6 +423,13 @@ static struct ceph_vxattr ceph_file_vxattrs[] = {
 		.exists_cb = ceph_vxattrcb_snap_btime_exists,
 		.flags = VXATTR_FLAG_READONLY,
 	},
+	{
+		.name = "ceph.caps",
+		.name_size = sizeof("ceph.caps"),
+		.getxattr_cb = ceph_vxattrcb_caps,
+		.exists_cb = NULL,
+		.flags = VXATTR_FLAG_HIDDEN,
+	},
 	{ .name = NULL, 0 }	/* Required table terminator */
 };
 

Re: [PATCH v2] ceph: add ceph.caps vxattr

[Jeff Layton]

On Mon, 2020-11-23 at 17:38 +0000, Luis Henriques wrote:
> Add a new vxattr that allows userspace to list the caps for a specific
> directory or file.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>
> ---
> Hi!
> 
> Here's a version that also shows the caps in hexadecimal format, as
> suggested by Jeff.  IMO the parenthesis and the '0x' prefix help the
> readability, but they may make it a bit harder for scripts to parsing the
> output.  I'm OK dropping those.
> 
> Cheers,

Looks good, merged into "testing".

I did make a slight change to the format -- instead of putting the hex
value in parenthesis, I separated the two fields with a /, which I think
should make things easier for scripts to parse.

You should be able to do something like this to get at the hex value for
testing:

    $ getfattr -n ceph.caps foo | cut -d / -f2

Let me know if you see issues with that and we can revisit the format.

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

Re: [PATCH v2] ceph: add ceph.caps vxattr

[Luis Henriques]

Jeff Layton <jlayton@kernel.org> writes:

> On Mon, 2020-11-23 at 17:38 +0000, Luis Henriques wrote:
>> Add a new vxattr that allows userspace to list the caps for a specific
>> directory or file.
>> 
>> Signed-off-by: Luis Henriques <lhenriques@suse.de>
>> ---
>> Hi!
>> 
>> Here's a version that also shows the caps in hexadecimal format, as
>> suggested by Jeff.  IMO the parenthesis and the '0x' prefix help the
>> readability, but they may make it a bit harder for scripts to parsing the
>> output.  I'm OK dropping those.
>> 
>> Cheers,
>
> Looks good, merged into "testing".

Awesome, thanks!

> I did make a slight change to the format -- instead of putting the hex
> value in parenthesis, I separated the two fields with a /, which I think
> should make things easier for scripts to parse.
>
> You should be able to do something like this to get at the hex value for
> testing:
>
>     $ getfattr -n ceph.caps foo | cut -d / -f2
>
> Let me know if you see issues with that and we can revisit the format.

Sure, I'm OK with that.  Or even simply dropping any separator, having
only a space/tab between the string and the hex value.

Another option I saw was to have two vxattrs: ceph.caps.string and
ceph.caps.int.  But that's probably overkill.

Cheers,
-- 
Luis