linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] hoist btrfs' label set/get ioctls to vfs, and document
@ 2018-05-09 15:56 Eric Sandeen
  2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
  2018-05-09 16:04 ` [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls Eric Sandeen
  0 siblings, 2 replies; 22+ messages in thread
From: Eric Sandeen @ 2018-05-09 15:56 UTC (permalink / raw)
  To: fsdevel, linux-btrfs, Linux API

I'm planning to add online label set/get support to xfs, and to do so
I plan to re-use the existing btrfs ioctls, BTRFS_IOC_[SG]ET_FSLABEL

We're still working out minor details on the xfs side, but I'd like to
start the conversation regarding the new more generic interface ASAP,
so here goes - patches to move the ioctls to the vfs and document them.

(Other filesystems may wish to use this interface in the future as well)

Thanks,
-Eric

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 15:56 [PATCH 0/2] hoist btrfs' label set/get ioctls to vfs, and document Eric Sandeen
@ 2018-05-09 16:01 ` Eric Sandeen
  2018-05-09 16:10   ` Darrick J. Wong
                     ` (3 more replies)
  2018-05-09 16:04 ` [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls Eric Sandeen
  1 sibling, 4 replies; 22+ messages in thread
From: Eric Sandeen @ 2018-05-09 16:01 UTC (permalink / raw)
  To: Eric Sandeen, fsdevel, linux-btrfs, Linux API

Move the btrfs label ioctls up to the vfs for general use.

This retains 256 chars as the maximum size through the interface, which
is the btrfs limit and AFAIK exceeds any other filesystem's maximum
label size.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Let the bikeshedding on the exact ioctl name begin ;)

 fs/btrfs/ioctl.c           | 8 ++++----
 include/uapi/linux/btrfs.h | 6 ++----
 include/uapi/linux/fs.h    | 8 ++++++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 632e26d..2dd4cdf 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_setflags(file, argp);
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
+	case FS_IOC_GETFSLABEL:
+		return btrfs_ioctl_get_fslabel(file, argp);
+	case FS_IOC_SETFSLABEL:
+		return btrfs_ioctl_set_fslabel(file, argp);
 	case FITRIM:
 		return btrfs_ioctl_fitrim(file, argp);
 	case BTRFS_IOC_SNAP_CREATE:
@@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_quota_rescan_wait(file, argp);
 	case BTRFS_IOC_DEV_REPLACE:
 		return btrfs_ioctl_dev_replace(fs_info, argp);
-	case BTRFS_IOC_GET_FSLABEL:
-		return btrfs_ioctl_get_fslabel(file, argp);
-	case BTRFS_IOC_SET_FSLABEL:
-		return btrfs_ioctl_set_fslabel(file, argp);
 	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
 		return btrfs_ioctl_get_supported_features(argp);
 	case BTRFS_IOC_GET_FEATURES:
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index c8d99b9..ec611c8 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -823,10 +823,8 @@ enum btrfs_err_code {
 #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
 			       struct btrfs_ioctl_quota_rescan_args)
 #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
-#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
-				   char[BTRFS_LABEL_SIZE])
-#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
-				   char[BTRFS_LABEL_SIZE])
+#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
+#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
 #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index d2a8313..1df3707 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -242,6 +242,8 @@ struct fsxattr {
 #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
 #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
 
+#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
+
 #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
 #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
 #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
@@ -251,8 +253,10 @@ struct fsxattr {
 #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
 #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
 #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
-#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
-#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
+#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
+#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
+#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
+#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
 
 /*
  * File system encryption support
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls
  2018-05-09 15:56 [PATCH 0/2] hoist btrfs' label set/get ioctls to vfs, and document Eric Sandeen
  2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
@ 2018-05-09 16:04 ` Eric Sandeen
  2018-05-09 16:15   ` Darrick J. Wong
  2018-05-10 17:29   ` [PATCH 2/2 V2] " Eric Sandeen
  1 sibling, 2 replies; 22+ messages in thread
From: Eric Sandeen @ 2018-05-09 16:04 UTC (permalink / raw)
  To: Eric Sandeen, fsdevel, linux-btrfs, Linux API,
	Michael Kerrisk (man-pages)

This documents the proposed new vfs-level ioctls which can
get or set a mounted filesytem's label.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

btrfs folks, please verify that this accurately describes your
current behavior, thanks.

