| Message ID | 20221118211408.72796-2-catherine.hoang@oracle.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | porting the GETFSUUID ioctl to xfs | expand |
On Fri, Nov 18, 2022 at 01:14:07PM -0800, Catherine Hoang wrote: > Hoist the EXT4_IOC_[GS]ETFSUUID ioctls so that they can be used by all > filesystems. This allows us to have a common interface for tools such as > coreutils. > > Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/ext4/ext4.h | 13 ++----------- > include/uapi/linux/fs.h | 11 +++++++++++ > 2 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 8d5453852f98..b200302a3732 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -722,8 +722,8 @@ enum { > #define EXT4_IOC_GETSTATE _IOW('f', 41, __u32) > #define EXT4_IOC_GET_ES_CACHE _IOWR('f', 42, struct fiemap) > #define EXT4_IOC_CHECKPOINT _IOW('f', 43, __u32) > -#define EXT4_IOC_GETFSUUID _IOR('f', 44, struct fsuuid) > -#define EXT4_IOC_SETFSUUID _IOW('f', 44, struct fsuuid) > +#define EXT4_IOC_GETFSUUID FS_IOC_GETFSUUID > +#define EXT4_IOC_SETFSUUID FS_IOC_SETFSUUID > > #define EXT4_IOC_SHUTDOWN _IOR ('X', 125, __u32) > > @@ -753,15 +753,6 @@ enum { > EXT4_IOC_CHECKPOINT_FLAG_ZEROOUT | \ > EXT4_IOC_CHECKPOINT_FLAG_DRY_RUN) > > -/* > - * Structure for EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID > - */ > -struct fsuuid { > - __u32 fsu_len; > - __u32 fsu_flags; > - __u8 fsu_uuid[]; > -}; > - > #if defined(__KERNEL__) && defined(CONFIG_COMPAT) > /* > * ioctl commands in 32 bit emulation > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > index b7b56871029c..63b925444592 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -121,6 +121,15 @@ struct fsxattr { > unsigned char fsx_pad[8]; > }; > > +/* > + * Structure for FS_IOC_GETFSUUID/FS_IOC_SETFSUUID > + */ > +struct fsuuid { > + __u32 fsu_len; > + __u32 fsu_flags; > + __u8 fsu_uuid[]; > +}; As I pointed out in my last comments, flex arrays in user APIs are really unfriendly, because it means the structures have to be dynamically allocated and can't be put on the stack. This makes the obvious use of the API (i.e. a local stack struct fsuuid declaration) dangerous to users. Also, UUIDs are 16 bytes long - always have been, always will be. So why does this API need to support -variable length UUIDs-? If this is intended for use with other types of filesystem IDs (e.g. GUIDs), then it needs to be named differently and it needs to have a man page written for it to explain what it contains.... Shouldn't these landmines get fixed before we promote the API to being a VFS-wide operation? Also, if this is VFS wide, then why do we need filesystem specific implementations to retreive the UUID? After all, the VFS superblock has the public filesystem UUID in it (i.e. sb->s_uuid), and so we should just have a single ioctl implementation that reads out the sb->s_uuid. Yes, Setting the UUID is a different matter altogether because filesystems need to change on-disk stuff, but we don't need to reimplement retreiving sb->s_uuid in every filesystem... Cheers, Dave.
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 8d5453852f98..b200302a3732 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -722,8 +722,8 @@ enum { #define EXT4_IOC_GETSTATE _IOW('f', 41, __u32) #define EXT4_IOC_GET_ES_CACHE _IOWR('f', 42, struct fiemap) #define EXT4_IOC_CHECKPOINT _IOW('f', 43, __u32) -#define EXT4_IOC_GETFSUUID _IOR('f', 44, struct fsuuid) -#define EXT4_IOC_SETFSUUID _IOW('f', 44, struct fsuuid) +#define EXT4_IOC_GETFSUUID FS_IOC_GETFSUUID +#define EXT4_IOC_SETFSUUID FS_IOC_SETFSUUID #define EXT4_IOC_SHUTDOWN _IOR ('X', 125, __u32) @@ -753,15 +753,6 @@ enum { EXT4_IOC_CHECKPOINT_FLAG_ZEROOUT | \ EXT4_IOC_CHECKPOINT_FLAG_DRY_RUN) -/* - * Structure for EXT4_IOC_GETFSUUID/EXT4_IOC_SETFSUUID - */ -struct fsuuid { - __u32 fsu_len; - __u32 fsu_flags; - __u8 fsu_uuid[]; -}; - #if defined(__KERNEL__) && defined(CONFIG_COMPAT) /* * ioctl commands in 32 bit emulation diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index b7b56871029c..63b925444592 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -121,6 +121,15 @@ struct fsxattr { unsigned char fsx_pad[8]; }; +/* + * Structure for FS_IOC_GETFSUUID/FS_IOC_SETFSUUID + */ +struct fsuuid { + __u32 fsu_len; + __u32 fsu_flags; + __u8 fsu_uuid[]; +}; + /* * Flags for the fsx_xflags field */ @@ -215,6 +224,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('f', 44, struct fsuuid) +#define FS_IOC_SETFSUUID _IOW('f', 44, struct fsuuid) /* * Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)