Hi all,
this patchset adds a few new ioctls to standardize a few interfaces we
want
- get/set UUID
- get sysfs path
The get/set UUID ioctls are lifted versions of the ext4 ioctls with one
difference, killing the flexible array member - we'll never have UUIDs
more than 16 bytes, and getting rid of the flexible array member makes
them easier to use.
FS_IOC_GETSYSFSNAME is new, but it addresses something that we've been
doing in fs specific code for awhile - "given a path on a mounted
filesystem, tell me where it lives in sysfs".
Cheers,
Kent
Kent Overstreet (6):
fs: super_block->s_uuid_len
fs: FS_IOC_GETUUID
fat: Hook up sb->s_uuid
fs: FS_IOC_GETSYSFSNAME
xfs: add support for FS_IOC_GETSYSFSNAME
bcachefs: add support for FS_IOC_GETSYSFSNAME
fs/bcachefs/fs.c | 1 +
fs/fat/inode.c | 4 ++++
fs/ioctl.c | 33 +++++++++++++++++++++++++++++++++
fs/super.c | 1 +
fs/xfs/xfs_mount.c | 2 ++
include/linux/fs.h | 2 ++
include/uapi/linux/fs.h | 21 +++++++++++++++++++++
7 files changed, 64 insertions(+)
--
2.43.0
Add a new generic ioctls for querying the filesystem UUID.
These are lifted versions of the ext4 ioctls, with one change: we're not
using a flexible array member, because UUIDs will never be more than 16
bytes.
This patch adds a generic implementation of FS_IOC_GETFSUUID, which
reads from super_block->s_uuid; FS_IOC_SETFSUUID is left for individual
filesystems to implement.
Signed-off-by: Kent Overstreet <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Dave Chinner <[email protected]>
Cc: "Darrick J. Wong" <[email protected]>
Cc: Theodore Ts'o <[email protected]>
Cc: [email protected]
Signed-off-by: Kent Overstreet <[email protected]>
---
fs/ioctl.c | 16 ++++++++++++++++
include/uapi/linux/fs.h | 16 ++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 76cf22ac97d7..858801060408 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -763,6 +763,19 @@ static int ioctl_fssetxattr(struct file *file, void __user *argp)
return err;
}
+static int ioctl_getfsuuid(struct file *file, void __user *argp)
+{
+ struct super_block *sb = file_inode(file)->i_sb;
+
+ if (WARN_ON(sb->s_uuid_len > sizeof(sb->s_uuid)))
+ sb->s_uuid_len = sizeof(sb->s_uuid);
+
+ struct fsuuid2 u = { .fsu_len = sb->s_uuid_len, };
+ memcpy(&u.fsu_uuid[0], &sb->s_uuid, sb->s_uuid_len);
+
+ return copy_to_user(argp, &u, sizeof(u)) ? -EFAULT : 0;
+}
+
/*
* do_vfs_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
* It's just a simple helper for sys_ioctl and compat_sys_ioctl.
@@ -845,6 +858,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
case FS_IOC_FSSETXATTR:
return ioctl_fssetxattr(filp, argp);
+ case FS_IOC_GETFSUUID:
+ return ioctl_getfsuuid(filp, argp);
+
default:
if (S_ISREG(inode->i_mode))
return file_ioctl(filp, cmd, argp);
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 48ad69f7722e..0389fea87db5 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -64,6 +64,20 @@ struct fstrim_range {
__u64 minlen;
};
+/*
+ * We include a length field because some filesystems (vfat) have an identifier
+ * that we do want to expose as a UUID, but doesn't have the standard length.
+ *
+ * We use a fixed size buffer beacuse this interface will, by fiat, never
+ * support "UUIDs" longer than 16 bytes; we don't want to force all downstream
+ * users to have to deal with that.
+ */
+struct fsuuid2 {
+ __u32 fsu_len;
+ __u32 fsu_flags;
+ __u8 fsu_uuid[16];
+};
+
/* extent-same (dedupe) ioctls; these MUST match the btrfs ioctl definitions */
#define FILE_DEDUPE_RANGE_SAME 0
#define FILE_DEDUPE_RANGE_DIFFERS 1
@@ -215,6 +229,8 @@ 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])
+#define FS_IOC_GETFSUUID _IOR(0x94, 51, struct fsuuid2)
+#define FS_IOC_SETFSUUID _IOW(0x94, 52, struct fsuuid2)
/*
* Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)
--
2.43.0
Signed-off-by: Kent Overstreet <[email protected]>
---
fs/bcachefs/fs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 77ea61090e91..50b2fd3ddd23 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1947,6 +1947,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
sb->s_time_min = div_s64(S64_MIN, c->sb.time_units_per_sec) + 1;
sb->s_time_max = div_s64(S64_MAX, c->sb.time_units_per_sec);
sb->s_uuid = c->sb.user_uuid;
+ snprintf(sb->s_sysfs_name, sizeof(sb->s_sysfs_name), "%pU", c->sb.user_uuid.b);
c->vfs_sb = sb;
strscpy(sb->s_id, c->name, sizeof(sb->s_id));
--
2.43.0
Add a new ioctl for getting the sysfs name of a filesystem - the path
under /sys/fs.
This is going to let us standardize exporting data from sysfs across
filesystems, e.g. time stats.
The returned path will always be of the form "$FSTYP/$SYSFS_IDENTIFIER",
where the sysfs identifier may be a UUID (for bcachefs) or a device name
(xfs).
Signed-off-by: Kent Overstreet <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Dave Chinner <[email protected]>
Cc: "Darrick J. Wong" <[email protected]>
Cc: Theodore Ts'o <[email protected]>
Cc: Josef Bacik <[email protected]>
---
fs/ioctl.c | 17 +++++++++++++++++
include/linux/fs.h | 1 +
include/uapi/linux/fs.h | 5 +++++
3 files changed, 23 insertions(+)
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 858801060408..cb3690811d3d 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -776,6 +776,20 @@ static int ioctl_getfsuuid(struct file *file, void __user *argp)
return copy_to_user(argp, &u, sizeof(u)) ? -EFAULT : 0;
}
+static int ioctl_getfssysfsname(struct file *file, void __user *argp)
+{
+ struct super_block *sb = file_inode(file)->i_sb;
+
+ if (!strlen(sb->s_sysfs_name))
+ return -ENOIOCTLCMD;
+
+ struct fssysfsname u = {};
+
+ snprintf(u.name, sizeof(u.name), "%s/%s", sb->s_type->name, sb->s_sysfs_name);
+
+ return copy_to_user(argp, &u, sizeof(u)) ? -EFAULT : 0;
+}
+
/*
* do_vfs_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
* It's just a simple helper for sys_ioctl and compat_sys_ioctl.
@@ -861,6 +875,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
case FS_IOC_GETFSUUID:
return ioctl_getfsuuid(filp, argp);
+ case FS_IOC_GETFSSYSFSNAME:
+ return ioctl_getfssysfsname(filp, argp);
+
default:
if (S_ISREG(inode->i_mode))
return file_ioctl(filp, cmd, argp);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ff41ea6c3a9c..7f23f593f17c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1258,6 +1258,7 @@ struct super_block {
char s_id[32]; /* Informational name */
uuid_t s_uuid; /* UUID */
u8 s_uuid_len; /* Default 16, possibly smaller for weird filesystems */
+ char s_sysfs_name[UUID_STRING_LEN + 1];
unsigned int s_max_links;
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 0389fea87db5..6dd14a453277 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -78,6 +78,10 @@ struct fsuuid2 {
__u8 fsu_uuid[16];
};
+struct fssysfsname {
+ __u8 name[64];
+};
+
/* extent-same (dedupe) ioctls; these MUST match the btrfs ioctl definitions */
#define FILE_DEDUPE_RANGE_SAME 0
#define FILE_DEDUPE_RANGE_DIFFERS 1
@@ -231,6 +235,7 @@ struct fsxattr {
#define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX])
#define FS_IOC_GETFSUUID _IOR(0x94, 51, struct fsuuid2)
#define FS_IOC_SETFSUUID _IOW(0x94, 52, struct fsuuid2)
+#define FS_IOC_GETFSSYSFSNAME _IOR(0x94, 53, struct fssysfsname)
/*
* Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)
--
2.43.0
On Mon, Feb 05, 2024 at 02:27:32PM -0800, Darrick J. Wong wrote:
> On Mon, Feb 05, 2024 at 03:05:15PM -0500, Kent Overstreet wrote:
> > Add a new ioctl for getting the sysfs name of a filesystem - the path
> > under /sys/fs.
> >
> > This is going to let us standardize exporting data from sysfs across
> > filesystems, e.g. time stats.
> >
> > The returned path will always be of the form "$FSTYP/$SYSFS_IDENTIFIER",
> > where the sysfs identifier may be a UUID (for bcachefs) or a device name
> > (xfs).
> >
> > Signed-off-by: Kent Overstreet <[email protected]>
> > Cc: Christian Brauner <[email protected]>
> > Cc: Jan Kara <[email protected]>
> > Cc: Dave Chinner <[email protected]>
> > Cc: "Darrick J. Wong" <[email protected]>
> > Cc: Theodore Ts'o <[email protected]>
> > Cc: Josef Bacik <[email protected]>
> > ---
> > fs/ioctl.c | 17 +++++++++++++++++
> > include/linux/fs.h | 1 +
> > include/uapi/linux/fs.h | 5 +++++
> > 3 files changed, 23 insertions(+)
> >
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 858801060408..cb3690811d3d 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -776,6 +776,20 @@ static int ioctl_getfsuuid(struct file *file, void __user *argp)
> > return copy_to_user(argp, &u, sizeof(u)) ? -EFAULT : 0;
> > }
> >
> > +static int ioctl_getfssysfsname(struct file *file, void __user *argp)
>
> ackpthspacesplease.
>
> "ioctl_get_fs_sysfs_name"?
It did feel a bit trolling writing that :)
>
> > +{
> > + struct super_block *sb = file_inode(file)->i_sb;
> > +
> > + if (!strlen(sb->s_sysfs_name))
> > + return -ENOIOCTLCMD;
> > +
> > + struct fssysfsname u = {};
> > +
> > + snprintf(u.name, sizeof(u.name), "%s/%s", sb->s_type->name, sb->s_sysfs_name);
>
> Does this actually guarantee that there will be a trailing null in the
> output? It's really stupid that GETFSLABEL can return an unterminated
> string if the label is exactly the size of the char array.
It's snprintf, so yes.
(queue another "why are we using raw char arrays everywhere in 2024"
rant, I have to double check this stuff too).
>
> > +
> > + return copy_to_user(argp, &u, sizeof(u)) ? -EFAULT : 0;
> > +}
> > +
> > /*
> > * do_vfs_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
> > * It's just a simple helper for sys_ioctl and compat_sys_ioctl.
> > @@ -861,6 +875,9 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > case FS_IOC_GETFSUUID:
> > return ioctl_getfsuuid(filp, argp);
> >
> > + case FS_IOC_GETFSSYSFSNAME:
>
> File System Ioctl Get File System System File System Name.
>
> Yuck.
>
> FS_IOC_GETSYSFSPATH?
>
> Also, do we want to establish that this works for /sys/fs and
> /sys/kernel/debug at the same time?
Yeah, I'll add a comment to that effect.
>
> > + return ioctl_getfssysfsname(filp, argp);
> > +
> > default:
> > if (S_ISREG(inode->i_mode))
> > return file_ioctl(filp, cmd, argp);
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index ff41ea6c3a9c..7f23f593f17c 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1258,6 +1258,7 @@ struct super_block {
> > char s_id[32]; /* Informational name */
> > uuid_t s_uuid; /* UUID */
> > u8 s_uuid_len; /* Default 16, possibly smaller for weird filesystems */
> > + char s_sysfs_name[UUID_STRING_LEN + 1];
> >
> > unsigned int s_max_links;
> >
> > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > index 0389fea87db5..6dd14a453277 100644
> > --- a/include/uapi/linux/fs.h
> > +++ b/include/uapi/linux/fs.h
> > @@ -78,6 +78,10 @@ struct fsuuid2 {
> > __u8 fsu_uuid[16];
> > };
> >
> > +struct fssysfsname {
> > + __u8 name[64];
> > +};
> > +
> > /* extent-same (dedupe) ioctls; these MUST match the btrfs ioctl definitions */
> > #define FILE_DEDUPE_RANGE_SAME 0
> > #define FILE_DEDUPE_RANGE_DIFFERS 1
> > @@ -231,6 +235,7 @@ struct fsxattr {
> > #define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX])
> > #define FS_IOC_GETFSUUID _IOR(0x94, 51, struct fsuuid2)
> > #define FS_IOC_SETFSUUID _IOW(0x94, 52, struct fsuuid2)
> > +#define FS_IOC_GETFSSYSFSNAME _IOR(0x94, 53, struct fssysfsname)
>
> 0x94 is btrfs, don't add things to their "name" space.
Can we please document this somewhere!?
What, dare I ask, is the "namespace" I should be using?
On Mon, Feb 05, 2024 at 08:20:10PM -0800, Randy Dunlap wrote:
>
>
> On 2/5/24 17:39, David Sterba wrote:
> > On Mon, Feb 05, 2024 at 05:43:37PM -0500, Kent Overstreet wrote:
> >> On Mon, Feb 05, 2024 at 02:27:32PM -0800, Darrick J. Wong wrote:
> >>> On Mon, Feb 05, 2024 at 03:05:15PM -0500, Kent Overstreet wrote:
> >>>> @@ -231,6 +235,7 @@ struct fsxattr {
> >>>> #define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX])
> >>>> #define FS_IOC_GETFSUUID _IOR(0x94, 51, struct fsuuid2)
> >>>> #define FS_IOC_SETFSUUID _IOW(0x94, 52, struct fsuuid2)
> >>>> +#define FS_IOC_GETFSSYSFSNAME _IOR(0x94, 53, struct fssysfsname)
> >>>
> >>> 0x94 is btrfs, don't add things to their "name" space.
> >>
> >> Can we please document this somewhere!?
> >>
> >> What, dare I ask, is the "namespace" I should be using?
> >
> > Grep for _IOCTL_MAGIC in include/uapi:
> >
> > uapi/linux/aspeed-lpc-ctrl.h:#define __ASPEED_LPC_CTRL_IOCTL_MAGIC 0xb2
> > uapi/linux/aspeed-p2a-ctrl.h:#define __ASPEED_P2A_CTRL_IOCTL_MAGIC 0xb3
> > uapi/linux/bt-bmc.h:#define __BT_BMC_IOCTL_MAGIC 0xb1
> > uapi/linux/btrfs.h:#define BTRFS_IOCTL_MAGIC 0x94
> > uapi/linux/f2fs.h:#define F2FS_IOCTL_MAGIC 0xf5
> > uapi/linux/ipmi_bmc.h:#define __IPMI_BMC_IOCTL_MAGIC 0xB1
> > uapi/linux/pfrut.h:#define PFRUT_IOCTL_MAGIC 0xEE
> > uapi/rdma/rdma_user_ioctl.h:#define IB_IOCTL_MAGIC RDMA_IOCTL_MAGIC
> > uapi/rdma/rdma_user_ioctl_cmds.h:#define RDMA_IOCTL_MAGIC 0x1b
> >
> > The label ioctls inherited the 0x94 namespace for backward
> > compatibility but as already said, it's the private namespace of btrfs.
> >
>
> or more generally, see Documentation/userspace-api/ioctl/ioctl-number.rst.
>
> For 0x94, it says:
>
> 0x94 all fs/btrfs/ioctl.h Btrfs filesystem
> and linux/fs.h some lifted to vfs/generic
You guys keep giving the same info over and over again, instead of
anything that would be actually helpful...
Does anyone know what the proper "namespace" is for new VFS level
ioctls?
..Anyone?
On Tue, Feb 6, 2024 at 12:49 AM Kent Overstreet
<[email protected]> wrote:
>
> On Tue, Feb 06, 2024 at 09:17:58AM +1100, Dave Chinner wrote:
> > On Mon, Feb 05, 2024 at 03:05:13PM -0500, Kent Overstreet wrote:
> > > Add a new generic ioctls for querying the filesystem UUID.
> > >
> > > These are lifted versions of the ext4 ioctls, with one change: we're not
> > > using a flexible array member, because UUIDs will never be more than 16
> > > bytes.
> > >
> > > This patch adds a generic implementation of FS_IOC_GETFSUUID, which
> > > reads from super_block->s_uuid; FS_IOC_SETFSUUID is left for individual
> > > filesystems to implement.
> > >
It's fine to have a generic implementation, but the filesystem should
have the option to opt-in for a specific implementation.
There are several examples, even with xfs and btrfs where ->s_uuid
does not contain the filesystem's UUID or there is more than one
uuid and ->s_uuid is not the correct one to expose to the user.
A model like ioctl_[gs]etflags() looks much more appropriate
and could be useful for network filesystems/FUSE as well.
> > > Signed-off-by: Kent Overstreet <[email protected]>
> > > Cc: Christian Brauner <[email protected]>
> > > Cc: Jan Kara <[email protected]>
> > > Cc: Dave Chinner <[email protected]>
> > > Cc: "Darrick J. Wong" <[email protected]>
> > > Cc: Theodore Ts'o <[email protected]>
> > > Cc: [email protected]
> > > Signed-off-by: Kent Overstreet <[email protected]>
> > > ---
> > > fs/ioctl.c | 16 ++++++++++++++++
> > > include/uapi/linux/fs.h | 16 ++++++++++++++++
> > > 2 files changed, 32 insertions(+)
> > >
> > > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > > index 76cf22ac97d7..858801060408 100644
> > > --- a/fs/ioctl.c
> > > +++ b/fs/ioctl.c
> > > @@ -763,6 +763,19 @@ static int ioctl_fssetxattr(struct file *file, void __user *argp)
> > > return err;
> > > }
> > >
> > > +static int ioctl_getfsuuid(struct file *file, void __user *argp)
> > > +{
> > > + struct super_block *sb = file_inode(file)->i_sb;
> > > +
> > > + if (WARN_ON(sb->s_uuid_len > sizeof(sb->s_uuid)))
> > > + sb->s_uuid_len = sizeof(sb->s_uuid);
> >
> > A "get"/read only ioctl should not be change superblock fields -
> > this is not the place for enforcing superblock filed constraints.
> > Make a helper function super_set_uuid(sb, uuid, uuid_len) for the
> > filesystems to call that does all the validity checking and then
> > sets the superblock fields appropriately.
>
> *nod* good thought...
>
> > > +struct fsuuid2 {
> > > + __u32 fsu_len;
> > > + __u32 fsu_flags;
> > > + __u8 fsu_uuid[16];
> > > +};
> >
> > Nobody in userspace will care that this is "version 2" of the ext4
> > ioctl. I'd just name it "fs_uuid" as though the ext4 version didn't
> > ever exist.
>
> I considered that - but I decided I wanted the explicit versioning,
> because too often we live with unfixed mistakes because versioning is
> ugly, or something?
>
> Doing a new revision of an API should be a normal, frequent thing, and I
> want to start making it a convention.
>
> >
> > > +
> > > /* extent-same (dedupe) ioctls; these MUST match the btrfs ioctl definitions */
> > > #define FILE_DEDUPE_RANGE_SAME 0
> > > #define FILE_DEDUPE_RANGE_DIFFERS 1
> > > @@ -215,6 +229,8 @@ 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])
> > > +#define FS_IOC_GETFSUUID _IOR(0x94, 51, struct fsuuid2)
> > > +#define FS_IOC_SETFSUUID _IOW(0x94, 52, struct fsuuid2)
> >
> > 0x94 is the btrfs ioctl space, not the VFS space - why did you
> > choose that? That said, what is the VFS ioctl space identifier? 'v',
> > perhaps?
>
> "Promoting ioctls from fs to vfs without revising and renaming
> considered harmful"... this is a mess that could have been avoided if we
> weren't taking the lazy route.
>
> And 'v' doesn't look like it to me, I really have no idea what to use
> here. Does anyone?
>
All the other hoisted FS_IOC_* use the original fs ioctl namespace they
came from. Although it is not an actual hoist, I'd use:
struct fsuuid128 {
__u32 fsu_len;
__u32 fsu_flags;
__u8 fsu_uuid[16];
};
#define FS_IOC_GETFSUUID _IOR('f', 45, struct fsuuid128)
#define FS_IOC_SETFSUUID _IOW('f', 46, struct fsuuid128)
Technically, could also overload EXT4_IOC_[GS]ETFSUUID numbers
because of the different type:
#define FS_IOC_GETFSUUID _IOR('f', 44, struct fsuuid128)
#define FS_IOC_SETFSUUID _IOW('f', 44, struct fsuuid128)
and then ext4 can follow up with this patch, because as far as I can tell,
the ext4 implementation is already compatible with the new ioctls.
Thanks,
Amir.
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1613,8 +1613,10 @@ static long __ext4_ioctl(struct file *filp,
unsigned int cmd, unsigned long arg)
return ext4_ioctl_setlabel(filp,
(const void __user *)arg);
+ case FS_IOC_GETFSUUID:
case EXT4_IOC_GETFSUUID:
return ext4_ioctl_getuuid(EXT4_SB(sb), (void __user *)arg);
+ case FS_IOC_SETFSUUID:
case EXT4_IOC_SETFSUUID:
return ext4_ioctl_setuuid(filp, (const void __user *)arg);
On 2/5/24 17:39, David Sterba wrote:
> On Mon, Feb 05, 2024 at 05:43:37PM -0500, Kent Overstreet wrote:
>> On Mon, Feb 05, 2024 at 02:27:32PM -0800, Darrick J. Wong wrote:
>>> On Mon, Feb 05, 2024 at 03:05:15PM -0500, Kent Overstreet wrote:
>>>> @@ -231,6 +235,7 @@ struct fsxattr {
>>>> #define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX])
>>>> #define FS_IOC_GETFSUUID _IOR(0x94, 51, struct fsuuid2)
>>>> #define FS_IOC_SETFSUUID _IOW(0x94, 52, struct fsuuid2)
>>>> +#define FS_IOC_GETFSSYSFSNAME _IOR(0x94, 53, struct fssysfsname)
>>>
>>> 0x94 is btrfs, don't add things to their "name" space.
>>
>> Can we please document this somewhere!?
>>
>> What, dare I ask, is the "namespace" I should be using?
>
> Grep for _IOCTL_MAGIC in include/uapi:
>
> uapi/linux/aspeed-lpc-ctrl.h:#define __ASPEED_LPC_CTRL_IOCTL_MAGIC 0xb2
> uapi/linux/aspeed-p2a-ctrl.h:#define __ASPEED_P2A_CTRL_IOCTL_MAGIC 0xb3
> uapi/linux/bt-bmc.h:#define __BT_BMC_IOCTL_MAGIC 0xb1
> uapi/linux/btrfs.h:#define BTRFS_IOCTL_MAGIC 0x94
> uapi/linux/f2fs.h:#define F2FS_IOCTL_MAGIC 0xf5
> uapi/linux/ipmi_bmc.h:#define __IPMI_BMC_IOCTL_MAGIC 0xB1
> uapi/linux/pfrut.h:#define PFRUT_IOCTL_MAGIC 0xEE
> uapi/rdma/rdma_user_ioctl.h:#define IB_IOCTL_MAGIC RDMA_IOCTL_MAGIC
> uapi/rdma/rdma_user_ioctl_cmds.h:#define RDMA_IOCTL_MAGIC 0x1b
>
> The label ioctls inherited the 0x94 namespace for backward
> compatibility but as already said, it's the private namespace of btrfs.
>
or more generally, see Documentation/userspace-api/ioctl/ioctl-number.rst.
For 0x94, it says:
0x94 all fs/btrfs/ioctl.h Btrfs filesystem
and linux/fs.h some lifted to vfs/generic
--
#Randy
On Tue, Feb 06, 2024 at 10:24:45AM +0200, Amir Goldstein wrote:
> On Tue, Feb 6, 2024 at 12:49 AM Kent Overstreet
> <[email protected]> wrote:
> >
> > On Tue, Feb 06, 2024 at 09:17:58AM +1100, Dave Chinner wrote:
> > > On Mon, Feb 05, 2024 at 03:05:13PM -0500, Kent Overstreet wrote:
> > > > Add a new generic ioctls for querying the filesystem UUID.
> > > >
> > > > These are lifted versions of the ext4 ioctls, with one change: we're not
> > > > using a flexible array member, because UUIDs will never be more than 16
> > > > bytes.
> > > >
> > > > This patch adds a generic implementation of FS_IOC_GETFSUUID, which
> > > > reads from super_block->s_uuid; FS_IOC_SETFSUUID is left for individual
> > > > filesystems to implement.
> > > >
>
> It's fine to have a generic implementation, but the filesystem should
> have the option to opt-in for a specific implementation.
>
> There are several examples, even with xfs and btrfs where ->s_uuid
> does not contain the filesystem's UUID or there is more than one
> uuid and ->s_uuid is not the correct one to expose to the user.
Yeah, some of you were smoking some good stuff from the stories I've
been hearing...
> A model like ioctl_[gs]etflags() looks much more appropriate
> and could be useful for network filesystems/FUSE as well.
A filesystem needs to store two UUIDs (that identify the filesystem as a
whole).
- Your internal UUID, which can never change because it's referenced in
various other on disk data structures
- Your external UUID, which identifies the filesystem to the outside
world. Users want to be able to change this - which is why it has to
be distinct from the internal UUID.
The internal UUID must never be exposed to the outside world, and that
includes the VFS; storing your private UUID in sb->s_uuid is wrong -
separation of concerns.
yes, I am aware of fscrypt, and yes, someone's going to have to fix
that.
This interface is only for the external/public UUID.