diff --git a/man2/ioctl_fslabel.2 b/man2/ioctl_fslabel.2
new file mode 100644
index 0000000..150aa53
--- /dev/null
+++ b/man2/ioctl_fslabel.2
@@ -0,0 +1,83 @@
+.\" Copyright (c) 2018, Red Hat, Inc.  All rights reserved.
+.\"
+.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
+.\" This is free documentation; you can redistribute it and/or
+.\" modify it under the terms of the GNU General Public License as
+.\" published by the Free Software Foundation; either version 2 of
+.\" the License, or (at your option) any later version.
+.\"
+.\" The GNU General Public License's references to "object code"
+.\" and "executables" are to be interpreted as the output of any
+.\" document formatting or typesetting system, including
+.\" intermediate and printed output.
+.\"
+.\" This manual is distributed in the hope that it will be useful,
+.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
+.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+.\" GNU General Public License for more details.
+.\"
+.\" You should have received a copy of the GNU General Public
+.\" License along with this manual; if not, see
+.\" <http://www.gnu.org/licenses/>.
+.\" %%%LICENSE_END
+.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual"
+.SH NAME
+ioctl_fslabel \- get or set a filesystem label
+.SH SYNOPSIS
+.br
+.B #include <sys/ioctl.h>
+.br
+.B #include <linux/fs.h>
+.sp
+.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]);
+.br
+.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]);
+.SH DESCRIPTION
+If a filesystem supports online label manipulation, these
+.BR ioctl (2)
+operations can be used to get or set the filesystem label for the filesystem
+on which
+.B fd
+resides.
+.SH RETURN VALUE
+On success zero is returned.  On error, \-1 is returned, and
+.I errno
+is set to indicate the error.
+.PP
+.SH ERRORS
+Error codes can be one of, but are not limited to, the following:
+.TP
+.B EINVAL
+The specified label exceeds the maximum label length for the filesystem.
+.TP
+.B ENOTTY
+This can appear if the filesystem does not support online label manipulation.
+.TP
+.B EPERM
+The calling process does not have sufficient permissions to set the label.
+.TP
+.B EFAULT
+.I label
+references an inaccessible memory area.
+.SH VERSIONS
+These ioctl operations first appeared in Linux 4.18.
+They were previously known as
+.B BTRFS_IOC_GET_FSLABEL
+and
+.B BTRFS_IOC_SET_FSLABEL
+and were private to Btrfs.
+.SH CONFORMING TO
+This API is Linux-specific.
+.SH NOTES
+The maximum string length for this interface is
+.BR FSLABEL_MAX ,
+including the terminating null byte (\(aq\\0\(aq).
+Filesystems have differing maximum label lengths, which may or
+may not include the terminating null.  The string provided to
+.B FS_IOC_SETFSLABEL
+must always be null-terminated, and the string returned by
+.B FS_IOC_GETFSLABEL
+will always be null-terminated.
+.SH SEE ALSO
+.BR ioctl (2),
+.BR blkid (8)
diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2
new file mode 100644
index 0000000..bfa8dca
--- /dev/null
+++ b/man2/ioctl_getfslabel.2
@@ -0,0 +1 @@
+.so man2/ioctl_fslabel.2
diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2
new file mode 100644
index 0000000..bfa8dca
--- /dev/null
+++ b/man2/ioctl_setfslabel.2
@@ -0,0 +1 @@
+.so man2/ioctl_fslabel.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
@ 2018-05-09 16:10   ` Darrick J. Wong
  2018-05-09 17:15     ` Andreas Dilger
  2018-05-09 17:35   ` Randy Dunlap
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 22+ messages in thread
From: Darrick J. Wong @ 2018-05-09 16:10 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, fsdevel, linux-btrfs, Linux API

On Wed, May 09, 2018 at 11:01:21AM -0500, Eric Sandeen wrote:
> Move the btrfs label ioctls up to the vfs for general use.
> 
> This retains 256 chars as the maximum size through the interface, which
> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> label size.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Let the bikeshedding on the exact ioctl name begin ;)
> 
>  fs/btrfs/ioctl.c           | 8 ++++----
>  include/uapi/linux/btrfs.h | 6 ++----
>  include/uapi/linux/fs.h    | 8 ++++++--
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 632e26d..2dd4cdf 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_setflags(file, argp);
>  	case FS_IOC_GETVERSION:
>  		return btrfs_ioctl_getversion(file, argp);
> +	case FS_IOC_GETFSLABEL:
> +		return btrfs_ioctl_get_fslabel(file, argp);
> +	case FS_IOC_SETFSLABEL:
> +		return btrfs_ioctl_set_fslabel(file, argp);
>  	case FITRIM:
>  		return btrfs_ioctl_fitrim(file, argp);
>  	case BTRFS_IOC_SNAP_CREATE:
> @@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_quota_rescan_wait(file, argp);
>  	case BTRFS_IOC_DEV_REPLACE:
>  		return btrfs_ioctl_dev_replace(fs_info, argp);
> -	case BTRFS_IOC_GET_FSLABEL:
> -		return btrfs_ioctl_get_fslabel(file, argp);
> -	case BTRFS_IOC_SET_FSLABEL:
> -		return btrfs_ioctl_set_fslabel(file, argp);
>  	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
>  		return btrfs_ioctl_get_supported_features(argp);
>  	case BTRFS_IOC_GET_FEATURES:
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index c8d99b9..ec611c8 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -823,10 +823,8 @@ enum btrfs_err_code {
>  #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
>  			       struct btrfs_ioctl_quota_rescan_args)
>  #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
> -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
> -				   char[BTRFS_LABEL_SIZE])
> -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
> -				   char[BTRFS_LABEL_SIZE])
> +#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
> +#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
>  #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
>  				      struct btrfs_ioctl_get_dev_stats)
>  #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index d2a8313..1df3707 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -242,6 +242,8 @@ struct fsxattr {
>  #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
>  #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
>  
> +#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
> +
>  #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
>  #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
>  #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
> @@ -251,8 +253,10 @@ struct fsxattr {
>  #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
>  #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
>  #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
> -#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
> -#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
> +#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
> +#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)

Separate patch for whitespace cleanup.

> +#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
> +#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])

Looks ok otherwise,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  
>  /*
>   * File system encryption support
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls
  2018-05-09 16:04 ` [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls Eric Sandeen
@ 2018-05-09 16:15   ` Darrick J. Wong
  2018-05-10 17:29   ` [PATCH 2/2 V2] " Eric Sandeen
  1 sibling, 0 replies; 22+ messages in thread
From: Darrick J. Wong @ 2018-05-09 16:15 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Eric Sandeen, fsdevel, linux-btrfs, Linux API,
	Michael Kerrisk (man-pages)

On Wed, May 09, 2018 at 11:04:03AM -0500, Eric Sandeen wrote:
> This documents the proposed new vfs-level ioctls which can
> get or set a mounted filesytem's label.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> btrfs folks, please verify that this accurately describes your
> current behavior, thanks.
> 
> diff --git a/man2/ioctl_fslabel.2 b/man2/ioctl_fslabel.2
> new file mode 100644
> index 0000000..150aa53
> --- /dev/null
> +++ b/man2/ioctl_fslabel.2
> @@ -0,0 +1,83 @@
> +.\" Copyright (c) 2018, Red Hat, Inc.  All rights reserved.
> +.\"
> +.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
> +.\" This is free documentation; you can redistribute it and/or
> +.\" modify it under the terms of the GNU General Public License as
> +.\" published by the Free Software Foundation; either version 2 of
> +.\" the License, or (at your option) any later version.
> +.\"
> +.\" The GNU General Public License's references to "object code"
> +.\" and "executables" are to be interpreted as the output of any
> +.\" document formatting or typesetting system, including
> +.\" intermediate and printed output.
> +.\"
> +.\" This manual is distributed in the hope that it will be useful,
> +.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
> +.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +.\" GNU General Public License for more details.
> +.\"
> +.\" You should have received a copy of the GNU General Public
> +.\" License along with this manual; if not, see
> +.\" <http://www.gnu.org/licenses/>.
> +.\" %%%LICENSE_END
> +.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual"
> +.SH NAME
> +ioctl_fslabel \- get or set a filesystem label
> +.SH SYNOPSIS
> +.br
> +.B #include <sys/ioctl.h>
> +.br
> +.B #include <linux/fs.h>
> +.sp
> +.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]);
> +.br
> +.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]);
> +.SH DESCRIPTION
> +If a filesystem supports online label manipulation, these
> +.BR ioctl (2)
> +operations can be used to get or set the filesystem label for the filesystem
> +on which
> +.B fd
> +resides.

Does the calling process need special capabilities or permissions?  If
so, those should be listed here.

