* [PATCH v1 0/2] porting the GETFSUUID ioctl to xfs @ 2022-11-09 22:19 Catherine Hoang 2022-11-09 22:19 ` [PATCH v1 1/2] fs: hoist get/set UUID ioctls Catherine Hoang 2022-11-09 22:19 ` [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl Catherine Hoang 0 siblings, 2 replies; 9+ messages in thread From: Catherine Hoang @ 2022-11-09 22:19 UTC (permalink / raw) To: linux-xfs; +Cc: tytso Hi all, This patch aims to hoist the ext4 get/set ioctls to the VFS so we have a common interface for tools such as coreutils. The second patch adds support for FS_IOC_GETFSUUID in xfs (with FS_IOC_SETFSUUID planned for future patches). In addition, the new xfs_spaceman fsuuid command uses this ioctl to retrieve the UUID of a mounted filesystem. Comments and feedback appreciated! Catherine Catherine Hoang (2): fs: hoist get/set UUID ioctls xfs: add FS_IOC_GETFSUUID ioctl fs/ext4/ext4.h | 13 ++----------- fs/xfs/xfs_ioctl.c | 32 ++++++++++++++++++++++++++++++++ include/uapi/linux/fs.h | 11 +++++++++++ 3 files changed, 45 insertions(+), 11 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] fs: hoist get/set UUID ioctls 2022-11-09 22:19 [PATCH v1 0/2] porting the GETFSUUID ioctl to xfs Catherine Hoang @ 2022-11-09 22:19 ` Catherine Hoang 2022-11-10 20:06 ` Darrick J. Wong 2022-11-11 21:31 ` Allison Henderson 2022-11-09 22:19 ` [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl Catherine Hoang 1 sibling, 2 replies; 9+ messages in thread From: Catherine Hoang @ 2022-11-09 22:19 UTC (permalink / raw) To: linux-xfs; +Cc: tytso 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> --- 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[]; +}; + /* * 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) -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] fs: hoist get/set UUID ioctls 2022-11-09 22:19 ` [PATCH v1 1/2] fs: hoist get/set UUID ioctls Catherine Hoang @ 2022-11-10 20:06 ` Darrick J. Wong 2022-11-11 0:44 ` Catherine Hoang 2022-11-11 21:31 ` Allison Henderson 1 sibling, 1 reply; 9+ messages in thread From: Darrick J. Wong @ 2022-11-10 20:06 UTC (permalink / raw) To: Catherine Hoang; +Cc: linux-xfs, tytso On Wed, Nov 09, 2022 at 02:19:58PM -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> Looks good, Reviewed-by: Darrick J. Wong <djwong@kernel.org> Also, for the inevitable v2 patchset after we sort out some weird bugs in the ext4 implementation of this, can you please cc linux-ext4@vger.kernel.org? --D > --- > 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[]; > +}; > + > /* > * 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) > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] fs: hoist get/set UUID ioctls 2022-11-10 20:06 ` Darrick J. Wong @ 2022-11-11 0:44 ` Catherine Hoang 0 siblings, 0 replies; 9+ messages in thread From: Catherine Hoang @ 2022-11-11 0:44 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, tytso > On Nov 10, 2022, at 12:06 PM, Darrick J. Wong <djwong@kernel.org> wrote: > > On Wed, Nov 09, 2022 at 02:19:58PM -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> > > Looks good, > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > Also, for the inevitable v2 patchset after we sort out some weird bugs > in the ext4 implementation of this, can you please cc > linux-ext4@vger.kernel.org? Thanks for reviewing! And sure, I’ll cc them in the next version > > --D > >> --- >> 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[]; >> +}; >> + >> /* >> * 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) >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] fs: hoist get/set UUID ioctls 2022-11-09 22:19 ` [PATCH v1 1/2] fs: hoist get/set UUID ioctls Catherine Hoang 2022-11-10 20:06 ` Darrick J. Wong @ 2022-11-11 21:31 ` Allison Henderson 1 sibling, 0 replies; 9+ messages in thread From: Allison Henderson @ 2022-11-11 21:31 UTC (permalink / raw) To: Catherine Hoang, linux-xfs; +Cc: tytso On Wed, 2022-11-09 at 14:19 -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> Looks straight forward to me 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_F > LAG_ZEROOUT | \ > EXT4_IOC_CHECKPOINT_F > LAG_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) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl 2022-11-09 22:19 [PATCH v1 0/2] porting the GETFSUUID ioctl to xfs Catherine Hoang 2022-11-09 22:19 ` [PATCH v1 1/2] fs: hoist get/set UUID ioctls Catherine Hoang @ 2022-11-09 22:19 ` Catherine Hoang 2022-11-10 20:05 ` Darrick J. Wong 2022-11-11 21:31 ` Allison Henderson 1 sibling, 2 replies; 9+ messages in thread From: Catherine Hoang @ 2022-11-09 22:19 UTC (permalink / raw) To: linux-xfs; +Cc: tytso Add a new ioctl to retrieve the UUID of a mounted xfs filesystem. Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> --- fs/xfs/xfs_ioctl.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 1f783e979629..657fe058dfba 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -1865,6 +1865,35 @@ xfs_fs_eofblocks_from_user( return 0; } +static int xfs_ioctl_getuuid( + struct xfs_mount *mp, + struct fsuuid __user *ufsuuid) +{ + struct fsuuid fsuuid; + __u8 uuid[UUID_SIZE]; + + if (copy_from_user(&fsuuid, ufsuuid, sizeof(fsuuid))) + return -EFAULT; + + if (fsuuid.fsu_len == 0) { + fsuuid.fsu_len = UUID_SIZE; + if (copy_to_user(ufsuuid, &fsuuid, sizeof(fsuuid.fsu_len))) + return -EFAULT; + return -EINVAL; + } + + if (fsuuid.fsu_len != UUID_SIZE || fsuuid.fsu_flags != 0) + return -EINVAL; + + spin_lock(&mp->m_sb_lock); + memcpy(uuid, &mp->m_sb.sb_uuid, UUID_SIZE); + spin_unlock(&mp->m_sb_lock); + + if (copy_to_user(&ufsuuid->fsu_uuid[0], uuid, UUID_SIZE)) + return -EFAULT; + return 0; +} + /* * These long-unused ioctls were removed from the official ioctl API in 5.17, * but retain these definitions so that we can log warnings about them. @@ -2153,6 +2182,9 @@ xfs_file_ioctl( return error; } + case FS_IOC_GETFSUUID: + return xfs_ioctl_getuuid(mp, arg); + default: return -ENOTTY; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl 2022-11-09 22:19 ` [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl Catherine Hoang @ 2022-11-10 20:05 ` Darrick J. Wong 2022-11-11 0:41 ` Catherine Hoang 2022-11-11 21:31 ` Allison Henderson 1 sibling, 1 reply; 9+ messages in thread From: Darrick J. Wong @ 2022-11-10 20:05 UTC (permalink / raw) To: Catherine Hoang; +Cc: linux-xfs, tytso On Wed, Nov 09, 2022 at 02:19:59PM -0800, Catherine Hoang wrote: > Add a new ioctl to retrieve the UUID of a mounted xfs filesystem. I think it's worth mentioning that this is the precursor to trying to implement SETFSUUID... but that's something for a future series, since changing the uuid will upset the log, and we have to figure out how to deal with that gracefully. > Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> > --- > fs/xfs/xfs_ioctl.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 1f783e979629..657fe058dfba 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -1865,6 +1865,35 @@ xfs_fs_eofblocks_from_user( > return 0; > } > > +static int xfs_ioctl_getuuid( Nit: function names should start on a new line. > + struct xfs_mount *mp, > + struct fsuuid __user *ufsuuid) > +{ > + struct fsuuid fsuuid; > + __u8 uuid[UUID_SIZE]; > + > + if (copy_from_user(&fsuuid, ufsuuid, sizeof(fsuuid))) > + return -EFAULT; > + > + if (fsuuid.fsu_len == 0) { > + fsuuid.fsu_len = UUID_SIZE; > + if (copy_to_user(ufsuuid, &fsuuid, sizeof(fsuuid.fsu_len))) > + return -EFAULT; > + return -EINVAL; Ted and I were looking through the ext4_ioctl_getuuid function on this morning's ext4 concall, and we decided that copying the desired uuid buffer length out to userspace shouldn't result in an EINVAL return here... > + } > + > + if (fsuuid.fsu_len != UUID_SIZE || fsuuid.fsu_flags != 0) ...and that we shouldn't reject the case where fsu_len > UUID_SIZE. Instead, we should copy the uuid and update the caller's fsu_len to reflect however many bytes we copied out. I'll send patches to do that shortly. > + return -EINVAL; > + > + spin_lock(&mp->m_sb_lock); > + memcpy(uuid, &mp->m_sb.sb_uuid, UUID_SIZE); > + spin_unlock(&mp->m_sb_lock); > + > + if (copy_to_user(&ufsuuid->fsu_uuid[0], uuid, UUID_SIZE)) > + return -EFAULT; The rest of this logic looks correct to me. Thanks for getting this out there. --D > + return 0; > +} > + > /* > * These long-unused ioctls were removed from the official ioctl API in 5.17, > * but retain these definitions so that we can log warnings about them. > @@ -2153,6 +2182,9 @@ xfs_file_ioctl( > return error; > } > > + case FS_IOC_GETFSUUID: > + return xfs_ioctl_getuuid(mp, arg); > + > default: > return -ENOTTY; > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl 2022-11-10 20:05 ` Darrick J. Wong @ 2022-11-11 0:41 ` Catherine Hoang 0 siblings, 0 replies; 9+ messages in thread From: Catherine Hoang @ 2022-11-11 0:41 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, tytso > On Nov 10, 2022, at 12:05 PM, Darrick J. Wong <djwong@kernel.org> wrote: > > On Wed, Nov 09, 2022 at 02:19:59PM -0800, Catherine Hoang wrote: >> Add a new ioctl to retrieve the UUID of a mounted xfs filesystem. > > I think it's worth mentioning that this is the precursor to trying to > implement SETFSUUID... but that's something for a future series, since > changing the uuid will upset the log, and we have to figure out how to > deal with that gracefully. > >> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> >> --- >> fs/xfs/xfs_ioctl.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c >> index 1f783e979629..657fe058dfba 100644 >> --- a/fs/xfs/xfs_ioctl.c >> +++ b/fs/xfs/xfs_ioctl.c >> @@ -1865,6 +1865,35 @@ xfs_fs_eofblocks_from_user( >> return 0; >> } >> >> +static int xfs_ioctl_getuuid( > > Nit: function names should start on a new line. > Ok, will fix that >> + struct xfs_mount *mp, >> + struct fsuuid __user *ufsuuid) >> +{ >> + struct fsuuid fsuuid; >> + __u8 uuid[UUID_SIZE]; >> + >> + if (copy_from_user(&fsuuid, ufsuuid, sizeof(fsuuid))) >> + return -EFAULT; >> + >> + if (fsuuid.fsu_len == 0) { >> + fsuuid.fsu_len = UUID_SIZE; >> + if (copy_to_user(ufsuuid, &fsuuid, sizeof(fsuuid.fsu_len))) >> + return -EFAULT; >> + return -EINVAL; > > Ted and I were looking through the ext4_ioctl_getuuid function on this > morning's ext4 concall, and we decided that copying the desired uuid > buffer length out to userspace shouldn't result in an EINVAL return > here... > >> + } >> + >> + if (fsuuid.fsu_len != UUID_SIZE || fsuuid.fsu_flags != 0) > > ...and that we shouldn't reject the case where fsu_len > UUID_SIZE. > Instead, we should copy the uuid and update the caller's fsu_len to > reflect however many bytes we copied out. I'll send patches to do that > shortly. Ok, that makes sense. I’ll apply those changes over to xfs. Thanks! > >> + return -EINVAL; >> + >> + spin_lock(&mp->m_sb_lock); >> + memcpy(uuid, &mp->m_sb.sb_uuid, UUID_SIZE); >> + spin_unlock(&mp->m_sb_lock); >> + >> + if (copy_to_user(&ufsuuid->fsu_uuid[0], uuid, UUID_SIZE)) >> + return -EFAULT; > > The rest of this logic looks correct to me. Thanks for getting this out > there. > > --D > >> + return 0; >> +} >> + >> /* >> * These long-unused ioctls were removed from the official ioctl API in 5.17, >> * but retain these definitions so that we can log warnings about them. >> @@ -2153,6 +2182,9 @@ xfs_file_ioctl( >> return error; >> } >> >> + case FS_IOC_GETFSUUID: >> + return xfs_ioctl_getuuid(mp, arg); >> + >> default: >> return -ENOTTY; >> } >> -- >> 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl 2022-11-09 22:19 ` [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl Catherine Hoang 2022-11-10 20:05 ` Darrick J. Wong @ 2022-11-11 21:31 ` Allison Henderson 1 sibling, 0 replies; 9+ messages in thread From: Allison Henderson @ 2022-11-11 21:31 UTC (permalink / raw) To: Catherine Hoang, linux-xfs; +Cc: tytso On Wed, 2022-11-09 at 14:19 -0800, Catherine Hoang wrote: > Add a new ioctl to retrieve the UUID of a mounted xfs filesystem. > > Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> With Darricks commentary addressed, I think this one looks good Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/xfs/xfs_ioctl.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 1f783e979629..657fe058dfba 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -1865,6 +1865,35 @@ xfs_fs_eofblocks_from_user( > return 0; > } > > +static int xfs_ioctl_getuuid( > + struct xfs_mount *mp, > + struct fsuuid __user *ufsuuid) > +{ > + struct fsuuid fsuuid; > + __u8 uuid[UUID_SIZE]; > + > + if (copy_from_user(&fsuuid, ufsuuid, sizeof(fsuuid))) > + return -EFAULT; > + > + if (fsuuid.fsu_len == 0) { > + fsuuid.fsu_len = UUID_SIZE; > + if (copy_to_user(ufsuuid, &fsuuid, > sizeof(fsuuid.fsu_len))) > + return -EFAULT; > + return -EINVAL; > + } > + > + if (fsuuid.fsu_len != UUID_SIZE || fsuuid.fsu_flags != 0) > + return -EINVAL; > + > + spin_lock(&mp->m_sb_lock); > + memcpy(uuid, &mp->m_sb.sb_uuid, UUID_SIZE); > + spin_unlock(&mp->m_sb_lock); > + > + if (copy_to_user(&ufsuuid->fsu_uuid[0], uuid, UUID_SIZE)) > + return -EFAULT; > + return 0; > +} > + > /* > * These long-unused ioctls were removed from the official ioctl API > in 5.17, > * but retain these definitions so that we can log warnings about > them. > @@ -2153,6 +2182,9 @@ xfs_file_ioctl( > return error; > } > > + case FS_IOC_GETFSUUID: > + return xfs_ioctl_getuuid(mp, arg); > + > default: > return -ENOTTY; > } ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-11 21:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-09 22:19 [PATCH v1 0/2] porting the GETFSUUID ioctl to xfs Catherine Hoang 2022-11-09 22:19 ` [PATCH v1 1/2] fs: hoist get/set UUID ioctls Catherine Hoang 2022-11-10 20:06 ` Darrick J. Wong 2022-11-11 0:44 ` Catherine Hoang 2022-11-11 21:31 ` Allison Henderson 2022-11-09 22:19 ` [PATCH v1 2/2] xfs: add FS_IOC_GETFSUUID ioctl Catherine Hoang 2022-11-10 20:05 ` Darrick J. Wong 2022-11-11 0:41 ` Catherine Hoang 2022-11-11 21:31 ` Allison Henderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.