> +.SH RETURN VALUE
> +On success zero is returned.  On error, \-1 is returned, and
> +.I errno
> +is set to indicate the error.
> +.PP
> +.SH ERRORS
> +Error codes can be one of, but are not limited to, the following:
> +.TP
> +.B EINVAL
> +The specified label exceeds the maximum label length for the filesystem.
> +.TP
> +.B ENOTTY
> +This can appear if the filesystem does not support online label manipulation.
> +.TP
> +.B EPERM
> +The calling process does not have sufficient permissions to set the label.
> +.TP
> +.B EFAULT
> +.I label
> +references an inaccessible memory area.
> +.SH VERSIONS
> +These ioctl operations first appeared in Linux 4.18.
> +They were previously known as
> +.B BTRFS_IOC_GET_FSLABEL
> +and
> +.B BTRFS_IOC_SET_FSLABEL
> +and were private to Btrfs.
> +.SH CONFORMING TO
> +This API is Linux-specific.
> +.SH NOTES
> +The maximum string length for this interface is
> +.BR FSLABEL_MAX ,
> +including the terminating null byte (\(aq\\0\(aq).
> +Filesystems have differing maximum label lengths, which may or
> +may not include the terminating null.  The string provided to
> +.B FS_IOC_SETFSLABEL
> +must always be null-terminated, and the string returned by
> +.B FS_IOC_GETFSLABEL
> +will always be null-terminated.
> +.SH SEE ALSO
> +.BR ioctl (2),
> +.BR blkid (8)
> diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2
> new file mode 100644
> index 0000000..bfa8dca
> --- /dev/null
> +++ b/man2/ioctl_getfslabel.2
> @@ -0,0 +1 @@
> +.so man2/ioctl_fslabel.2
> diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2
> new file mode 100644
> index 0000000..bfa8dca
> --- /dev/null
> +++ b/man2/ioctl_setfslabel.2
> @@ -0,0 +1 @@
> +.so man2/ioctl_fslabel.2

Put all the manpage content into ioctl_getfslabel.2 and have
ioctl_setfslabel.2 point to it, rather than three files.

--D

> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 16:10   ` Darrick J. Wong
@ 2018-05-09 17:15     ` Andreas Dilger
  2018-05-09 17:26       ` Darrick J. Wong
  0 siblings, 1 reply; 22+ messages in thread
From: Andreas Dilger @ 2018-05-09 17:15 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Eric Sandeen, Eric Sandeen, fsdevel, linux-btrfs, Linux API

[-- Attachment #1: Type: text/plain, Size: 4954 bytes --]

On May 9, 2018, at 10:10 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> On Wed, May 09, 2018 at 11:01:21AM -0500, Eric Sandeen wrote:
>> Move the btrfs label ioctls up to the vfs for general use.
>> 
>> This retains 256 chars as the maximum size through the interface, which
>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
>> label size.
>> 
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>> 
>> Let the bikeshedding on the exact ioctl name begin ;)
>> 
>> fs/btrfs/ioctl.c           | 8 ++++----
>> include/uapi/linux/btrfs.h | 6 ++----
>> include/uapi/linux/fs.h    | 8 ++++++--
>> 3 files changed, 12 insertions(+), 10 deletions(-)
>> 
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 632e26d..2dd4cdf 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>> 		return btrfs_ioctl_setflags(file, argp);
>> 	case FS_IOC_GETVERSION:
>> 		return btrfs_ioctl_getversion(file, argp);
>> +	case FS_IOC_GETFSLABEL:
>> +		return btrfs_ioctl_get_fslabel(file, argp);
>> +	case FS_IOC_SETFSLABEL:
>> +		return btrfs_ioctl_set_fslabel(file, argp);
>> 	case FITRIM:
>> 		return btrfs_ioctl_fitrim(file, argp);
>> 	case BTRFS_IOC_SNAP_CREATE:
>> @@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
>> 		return btrfs_ioctl_quota_rescan_wait(file, argp);
>> 	case BTRFS_IOC_DEV_REPLACE:
>> 		return btrfs_ioctl_dev_replace(fs_info, argp);
>> -	case BTRFS_IOC_GET_FSLABEL:
>> -		return btrfs_ioctl_get_fslabel(file, argp);
>> -	case BTRFS_IOC_SET_FSLABEL:
>> -		return btrfs_ioctl_set_fslabel(file, argp);
>> 	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
>> 		return btrfs_ioctl_get_supported_features(argp);
>> 	case BTRFS_IOC_GET_FEATURES:
>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>> index c8d99b9..ec611c8 100644
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -823,10 +823,8 @@ enum btrfs_err_code {
>> #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
>> 			       struct btrfs_ioctl_quota_rescan_args)
>> #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
>> -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
>> -				   char[BTRFS_LABEL_SIZE])
>> -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
>> -				   char[BTRFS_LABEL_SIZE])
>> +#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
>> +#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
>> #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
>> 				      struct btrfs_ioctl_get_dev_stats)
>> #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
>> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> index d2a8313..1df3707 100644
>> --- a/include/uapi/linux/fs.h
>> +++ b/include/uapi/linux/fs.h
>> @@ -242,6 +242,8 @@ struct fsxattr {
>> #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
>> #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
>> 
>> +#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
>> +
>> #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
>> #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
>> #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
>> @@ -251,8 +253,10 @@ struct fsxattr {
>> #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
>> #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
>> #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
>> -#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
>> -#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
>> +#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
>> +#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
> 
> Separate patch for whitespace cleanup.

Really?  I've heard Ted complain the other way, that whitespace cleanup
by itself is useless and should only be done as part of other changes.
As long as it is not overwhelming the rest of the patch I don't see an
issue with a minor improvement being part of another patch.

Otherwise, no bikeshedding from me.  Looks very reasonable, and the 256-char
limit is definitely large enough IMHO (also matches the normal filename size
limit, so if the label is used as a pathname component there are no added
restrictions).

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

Cheers, Andreas

>> +#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
>> +#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
> 
> Looks ok otherwise,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> --D
> 
>> 
>> /*
>>  * File system encryption support
>> --
>> 1.8.3.1
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-api" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 17:15     ` Andreas Dilger
@ 2018-05-09 17:26       ` Darrick J. Wong
  0 siblings, 0 replies; 22+ messages in thread
From: Darrick J. Wong @ 2018-05-09 17:26 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: Eric Sandeen, Eric Sandeen, fsdevel, linux-btrfs, Linux API

On Wed, May 09, 2018 at 11:15:46AM -0600, Andreas Dilger wrote:
> On May 9, 2018, at 10:10 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > 
> > On Wed, May 09, 2018 at 11:01:21AM -0500, Eric Sandeen wrote:
> >> Move the btrfs label ioctls up to the vfs for general use.
> >> 
> >> This retains 256 chars as the maximum size through the interface, which
> >> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> >> label size.
> >> 
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >> 
> >> Let the bikeshedding on the exact ioctl name begin ;)
> >> 
> >> fs/btrfs/ioctl.c           | 8 ++++----
> >> include/uapi/linux/btrfs.h | 6 ++----
> >> include/uapi/linux/fs.h    | 8 ++++++--
> >> 3 files changed, 12 insertions(+), 10 deletions(-)
> >> 
> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> >> index 632e26d..2dd4cdf 100644
> >> --- a/fs/btrfs/ioctl.c
> >> +++ b/fs/btrfs/ioctl.c
> >> @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
> >> 		return btrfs_ioctl_setflags(file, argp);
> >> 	case FS_IOC_GETVERSION:
> >> 		return btrfs_ioctl_getversion(file, argp);
> >> +	case FS_IOC_GETFSLABEL:
> >> +		return btrfs_ioctl_get_fslabel(file, argp);
> >> +	case FS_IOC_SETFSLABEL:
> >> +		return btrfs_ioctl_set_fslabel(file, argp);
> >> 	case FITRIM:
> >> 		return btrfs_ioctl_fitrim(file, argp);
> >> 	case BTRFS_IOC_SNAP_CREATE:
> >> @@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
> >> 		return btrfs_ioctl_quota_rescan_wait(file, argp);
> >> 	case BTRFS_IOC_DEV_REPLACE:
> >> 		return btrfs_ioctl_dev_replace(fs_info, argp);
> >> -	case BTRFS_IOC_GET_FSLABEL:
> >> -		return btrfs_ioctl_get_fslabel(file, argp);
> >> -	case BTRFS_IOC_SET_FSLABEL:
> >> -		return btrfs_ioctl_set_fslabel(file, argp);
> >> 	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
> >> 		return btrfs_ioctl_get_supported_features(argp);
> >> 	case BTRFS_IOC_GET_FEATURES:
> >> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> >> index c8d99b9..ec611c8 100644
> >> --- a/include/uapi/linux/btrfs.h
> >> +++ b/include/uapi/linux/btrfs.h
> >> @@ -823,10 +823,8 @@ enum btrfs_err_code {
> >> #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
> >> 			       struct btrfs_ioctl_quota_rescan_args)
> >> #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
> >> -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
> >> -				   char[BTRFS_LABEL_SIZE])
> >> -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
> >> -				   char[BTRFS_LABEL_SIZE])
> >> +#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
> >> +#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
> >> #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
> >> 				      struct btrfs_ioctl_get_dev_stats)
> >> #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
> >> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> >> index d2a8313..1df3707 100644
> >> --- a/include/uapi/linux/fs.h
> >> +++ b/include/uapi/linux/fs.h
> >> @@ -242,6 +242,8 @@ struct fsxattr {
> >> #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
> >> #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
> >> 
> >> +#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
> >> +
> >> #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
> >> #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
> >> #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
> >> @@ -251,8 +253,10 @@ struct fsxattr {
> >> #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
> >> #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
> >> #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
> >> -#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
> >> -#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
> >> +#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
> >> +#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
> > 
> > Separate patch for whitespace cleanup.
> 
> Really?  I've heard Ted complain the other way, that whitespace cleanup
> by itself is useless and should only be done as part of other changes.
> As long as it is not overwhelming the rest of the patch I don't see an
> issue with a minor improvement being part of another patch.

I really only meant this as: put the whitespace changes in a second
patch after this one so that we don't have a patch that implements two
different changes, but fmeh, tired of discussing this.

--D

> Otherwise, no bikeshedding from me.  Looks very reasonable, and the 256-char
> limit is definitely large enough IMHO (also matches the normal filename size
> limit, so if the label is used as a pathname component there are no added
> restrictions).
> 
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> 
> Cheers, Andreas
> 
> >> +#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
> >> +#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
> > 
> > Looks ok otherwise,
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > --D
> > 
> >> 
> >> /*
> >>  * File system encryption support
> >> --
> >> 1.8.3.1
> >> 
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
  2018-05-09 16:10   ` Darrick J. Wong
@ 2018-05-09 17:35   ` Randy Dunlap
  2018-05-09 17:40     ` Eric Sandeen
  2018-05-09 21:35   ` David Sterba
  2018-05-10 18:13   ` [PATCH 1/2 V2] " Eric Sandeen
  3 siblings, 1 reply; 22+ messages in thread
From: Randy Dunlap @ 2018-05-09 17:35 UTC (permalink / raw)
  To: Eric Sandeen, Eric Sandeen, fsdevel, linux-btrfs, Linux API

On 05/09/2018 09:01 AM, Eric Sandeen wrote:
> Move the btrfs label ioctls up to the vfs for general use.
> 
> This retains 256 chars as the maximum size through the interface, which
> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> label size.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Let the bikeshedding on the exact ioctl name begin ;)
> 
>  fs/btrfs/ioctl.c           | 8 ++++----
>  include/uapi/linux/btrfs.h | 6 ++----
>  include/uapi/linux/fs.h    | 8 ++++++--
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 632e26d..2dd4cdf 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_setflags(file, argp);
>  	case FS_IOC_GETVERSION:
>  		return btrfs_ioctl_getversion(file, argp);
> +	case FS_IOC_GETFSLABEL:
> +		return btrfs_ioctl_get_fslabel(file, argp);
> +	case FS_IOC_SETFSLABEL:
> +		return btrfs_ioctl_set_fslabel(file, argp);
>  	case FITRIM:
>  		return btrfs_ioctl_fitrim(file, argp);
>  	case BTRFS_IOC_SNAP_CREATE:
> @@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_quota_rescan_wait(file, argp);
>  	case BTRFS_IOC_DEV_REPLACE:
>  		return btrfs_ioctl_dev_replace(fs_info, argp);
> -	case BTRFS_IOC_GET_FSLABEL:
> -		return btrfs_ioctl_get_fslabel(file, argp);
> -	case BTRFS_IOC_SET_FSLABEL:
> -		return btrfs_ioctl_set_fslabel(file, argp);
>  	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
>  		return btrfs_ioctl_get_supported_features(argp);
>  	case BTRFS_IOC_GET_FEATURES:
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index c8d99b9..ec611c8 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -823,10 +823,8 @@ enum btrfs_err_code {
>  #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
>  			       struct btrfs_ioctl_quota_rescan_args)
>  #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
> -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
> -				   char[BTRFS_LABEL_SIZE])
> -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
> -				   char[BTRFS_LABEL_SIZE])
> +#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
> +#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
>  #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
>  				      struct btrfs_ioctl_get_dev_stats)
>  #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index d2a8313..1df3707 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -242,6 +242,8 @@ struct fsxattr {
>  #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
>  #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
>  
> +#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
> +
>  #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
>  #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
>  #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
> @@ -251,8 +253,10 @@ struct fsxattr {
>  #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
>  #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
>  #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
> -#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
> -#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
> +#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
> +#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
> +#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
> +#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])

Also update Documentation/ioctl/ioctl-number.txt, where it says that 0x94:all
are used for btrfs:

0x94	all	fs/btrfs/ioctl.h

AFAICT 0x94 is now split between vfs and btrfs.  Please correct me if I
misunderstand.


thanks,
-- 
~Randy

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 17:35   ` Randy Dunlap
@ 2018-05-09 17:40     ` Eric Sandeen
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Sandeen @ 2018-05-09 17:40 UTC (permalink / raw)
  To: Randy Dunlap, Eric Sandeen, fsdevel, linux-btrfs, Linux API



On 5/9/18 12:35 PM, Randy Dunlap wrote:
> On 05/09/2018 09:01 AM, Eric Sandeen wrote:
>> Move the btrfs label ioctls up to the vfs for general use.
>>
>> This retains 256 chars as the maximum size through the interface, which
>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
>> label size.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> Let the bikeshedding on the exact ioctl name begin ;)
>>
>>  fs/btrfs/ioctl.c           | 8 ++++----
>>  include/uapi/linux/btrfs.h | 6 ++----
>>  include/uapi/linux/fs.h    | 8 ++++++--
>>  3 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 632e26d..2dd4cdf 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>>  		return btrfs_ioctl_setflags(file, argp);
>>  	case FS_IOC_GETVERSION:
>>  		return btrfs_ioctl_getversion(file, argp);
>> +	case FS_IOC_GETFSLABEL:
>> +		return btrfs_ioctl_get_fslabel(file, argp);
>> +	case FS_IOC_SETFSLABEL:
>> +		return btrfs_ioctl_set_fslabel(file, argp);
>>  	case FITRIM:
>>  		return btrfs_ioctl_fitrim(file, argp);
>>  	case BTRFS_IOC_SNAP_CREATE:
>> @@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
>>  		return btrfs_ioctl_quota_rescan_wait(file, argp);
>>  	case BTRFS_IOC_DEV_REPLACE:
>>  		return btrfs_ioctl_dev_replace(fs_info, argp);
>> -	case BTRFS_IOC_GET_FSLABEL:
>> -		return btrfs_ioctl_get_fslabel(file, argp);
>> -	case BTRFS_IOC_SET_FSLABEL:
>> -		return btrfs_ioctl_set_fslabel(file, argp);
>>  	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
>>  		return btrfs_ioctl_get_supported_features(argp);
>>  	case BTRFS_IOC_GET_FEATURES:
>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>> index c8d99b9..ec611c8 100644
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -823,10 +823,8 @@ enum btrfs_err_code {
>>  #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
>>  			       struct btrfs_ioctl_quota_rescan_args)
>>  #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
>> -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
>> -				   char[BTRFS_LABEL_SIZE])
>> -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
>> -				   char[BTRFS_LABEL_SIZE])
>> +#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
>> +#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
>>  #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
>>  				      struct btrfs_ioctl_get_dev_stats)
>>  #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
>> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> index d2a8313..1df3707 100644
>> --- a/include/uapi/linux/fs.h
>> +++ b/include/uapi/linux/fs.h
>> @@ -242,6 +242,8 @@ struct fsxattr {
>>  #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
>>  #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
>>  
>> +#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
>> +
>>  #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
>>  #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
>>  #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
>> @@ -251,8 +253,10 @@ struct fsxattr {
>>  #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
>>  #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
>>  #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
>> -#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
>> -#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
>> +#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
>> +#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
>> +#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
>> +#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
> 
> Also update Documentation/ioctl/ioctl-number.txt, where it says that 0x94:all
> are used for btrfs:
> 
> 0x94	all	fs/btrfs/ioctl.h
> 
> AFAICT 0x94 is now split between vfs and btrfs.  Please correct me if I
> misunderstand.

It is split, though it has been for a while now, see also:

#define FICLONE         _IOW(0x94, 9, int)
#define FICLONERANGE    _IOW(0x94, 13, struct file_clone_range)
#define FIDEDUPERANGE   _IOWR(0x94, 54, struct file_dedupe_range)

but sure, I can send another patch for that on the next round.

Thanks,
-Eric

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
  2018-05-09 16:10   ` Darrick J. Wong
  2018-05-09 17:35   ` Randy Dunlap
@ 2018-05-09 21:35   ` David Sterba
  2018-05-10 18:13   ` [PATCH 1/2 V2] " Eric Sandeen
  3 siblings, 0 replies; 22+ messages in thread
From: David Sterba @ 2018-05-09 21:35 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, fsdevel, linux-btrfs, Linux API

On Wed, May 09, 2018 at 11:01:21AM -0500, Eric Sandeen wrote:
> Move the btrfs label ioctls up to the vfs for general use.
> 
> This retains 256 chars as the maximum size through the interface, which
> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> label size.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

The btrfs changes and new ioctl naming looks good to me,

Reviewed-by: David Sterba <dsterba@suse.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
  2018-05-09 16:04 ` [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls Eric Sandeen
  2018-05-09 16:15   ` Darrick J. Wong
@ 2018-05-10 17:29   ` Eric Sandeen
  2018-05-10 17:35     ` Eric Sandeen
  2020-04-20 12:04     ` Michael Kerrisk (man-pages)
  1 sibling, 2 replies; 22+ messages in thread
From: Eric Sandeen @ 2018-05-10 17:29 UTC (permalink / raw)
  To: Eric Sandeen, fsdevel, linux-btrfs, Linux API,
	Michael Kerrisk (man-pages)

This documents the proposed new vfs-level ioctls which can
get or set a mounted filesytem's label.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: make primary file ioctl_getfslabel, link ioctl_setfslabel to it
    note that getfslabel requires CAP_SYS_ADMIN

diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2
new file mode 100644
index 0000000..2c3375c
--- /dev/null
+++ b/man2/ioctl_getfslabel.2
@@ -0,0 +1,87 @@
+.\" Copyright (c) 2018, Red Hat, Inc.  All rights reserved.
+.\"
+.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
+.\" This is free documentation; you can redistribute it and/or
+.\" modify it under the terms of the GNU General Public License as
+.\" published by the Free Software Foundation; either version 2 of
+.\" the License, or (at your option) any later version.
+.\"
+.\" The GNU General Public License's references to "object code"
+.\" and "executables" are to be interpreted as the output of any
+.\" document formatting or typesetting system, including
+.\" intermediate and printed output.
+.\"
+.\" This manual is distributed in the hope that it will be useful,
+.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
+.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+.\" GNU General Public License for more details.
+.\"
+.\" You should have received a copy of the GNU General Public
+.\" License along with this manual; if not, see
+.\" <http://www.gnu.org/licenses/>.
+.\" %%%LICENSE_END
+.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual"
+.SH NAME
+ioctl_fslabel \- get or set a filesystem label
+.SH SYNOPSIS
+.br
+.B #include <sys/ioctl.h>
+.br
+.B #include <linux/fs.h>
+.sp
+.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]);
+.br
+.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]);
+.SH DESCRIPTION
+If a filesystem supports online label manipulation, these
+.BR ioctl (2)
+operations can be used to get or set the filesystem label for the filesystem
+on which
+.B fd
+resides.
+The
+.B FS_IOC_SETFSLABEL
+operation requires privilege
+.RB ( CAP_SYS_ADMIN ).
+.SH RETURN VALUE
+On success zero is returned.  On error, \-1 is returned, and
+.I errno
+is set to indicate the error.
+.PP
+.SH ERRORS
+Error codes can be one of, but are not limited to, the following:
+.TP
+.B EINVAL
+The specified label exceeds the maximum label length for the filesystem.
+.TP
+.B ENOTTY
+This can appear if the filesystem does not support online label manipulation.
+.TP
+.B EPERM
+The calling process does not have sufficient permissions to set the label.
+.TP
+.B EFAULT
+.I label
+references an inaccessible memory area.
+.SH VERSIONS
+These ioctl operations first appeared in Linux 4.18.
+They were previously known as
+.B BTRFS_IOC_GET_FSLABEL
+and
+.B BTRFS_IOC_SET_FSLABEL
+and were private to Btrfs.
+.SH CONFORMING TO
+This API is Linux-specific.
+.SH NOTES
+The maximum string length for this interface is
+.BR FSLABEL_MAX ,
+including the terminating null byte (\(aq\\0\(aq).
+Filesystems have differing maximum label lengths, which may or
+may not include the terminating null.  The string provided to
+.B FS_IOC_SETFSLABEL
+must always be null-terminated, and the string returned by
+.B FS_IOC_GETFSLABEL
+will always be null-terminated.
+.SH SEE ALSO
+.BR ioctl (2),
+.BR blkid (8)
diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2
new file mode 100644
index 0000000..2119835
--- /dev/null
+++ b/man2/ioctl_setfslabel.2
@@ -0,0 +1 @@
+.so man2/ioctl_getfslabel.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
  2018-05-10 17:29   ` [PATCH 2/2 V2] " Eric Sandeen
@ 2018-05-10 17:35     ` Eric Sandeen
  2020-04-20 12:04     ` Michael Kerrisk (man-pages)
  1 sibling, 0 replies; 22+ messages in thread
From: Eric Sandeen @ 2018-05-10 17:35 UTC (permalink / raw)
  To: Eric Sandeen, fsdevel, linux-btrfs, Linux API,
	Michael Kerrisk (man-pages)

On 5/10/18 12:29 PM, Eric Sandeen wrote:
> This documents the proposed new vfs-level ioctls which can
> get or set a mounted filesytem's label.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: make primary file ioctl_getfslabel, link ioctl_setfslabel to it
>     note that getfslabel requires CAP_SYS_ADMIN

*sigh* that should say "setfslabel" - man page is correct, patch changelog
is not.

-Eric

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2 V2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
                     ` (2 preceding siblings ...)
  2018-05-09 21:35   ` David Sterba
@ 2018-05-10 18:13   ` Eric Sandeen
  2018-05-10 19:16     ` Al Viro
  3 siblings, 1 reply; 22+ messages in thread
From: Eric Sandeen @ 2018-05-10 18:13 UTC (permalink / raw)
  To: Eric Sandeen, fsdevel, linux-btrfs, Linux API

Move the btrfs label ioctls up to the vfs for general use.

This retains 256 chars as the maximum size through the interface, which
is the btrfs limit and AFAIK exceeds any other filesystem's maximum
label size.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Reviewed-by: David Sterba <dsterba@suse.com>
---

V2: note that hoisted btrfs ioctls exist in ioctl-number.txt, new since
    reviews but I took a little license.

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 84bb74dcae12..1299df349b71 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -298,7 +298,8 @@ Code  Seq#(hex)	Include File		Comments
 0x90	00	drivers/cdrom/sbpcd.h
 0x92	00-0F	drivers/usb/mon/mon_bin.c
 0x93	60-7F	linux/auto_fs.h
-0x94	all	fs/btrfs/ioctl.h
+0x94	all	fs/btrfs/ioctl.h	Btrfs filesystem
+		and linux/fs.h		some lifted to vfs
 0x97	00-7F	fs/ceph/ioctl.h		Ceph file system
 0x99	00-0F				537-Addinboard driver
 					<mailto:buk@buks.ipn.de>
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 632e26d6f7ce..8feea790bd00 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_setflags(file, argp);
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
+	case FS_IOC_GETFSLABEL:
+		return btrfs_ioctl_get_fslabel(file, argp);
+	case FS_IOC_SETFSLABEL:
+		return btrfs_ioctl_set_fslabel(file, argp);
 	case FITRIM:
 		return btrfs_ioctl_fitrim(file, argp);
 	case BTRFS_IOC_SNAP_CREATE:
@@ -5555,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_quota_rescan_wait(file, argp);
 	case BTRFS_IOC_DEV_REPLACE:
 		return btrfs_ioctl_dev_replace(fs_info, argp);
-	case BTRFS_IOC_GET_FSLABEL:
-		return btrfs_ioctl_get_fslabel(file, argp);
-	case BTRFS_IOC_SET_FSLABEL:
-		return btrfs_ioctl_set_fslabel(file, argp);
 	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
 		return btrfs_ioctl_get_supported_features(argp);
 	case BTRFS_IOC_GET_FEATURES:
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index c8d99b9ca550..af29cc9032a2 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -823,10 +823,8 @@ enum btrfs_err_code {
 #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \
 			       struct btrfs_ioctl_quota_rescan_args)
 #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46)
-#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
-				   char[BTRFS_LABEL_SIZE])
-#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \
-				   char[BTRFS_LABEL_SIZE])
+#define BTRFS_IOC_GET_FSLABEL 	FS_IOC_GETFSLABEL
+#define BTRFS_IOC_SET_FSLABEL	FS_IOC_SETFSLABEL
 #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index d2a8313fabd7..9d132f8f2df8 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -242,6 +242,8 @@ struct fsxattr {
 #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
 #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
 
+#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
+
 #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
 #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
 #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
@@ -251,8 +253,10 @@ struct fsxattr {
 #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
 #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
 #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
-#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
-#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
+#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
+#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
+#define FS_IOC_GETFSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
+#define FS_IOC_SETFSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
 
 /*
  * File system encryption support

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2 V2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-10 18:13   ` [PATCH 1/2 V2] " Eric Sandeen
@ 2018-05-10 19:16     ` Al Viro
  2018-05-11 14:10       ` David Sterba
  0 siblings, 1 reply; 22+ messages in thread
From: Al Viro @ 2018-05-10 19:16 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, fsdevel, linux-btrfs, Linux API

On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote:
> Move the btrfs label ioctls up to the vfs for general use.
> 
> This retains 256 chars as the maximum size through the interface, which
> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> label size.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Reviewed-by: David Sterba <dsterba@suse.com>

No objections (and it obviously ought to go through btrfs tree).

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2 V2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-10 19:16     ` Al Viro
@ 2018-05-11 14:10       ` David Sterba
  2018-05-11 14:32         ` [PATCH 1/2 V2] " Chris Mason
  0 siblings, 1 reply; 22+ messages in thread
From: David Sterba @ 2018-05-11 14:10 UTC (permalink / raw)
  To: Al Viro; +Cc: Eric Sandeen, Eric Sandeen, fsdevel, linux-btrfs, Linux API

On Thu, May 10, 2018 at 08:16:09PM +0100, Al Viro wrote:
> On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote:
> > Move the btrfs label ioctls up to the vfs for general use.
> > 
> > This retains 256 chars as the maximum size through the interface, which
> > is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> > label size.
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> > Reviewed-by: David Sterba <dsterba@suse.com>
> 
> No objections (and it obviously ought to go through btrfs tree).

I can take it through my tree, but Eric mentioned that there's a patch
for xfs that depends on it. In this case it would make sense to take
both patches at once via the xfs tree. There are no pending conflicting
changes in btrfs.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2 V2] hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-11 14:10       ` David Sterba
@ 2018-05-11 14:32         ` Chris Mason
  2018-05-11 14:36           ` Eric Sandeen
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Mason @ 2018-05-11 14:32 UTC (permalink / raw)
  To: David Sterba
  Cc: Al Viro, Eric Sandeen, Eric Sandeen, fsdevel, linux-btrfs, Linux API

On 11 May 2018, at 10:10, David Sterba wrote:

> On Thu, May 10, 2018 at 08:16:09PM +0100, Al Viro wrote:
>> On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote:
>>> Move the btrfs label ioctls up to the vfs for general use.
>>>
>>> This retains 256 chars as the maximum size through the interface, 
>>> which
>>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
>>> label size.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
>>> Reviewed-by: David Sterba <dsterba@suse.com>
>>
>> No objections (and it obviously ought to go through btrfs tree).
>
> I can take it through my tree, but Eric mentioned that there's a patch
> for xfs that depends on it. In this case it would make sense to take
> both patches at once via the xfs tree. There are no pending 
> conflicting
> changes in btrfs.

Probably easiest to just have a separate pull dedicated just for this 
series.  That way it doesn't really matter which tree it goes through.

-chris

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2 V2] hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-11 14:32         ` [PATCH 1/2 V2] " Chris Mason
@ 2018-05-11 14:36           ` Eric Sandeen
  2018-05-11 14:41             ` David Sterba
  0 siblings, 1 reply; 22+ messages in thread
From: Eric Sandeen @ 2018-05-11 14:36 UTC (permalink / raw)
  To: Chris Mason, David Sterba
  Cc: Al Viro, Eric Sandeen, fsdevel, linux-btrfs, Linux API



On 5/11/18 9:32 AM, Chris Mason wrote:
> On 11 May 2018, at 10:10, David Sterba wrote:
> 
>> On Thu, May 10, 2018 at 08:16:09PM +0100, Al Viro wrote:
>>> On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote:
>>>> Move the btrfs label ioctls up to the vfs for general use.
>>>>
>>>> This retains 256 chars as the maximum size through the interface, which
>>>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
>>>> label size.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
>>>> Reviewed-by: David Sterba <dsterba@suse.com>
>>>
>>> No objections (and it obviously ought to go through btrfs tree).
>>
>> I can take it through my tree, but Eric mentioned that there's a patch
>> for xfs that depends on it. In this case it would make sense to take
>> both patches at once via the xfs tree. There are no pending conflicting
>> changes in btrfs.
> 
> Probably easiest to just have a separate pull dedicated just for this series.  That way it doesn't really matter which tree it goes through.

Actually, I just realized that the changes to include/uapi/linux/fs.h are completely
independent of any btrfs changes, right - there's nothing wrong w/ redefining
the common ioctl under a different name in btrfs.  So the fs.h patch could go first,
through the xfs tree since it'll be using it.

Once the common ioctl definition goes in, then btrfs can change to define its ioctls to
the common ioctls, or act on them directly as my patch did, etc.  Would that be
a better plan?  IOWs there's no urgent need to coordinate a btrfs change.

-Eric

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2 V2] hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-11 14:36           ` Eric Sandeen
@ 2018-05-11 14:41             ` David Sterba
  2018-05-12  0:20               ` Darrick J. Wong
  0 siblings, 1 reply; 22+ messages in thread
From: David Sterba @ 2018-05-11 14:41 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Chris Mason, David Sterba, Al Viro, Eric Sandeen, fsdevel,
	linux-btrfs, Linux API

On Fri, May 11, 2018 at 09:36:09AM -0500, Eric Sandeen wrote:
> On 5/11/18 9:32 AM, Chris Mason wrote:
> > On 11 May 2018, at 10:10, David Sterba wrote:
> > 
> >> On Thu, May 10, 2018 at 08:16:09PM +0100, Al Viro wrote:
> >>> On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote:
> >>>> Move the btrfs label ioctls up to the vfs for general use.
> >>>>
> >>>> This retains 256 chars as the maximum size through the interface, which
> >>>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> >>>> label size.
> >>>>
> >>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >>>> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> >>>> Reviewed-by: David Sterba <dsterba@suse.com>
> >>>
> >>> No objections (and it obviously ought to go through btrfs tree).
> >>
> >> I can take it through my tree, but Eric mentioned that there's a patch
> >> for xfs that depends on it. In this case it would make sense to take
> >> both patches at once via the xfs tree. There are no pending conflicting
> >> changes in btrfs.
> > 
> > Probably easiest to just have a separate pull dedicated just for this series.� That way it doesn't really matter which tree it goes through.
> 
> Actually, I just realized that the changes to include/uapi/linux/fs.h are completely
> independent of any btrfs changes, right - there's nothing wrong w/ redefining
> the common ioctl under a different name in btrfs.  So the fs.h patch could go first,
> through the xfs tree since it'll be using it.
> 
> Once the common ioctl definition goes in, then btrfs can change to define its ioctls to
> the common ioctls, or act on them directly as my patch did, etc.  Would that be
> a better plan?  IOWs there's no urgent need to coordinate a btrfs change.

Agreed, I like that plan.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2 V2] hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
  2018-05-11 14:41             ` David Sterba
@ 2018-05-12  0:20               ` Darrick J. Wong
  0 siblings, 0 replies; 22+ messages in thread
From: Darrick J. Wong @ 2018-05-12  0:20 UTC (permalink / raw)
  To: dsterba, Eric Sandeen, Chris Mason, Al Viro, Eric Sandeen,
	fsdevel, linux-btrfs, Linux API

On Fri, May 11, 2018 at 04:41:45PM +0200, David Sterba wrote:
> On Fri, May 11, 2018 at 09:36:09AM -0500, Eric Sandeen wrote:
> > On 5/11/18 9:32 AM, Chris Mason wrote:
> > > On 11 May 2018, at 10:10, David Sterba wrote:
> > > 
> > >> On Thu, May 10, 2018 at 08:16:09PM +0100, Al Viro wrote:
> > >>> On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote:
> > >>>> Move the btrfs label ioctls up to the vfs for general use.
> > >>>>
> > >>>> This retains 256 chars as the maximum size through the interface, which
> > >>>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum
> > >>>> label size.
> > >>>>
> > >>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > >>>> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> > >>>> Reviewed-by: David Sterba <dsterba@suse.com>
> > >>>
> > >>> No objections (and it obviously ought to go through btrfs tree).
> > >>
> > >> I can take it through my tree, but Eric mentioned that there's a patch
> > >> for xfs that depends on it. In this case it would make sense to take
> > >> both patches at once via the xfs tree. There are no pending conflicting
> > >> changes in btrfs.
> > > 
> > > Probably easiest to just have a separate pull dedicated just for this series.� That way it doesn't really matter which tree it goes through.
> > 
> > Actually, I just realized that the changes to include/uapi/linux/fs.h are completely
> > independent of any btrfs changes, right - there's nothing wrong w/ redefining
> > the common ioctl under a different name in btrfs.  So the fs.h patch could go first,
> > through the xfs tree since it'll be using it.
> > 
> > Once the common ioctl definition goes in, then btrfs can change to define its ioctls to
> > the common ioctls, or act on them directly as my patch did, etc.  Would that be
> > a better plan?  IOWs there's no urgent need to coordinate a btrfs change.
> 
> Agreed, I like that plan.

Ok, I'll await a new series with all the patches that Eric wants to
squeeze through the xfs tree.  I don't mind carrying the btrfs changes
too, so long as they're one-liners and the btrfs maintainers ack/rvb it.

--D

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
  2018-05-10 17:29   ` [PATCH 2/2 V2] " Eric Sandeen
  2018-05-10 17:35     ` Eric Sandeen
@ 2020-04-20 12:04     ` Michael Kerrisk (man-pages)
  2020-04-20 13:48       ` Eric Sandeen
  1 sibling, 1 reply; 22+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-20 12:04 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Eric Sandeen, fsdevel, linux-btrfs, Linux API, linux-man,
	Michael Kerrisk

Hello Eric,

So it seems like this feature eventually got merged in Linux 4.18. Is
this page up to date with what went into the kernel?

Thanks,

Michael

On Thu, 10 May 2018 at 19:29, Eric Sandeen <sandeen@sandeen.net> wrote:
>
> This documents the proposed new vfs-level ioctls which can
> get or set a mounted filesytem's label.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> V2: make primary file ioctl_getfslabel, link ioctl_setfslabel to it
>     note that getfslabel requires CAP_SYS_ADMIN
>
> diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2
> new file mode 100644
> index 0000000..2c3375c
> --- /dev/null
> +++ b/man2/ioctl_getfslabel.2
> @@ -0,0 +1,87 @@
> +.\" Copyright (c) 2018, Red Hat, Inc.  All rights reserved.
> +.\"
> +.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
> +.\" This is free documentation; you can redistribute it and/or
> +.\" modify it under the terms of the GNU General Public License as
> +.\" published by the Free Software Foundation; either version 2 of
> +.\" the License, or (at your option) any later version.
> +.\"
> +.\" The GNU General Public License's references to "object code"
> +.\" and "executables" are to be interpreted as the output of any
> +.\" document formatting or typesetting system, including
> +.\" intermediate and printed output.
> +.\"
> +.\" This manual is distributed in the hope that it will be useful,
> +.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
> +.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +.\" GNU General Public License for more details.
> +.\"
> +.\" You should have received a copy of the GNU General Public
> +.\" License along with this manual; if not, see
> +.\" <http://www.gnu.org/licenses/>.
> +.\" %%%LICENSE_END
> +.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual"
> +.SH NAME
> +ioctl_fslabel \- get or set a filesystem label
> +.SH SYNOPSIS
> +.br
> +.B #include <sys/ioctl.h>
> +.br
> +.B #include <linux/fs.h>
> +.sp
> +.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]);
> +.br
> +.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]);
> +.SH DESCRIPTION
> +If a filesystem supports online label manipulation, these
> +.BR ioctl (2)
> +operations can be used to get or set the filesystem label for the filesystem
> +on which
> +.B fd
> +resides.
> +The
> +.B FS_IOC_SETFSLABEL
> +operation requires privilege
> +.RB ( CAP_SYS_ADMIN ).
> +.SH RETURN VALUE
> +On success zero is returned.  On error, \-1 is returned, and
> +.I errno
> +is set to indicate the error.
> +.PP
> +.SH ERRORS
> +Error codes can be one of, but are not limited to, the following:
> +.TP
> +.B EINVAL
> +The specified label exceeds the maximum label length for the filesystem.
> +.TP
> +.B ENOTTY
> +This can appear if the filesystem does not support online label manipulation.
> +.TP
> +.B EPERM
> +The calling process does not have sufficient permissions to set the label.
> +.TP
> +.B EFAULT
> +.I label
> +references an inaccessible memory area.
> +.SH VERSIONS
> +These ioctl operations first appeared in Linux 4.18.
> +They were previously known as
> +.B BTRFS_IOC_GET_FSLABEL
> +and
> +.B BTRFS_IOC_SET_FSLABEL
> +and were private to Btrfs.
> +.SH CONFORMING TO
> +This API is Linux-specific.
> +.SH NOTES
> +The maximum string length for this interface is
> +.BR FSLABEL_MAX ,
> +including the terminating null byte (\(aq\\0\(aq).
> +Filesystems have differing maximum label lengths, which may or
> +may not include the terminating null.  The string provided to
> +.B FS_IOC_SETFSLABEL
> +must always be null-terminated, and the string returned by
> +.B FS_IOC_GETFSLABEL
> +will always be null-terminated.
> +.SH SEE ALSO
> +.BR ioctl (2),
> +.BR blkid (8)
> diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2
> new file mode 100644
> index 0000000..2119835
> --- /dev/null
> +++ b/man2/ioctl_setfslabel.2
> @@ -0,0 +1 @@
> +.so man2/ioctl_getfslabel.2
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
  2020-04-20 12:04     ` Michael Kerrisk (man-pages)
@ 2020-04-20 13:48       ` Eric Sandeen
  2020-04-20 15:29         ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 22+ messages in thread
From: Eric Sandeen @ 2020-04-20 13:48 UTC (permalink / raw)
  To: mtk.manpages, Eric Sandeen; +Cc: fsdevel, linux-btrfs, Linux API, linux-man

On 4/20/20 7:04 AM, Michael Kerrisk (man-pages) wrote:
> Hello Eric,
> 
> So it seems like this feature eventually got merged in Linux 4.18. Is
> this page up to date with what went into the kernel?

Yes, I believe that it's all still accurate.

Thanks,
-Eric

> Thanks,
> 
> Michael
> 
> On Thu, 10 May 2018 at 19:29, Eric Sandeen <sandeen@sandeen.net> wrote:
>>
>> This documents the proposed new vfs-level ioctls which can
>> get or set a mounted filesytem's label.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> V2: make primary file ioctl_getfslabel, link ioctl_setfslabel to it
>>     note that getfslabel requires CAP_SYS_ADMIN
>>
>> diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2
>> new file mode 100644
>> index 0000000..2c3375c
>> --- /dev/null
>> +++ b/man2/ioctl_getfslabel.2
>> @@ -0,0 +1,87 @@
>> +.\" Copyright (c) 2018, Red Hat, Inc.  All rights reserved.
>> +.\"
>> +.\" %%%LICENSE_START(GPLv2+_DOC_FULL)
>> +.\" This is free documentation; you can redistribute it and/or
>> +.\" modify it under the terms of the GNU General Public License as
>> +.\" published by the Free Software Foundation; either version 2 of
>> +.\" the License, or (at your option) any later version.
>> +.\"
>> +.\" The GNU General Public License's references to "object code"
>> +.\" and "executables" are to be interpreted as the output of any
>> +.\" document formatting or typesetting system, including
>> +.\" intermediate and printed output.
>> +.\"
>> +.\" This manual is distributed in the hope that it will be useful,
>> +.\" but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +.\" GNU General Public License for more details.
>> +.\"
>> +.\" You should have received a copy of the GNU General Public
>> +.\" License along with this manual; if not, see
>> +.\" <http://www.gnu.org/licenses/>.
>> +.\" %%%LICENSE_END
>> +.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual"
>> +.SH NAME
>> +ioctl_fslabel \- get or set a filesystem label
>> +.SH SYNOPSIS
>> +.br
>> +.B #include <sys/ioctl.h>
>> +.br
>> +.B #include <linux/fs.h>
>> +.sp
>> +.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]);
>> +.br
>> +.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]);
>> +.SH DESCRIPTION
>> +If a filesystem supports online label manipulation, these
>> +.BR ioctl (2)
>> +operations can be used to get or set the filesystem label for the filesystem
>> +on which
>> +.B fd
>> +resides.
>> +The
>> +.B FS_IOC_SETFSLABEL
>> +operation requires privilege
>> +.RB ( CAP_SYS_ADMIN ).
>> +.SH RETURN VALUE
>> +On success zero is returned.  On error, \-1 is returned, and
>> +.I errno
>> +is set to indicate the error.
>> +.PP
>> +.SH ERRORS
>> +Error codes can be one of, but are not limited to, the following:
>> +.TP
>> +.B EINVAL
>> +The specified label exceeds the maximum label length for the filesystem.
>> +.TP
>> +.B ENOTTY
>> +This can appear if the filesystem does not support online label manipulation.
>> +.TP
>> +.B EPERM
>> +The calling process does not have sufficient permissions to set the label.
>> +.TP
>> +.B EFAULT
>> +.I label
>> +references an inaccessible memory area.
>> +.SH VERSIONS
>> +These ioctl operations first appeared in Linux 4.18.
>> +They were previously known as
>> +.B BTRFS_IOC_GET_FSLABEL
>> +and
>> +.B BTRFS_IOC_SET_FSLABEL
>> +and were private to Btrfs.
>> +.SH CONFORMING TO
>> +This API is Linux-specific.
>> +.SH NOTES
>> +The maximum string length for this interface is
>> +.BR FSLABEL_MAX ,
>> +including the terminating null byte (\(aq\\0\(aq).
>> +Filesystems have differing maximum label lengths, which may or
>> +may not include the terminating null.  The string provided to
>> +.B FS_IOC_SETFSLABEL
>> +must always be null-terminated, and the string returned by
>> +.B FS_IOC_GETFSLABEL
>> +will always be null-terminated.
>> +.SH SEE ALSO
>> +.BR ioctl (2),
>> +.BR blkid (8)
>> diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2
>> new file mode 100644
>> index 0000000..2119835
>> --- /dev/null
>> +++ b/man2/ioctl_setfslabel.2
>> @@ -0,0 +1 @@
>> +.so man2/ioctl_getfslabel.2
>>
> 
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
  2020-04-20 13:48       ` Eric Sandeen
@ 2020-04-20 15:29         ` Michael Kerrisk (man-pages)
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-04-20 15:29 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, fsdevel, linux-btrfs, Linux API, linux-man

Hello Eric,

On Mon, 20 Apr 2020 at 15:48, Eric Sandeen <sandeen@redhat.com> wrote:
>
> On 4/20/20 7:04 AM, Michael Kerrisk (man-pages) wrote:
> > Hello Eric,
> >
> > So it seems like this feature eventually got merged in Linux 4.18. Is
> > this page up to date with what went into the kernel?
>
> Yes, I believe that it's all still accurate.

Thanks. I've merged the page.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-04-20 15:30 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-09 15:56 [PATCH 0/2] hoist btrfs' label set/get ioctls to vfs, and document Eric Sandeen
2018-05-09 16:01 ` [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs Eric Sandeen
2018-05-09 16:10   ` Darrick J. Wong
2018-05-09 17:15     ` Andreas Dilger
2018-05-09 17:26       ` Darrick J. Wong
2018-05-09 17:35   ` Randy Dunlap
2018-05-09 17:40     ` Eric Sandeen
2018-05-09 21:35   ` David Sterba
2018-05-10 18:13   ` [PATCH 1/2 V2] " Eric Sandeen
2018-05-10 19:16     ` Al Viro
2018-05-11 14:10       ` David Sterba
2018-05-11 14:32         ` [PATCH 1/2 V2] " Chris Mason
2018-05-11 14:36           ` Eric Sandeen
2018-05-11 14:41             ` David Sterba
2018-05-12  0:20               ` Darrick J. Wong
2018-05-09 16:04 ` [PATCH 2/2] man2: New page documenting filesystem get/set label ioctls Eric Sandeen
2018-05-09 16:15   ` Darrick J. Wong
2018-05-10 17:29   ` [PATCH 2/2 V2] " Eric Sandeen
2018-05-10 17:35     ` Eric Sandeen
2020-04-20 12:04     ` Michael Kerrisk (man-pages)
2020-04-20 13:48       ` Eric Sandeen
2020-04-20 15:29         ` Michael Kerrisk (man-pages)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).