xfs_io: enable extsize and stat -v support for ext4
cover xfs_io: enable extsize and stat -v support for ext4
With ext4 extsize hints support being worked on, enable extsize command to be run on FSes other than xfs so ext4 can utilize it. Also extend stat -v to perform FS_IOC_FSGETXATTR ioctl on ext4.
No funtional changes are intended for XFS.
Ojaswin Mujoo (3):
include/linux.h: Factor out generic platform_test_fs_fd() helper
xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
xfs_io: add extsize command support
include/linux.h | 25 +++++++++++++++++-------- io/open.c | 2 +- io/stat.c | 38 +++++++++++++++++++++----------------- 3 files changed, 39 insertions(+), 26 deletions(-)
Signature · 1 line
2.43.5
RFC 1/3 include/linux.h: Factor out generic platform_test_fs_fd() helper
Factor our the generic code to detect the FS type out of platform_test_fs_fd(). This can then be used to detect different file systems types based on magic number.
Also, add a helper to detect if the fd is from an ext4 filesystem.
--- include/linux.h | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
include/linux.h +17 -8
@@ -43,13 +43,7 @@ static __inline__ int xfsctl(const char *path, int fd, int cmd, void *p)return ioctl (fd, cmd, p); } -/* -* platform_test_xfs_*() implies that xfsctl will succeed on the file; -* on Linux, at least, special files don't get xfs file ops, -* so return 0 for those -*/ - -static __inline__int platform_test_xfs_fd (int fd) +static __inline__int platform_test_fs_fd (int fd,long type) {struct statfs statfsbuf;struct stat statbuf;
@@ -60,7 +54,22 @@ static __inline__ int platform_test_xfs_fd(int fd)return 0 ;if (!S_ISREG (statbuf.st_mode) && !S_ISDIR (statbuf.st_mode))return 0 ; -return (statfsbuf.f_type ==0x58465342 );/* XFSB */ +return (statfsbuf.f_type == type); +} + +/* +* platform_test_xfs_*() implies that xfsctl will succeed on the file; +* on Linux, at least, special files don't get xfs file ops, +* so return 0 for those +*/ +static __inline__int platform_test_xfs_fd (int fd) +{ +return platform_test_fs_fd (fd,0x58465342 );/* XFSB */ +} + +static __inline__int platform_test_ext4_fd (int fd) +{ +return platform_test_fs_fd (fd,0xef53 );/* EXT4 magic number */ }static __inline__int platform_test_xfs_path (const char *path)
Signature · 1 line
2.43.5
RFC 2/3 xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Ojaswin MujooCurrently with stat we only show FS_IOC_FSGETXATTR details if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.
No functional changes for filesystems other than ext4.
--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
io/stat.c +21 -17
@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) { -struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd); -if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {perror ("FS_IOC_FSGETXATTR" ); }else {printf (_ ("fsxattr.xflags = 0x%x " ), fsx.fsx_xflags);
@@ -113,14 +113,18 @@ print_xfs_info(int verbose)printf (_ ("fsxattr.extsize = %u\n" ), fsx.fsx_extsize);printf (_ ("fsxattr.cowextsize = %u\n" ), fsx.fsx_cowextsize);printf (_ ("fsxattr.nextents = %u\n" ), fsx.fsx_nextents); -printf (_ ("fsxattr.naextents = %u\n" ), fsxa.fsx_nextents); +if (is_xfs_fd) +printf (_ ("fsxattr.naextents = %u\n" ), fsxa.fsx_nextents); } -if ((xfsctl (file->name, file->fd,XFS_IOC_DIOINFO , &dio)) <0 ) { -perror ("XFS_IOC_DIOINFO" ); - }else { -printf (_ ("dioattr.mem = 0x%x\n" ), dio.d_mem); -printf (_ ("dioattr.miniosz = %u\n" ), dio.d_miniosz); -printf (_ ("dioattr.maxiosz = %u\n" ), dio.d_maxiosz); + +if (is_xfs_fd) { +if ((xfsctl (file->name, file->fd,XFS_IOC_DIOINFO , &dio)) <0 ) { +perror ("XFS_IOC_DIOINFO" ); + }else { +printf (_ ("dioattr.mem = 0x%x\n" ), dio.d_mem); +printf (_ ("dioattr.miniosz = %u\n" ), dio.d_miniosz); +printf (_ ("dioattr.maxiosz = %u\n" ), dio.d_maxiosz); + } } }
@@ -167,10 +171,10 @@ stat_f(printf (_ ("stat.ctime = %s" ),ctime (&st.st_ctime)); } -if (file->flags &IO_FOREIGN ) +if (file->flags &IO_FOREIGN && !platform_test_ext4_fd (file->fd))return 0 ; -print_xfs_info (verbose); +print_extended_info (verbose);return 0 ; }
@@ -440,10 +444,10 @@ statx_f( ctime((time_t *)&stx.stx_btime.tv_sec)); } -if (file->flags &IO_FOREIGN ) +if (file->flags &IO_FOREIGN && !platform_test_ext4_fd (file->fd))return 0 ; -print_xfs_info (verbose); +print_extended_info (verbose);return 0 ; }
Signature · 1 line
2.43.5
RFC 3/3 xfs_io: add extsize command support
In reply to Ojaswin Mujooextsize command is currently only supported with XFS filesystem. Lift this restriction now that ext4 is also supporting extsize hints.
--- io/open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
io/open.c +1 -1
@@ -997,7 +997,7 @@ open_init(void) extsize_cmd.args =_ ("[-D | -R] [extsize]" ); extsize_cmd.argmin =0 ; extsize_cmd.argmax =-1 ; - extsize_cmd.flags =CMD_NOMAP_OK ; + extsize_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; extsize_cmd.oneline =_ ("get/set preferred extent size (in bytes) for the open file" ); extsize_cmd.help = extsize_help;
Signature · 1 line
2.43.5
Re: [RFC 1/3] include/linux.h: Factor out generic platform_test_fs_fd() helper
In reply to Ojaswin MujooQuoted 41 lines · Ojaswin Mujoo
Factor our the generic code to detect the FS type out of
platform_test_fs_fd(). This can then be used to detect different file systems types based on magic number.Also, add a helper to detect if the fd is from an ext4 filesystem.
Signed-off-by Ojaswin Mujoo <[email protected]>--- include/linux.h | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)include/linux.h +7
-8@@ -43,13 +43,7 @@ static __inline__ int xfsctl(const char *path, int fd, int cmd, void *p)return ioctl (fd, cmd, p); } -/* -* platform_test_xfs_*() implies that xfsctl will succeed on the file; -* on Linux, at least, special files don't get xfs file ops, -* so return 0 for those -*/ - -static __inline__int platform_test_xfs_fd (int fd) +static __inline__int platform_test_fs_fd (int fd,long type) {struct statfs statfsbuf;struct stat statbuf;@@ -60,7 +54,22 @@ static __inline__ int platform_test_xfs_fd(int fd)return 0 ;if (!S_ISREG (statbuf.st_mode) && !S_ISDIR (statbuf.st_mode))return 0 ; -return (statfsbuf.f_type ==0x58465342 );/* XFSB */ +return (statfsbuf.f_type == type); +} + +/* +* platform_test_xfs_*() implies that xfsctl will succeed on the file; +* on Linux, at least, special files don't get xfs file ops,
include/linux.h static __inline__ int platform_test_xfs_fd(int fd) · line 72
Quoted 10 lines
include/linux.h +10
-0+* so return 0 for those +*/ +static __inline__int platform_test_xfs_fd (int fd) +{ +return platform_test_fs_fd (fd,0x58465342 );/* XFSB */ +} + +static __inline__int platform_test_ext4_fd (int fd) +{ +return platform_test_fs_fd (fd,0xef53 );/* EXT4 magic number */
Should this be pulling EXT4_SUPER_MAGIC from linux/magic.h?
--D
Quoted 7 lines
include/linux.h +0
-0}static __inline__int platform_test_xfs_path (const char *path)Signature · 1 line
2.43.5
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Ojaswin MujooQuoted 24 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +1
-2@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) {
io/stat.c print_file_info(void) · line 107
Quoted 10 lines
io/stat.c +5
-4-struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd); -if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {
Urgh... perhaps we should call FS_IOC_FSGETXATTR and if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?
e.g.
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) {
if (is_xfs_fd || (errno != EOPNOTSUPP &&
errno != ENOTTY))
perror("FS_IOC_GETXATTR");
} else {
printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
...
}
if (ioctl(file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
if (is_xfs_fd || (errno != EOPNOTSUPP &&
errno != ENOTTY))
perror("XFS_IOC_FSGETXATTRA");
} else {
printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
}That way we don't have to specialcase platform_test_*_fd() for every other filesystem that might want to return real fsxattr results? Same idea for DIOINFO.
--D
Quoted 59 lines
io/stat.c +15
-11perror ("FS_IOC_FSGETXATTR" ); }else {printf (_ ("fsxattr.xflags = 0x%x " ), fsx.fsx_xflags);@@ -113,14 +113,18 @@ print_xfs_info(int verbose)printf (_ ("fsxattr.extsize = %u\n" ), fsx.fsx_extsize);printf (_ ("fsxattr.cowextsize = %u\n" ), fsx.fsx_cowextsize);printf (_ ("fsxattr.nextents = %u\n" ), fsx.fsx_nextents); -printf (_ ("fsxattr.naextents = %u\n" ), fsxa.fsx_nextents); +if (is_xfs_fd) +printf (_ ("fsxattr.naextents = %u\n" ), fsxa.fsx_nextents); } -if ((xfsctl (file->name, file->fd,XFS_IOC_DIOINFO , &dio)) <0 ) { -perror ("XFS_IOC_DIOINFO" ); - }else { -printf (_ ("dioattr.mem = 0x%x\n" ), dio.d_mem); -printf (_ ("dioattr.miniosz = %u\n" ), dio.d_miniosz); -printf (_ ("dioattr.maxiosz = %u\n" ), dio.d_maxiosz); + +if (is_xfs_fd) { +if ((xfsctl (file->name, file->fd,XFS_IOC_DIOINFO , &dio)) <0 ) { +perror ("XFS_IOC_DIOINFO" ); + }else { +printf (_ ("dioattr.mem = 0x%x\n" ), dio.d_mem); +printf (_ ("dioattr.miniosz = %u\n" ), dio.d_miniosz); +printf (_ ("dioattr.maxiosz = %u\n" ), dio.d_maxiosz); + } } }@@ -167,10 +171,10 @@ stat_f(printf (_ ("stat.ctime = %s" ),ctime (&st.st_ctime)); } -if (file->flags &IO_FOREIGN ) +if (file->flags &IO_FOREIGN && !platform_test_ext4_fd (file->fd))return 0 ; -print_xfs_info (verbose); +print_extended_info (verbose);return 0 ; }@@ -440,10 +444,10 @@ statx_f( ctime((time_t *)&stx.stx_btime.tv_sec)); } -if (file->flags &IO_FOREIGN ) +if (file->flags &IO_FOREIGN && !platform_test_ext4_fd (file->fd))return 0 ; -print_xfs_info (verbose); +print_extended_info (verbose);return 0 ; }Signature · 1 line
2.43.5
Re: [RFC 3/3] xfs_io: add extsize command support
In reply to Ojaswin MujooQuoted 4 lines · Ojaswin Mujoo
extsize command is currently only supported with XFS filesystem. Lift this restriction now that ext4 is also supporting extsize hints.
Signed-off-by Ojaswin Mujoo <[email protected]>
Seems pretty straightforward to me. Are you planning to add an extsize option to chattr?
--D
Quoted 21 lines
--- io/open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)io/open.c +1
-1@@ -997,7 +997,7 @@ open_init(void) extsize_cmd.args =_ ("[-D | -R] [extsize]" ); extsize_cmd.argmin =0 ; extsize_cmd.argmax =-1 ; - extsize_cmd.flags =CMD_NOMAP_OK ; + extsize_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; extsize_cmd.oneline =_ ("get/set preferred extent size (in bytes) for the open file" ); extsize_cmd.help = extsize_help;Signature · 1 line
2.43.5
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Darrick J. WongQuoted 31 lines · Darrick J. Wong
Quoted 30 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +4
-4@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) { -struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd);
Quoted 8 lines
io/stat.c print_file_info(void) · line 107
Quoted 4 lines
io/stat.c +2
-2-if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {Urgh... perhaps we should call
FS_IOC_FSGETXATTRand if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?
Yes, please. FS_IOC_FSGETXATTR has been generic functionality for some time, we should treat it the same way for all filesystems.
Quoted 6 lines
e.g.
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("FS_IOC_GETXATTR");
Why do we even need "is_xfs_fd" there? XFS will never give a EOPNOTSUPP or ENOTTY error to this or the FS_IOC_GETXATTRA ioctl...
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Darrick J. WongQuoted 59 lines · Darrick J. Wong
Quoted 24 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +1
-2@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) {io/stat.c print_file_info(void) · line 107
Quoted 10 lines
io/stat.c +5
-4-struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd); -if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {Urgh... perhaps we should call
FS_IOC_FSGETXATTRand if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?e.g.
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("FS_IOC_GETXATTR"); } else { printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags); ... } if (ioctl(file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("XFS_IOC_FSGETXATTRA"); } else { printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents); }
Quoted 3 lines
That way we don't have to specialcase platform_test_*_fd() for every other filesystem that might want to return real fsxattr results? Same idea for DIOINFO.
Hi Darrick, thanks for the review.
I agree that this looks like a more modular approach, I'll make the change. IIUC we basically want to perform the ioctls regardless of the FS and then handle the error/output accordingly here so that we wont need the
if (file->flags & IO_FOREIGN && !platform_test_ext4_fd(file->fd))
return 0;line in the stat functions, right?
Also, with the suggested approach the user visible behavior might change subtly because earlier we used to fail if either FSGETXATTR or FGGETXATTRA failed but now theres a slim chance that we might print the output partially. It might not be a very big deal but just thought I'd point out.
Regards,
ojaswin
Quoted 62 lines
--D
Quoted 59 lines
io/stat.c +15
-11perror ("FS_IOC_FSGETXATTR" ); }else {printf (_ ("fsxattr.xflags = 0x%x " ), fsx.fsx_xflags);@@ -113,14 +113,18 @@ print_xfs_info(int verbose)printf (_ ("fsxattr.extsize = %u\n" ), fsx.fsx_extsize);printf (_ ("fsxattr.cowextsize = %u\n" ), fsx.fsx_cowextsize);printf (_ ("fsxattr.nextents = %u\n" ), fsx.fsx_nextents); -printf (_ ("fsxattr.naextents = %u\n" ), fsxa.fsx_nextents); +if (is_xfs_fd) +printf (_ ("fsxattr.naextents = %u\n" ), fsxa.fsx_nextents); } -if ((xfsctl (file->name, file->fd,XFS_IOC_DIOINFO , &dio)) <0 ) { -perror ("XFS_IOC_DIOINFO" ); - }else { -printf (_ ("dioattr.mem = 0x%x\n" ), dio.d_mem); -printf (_ ("dioattr.miniosz = %u\n" ), dio.d_miniosz); -printf (_ ("dioattr.maxiosz = %u\n" ), dio.d_maxiosz); + +if (is_xfs_fd) { +if ((xfsctl (file->name, file->fd,XFS_IOC_DIOINFO , &dio)) <0 ) { +perror ("XFS_IOC_DIOINFO" ); + }else { +printf (_ ("dioattr.mem = 0x%x\n" ), dio.d_mem); +printf (_ ("dioattr.miniosz = %u\n" ), dio.d_miniosz); +printf (_ ("dioattr.maxiosz = %u\n" ), dio.d_maxiosz); + } } }@@ -167,10 +171,10 @@ stat_f(printf (_ ("stat.ctime = %s" ),ctime (&st.st_ctime)); } -if (file->flags &IO_FOREIGN ) +if (file->flags &IO_FOREIGN && !platform_test_ext4_fd (file->fd))return 0 ; -print_xfs_info (verbose); +print_extended_info (verbose);return 0 ; }@@ -440,10 +444,10 @@ statx_f( ctime((time_t *)&stx.stx_btime.tv_sec)); } -if (file->flags &IO_FOREIGN ) +if (file->flags &IO_FOREIGN && !platform_test_ext4_fd (file->fd))return 0 ; -print_xfs_info (verbose); +print_extended_info (verbose);return 0 ; }Signature · 1 line
2.43.5
Re: [RFC 3/3] xfs_io: add extsize command support
In reply to Darrick J. WongQuoted 8 lines · Darrick J. Wong
Quoted 4 lines · Ojaswin Mujoo
extsize command is currently only supported with XFS filesystem. Lift this restriction now that ext4 is also supporting extsize hints.
Signed-off-by Ojaswin Mujoo <[email protected]>Seems pretty straightforward to me. Are you planning to add an extsize option to chattr?
Do you mean e2fsprogs? If so, then yes we'll add it there eventually however for now I only have xfs_io patches since I was working on them to make the extsize xfstests work with ext4.
Regards,
ojaswin
Quoted 26 lines
Reviewed-by "Darrick J. Wong" <[email protected]>--D
Quoted 21 lines
--- io/open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)io/open.c +1
-1@@ -997,7 +997,7 @@ open_init(void) extsize_cmd.args =_ ("[-D | -R] [extsize]" ); extsize_cmd.argmin =0 ; extsize_cmd.argmax =-1 ; - extsize_cmd.flags =CMD_NOMAP_OK ; + extsize_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; extsize_cmd.oneline =_ ("get/set preferred extent size (in bytes) for the open file" ); extsize_cmd.help = extsize_help;Signature · 1 line
2.43.5
Re: [RFC 1/3] include/linux.h: Factor out generic platform_test_fs_fd() helper
In reply to Darrick J. WongQuoted 46 lines · Darrick J. Wong
Quoted 45 lines · Ojaswin Mujoo
Factor our the generic code to detect the FS type out of
platform_test_fs_fd(). This can then be used to detect different file systems types based on magic number.Also, add a helper to detect if the fd is from an ext4 filesystem.
Signed-off-by Ojaswin Mujoo <[email protected]>--- include/linux.h | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)include/linux.h +11
-8@@ -43,13 +43,7 @@ static __inline__ int xfsctl(const char *path, int fd, int cmd, void *p)return ioctl (fd, cmd, p); } -/* -* platform_test_xfs_*() implies that xfsctl will succeed on the file; -* on Linux, at least, special files don't get xfs file ops, -* so return 0 for those -*/ - -static __inline__int platform_test_xfs_fd (int fd) +static __inline__int platform_test_fs_fd (int fd,long type) {struct statfs statfsbuf;struct stat statbuf;@@ -60,7 +54,22 @@ static __inline__ int platform_test_xfs_fd(int fd)return 0 ;if (!S_ISREG (statbuf.st_mode) && !S_ISDIR (statbuf.st_mode))return 0 ; -return (statfsbuf.f_type ==0x58465342 );/* XFSB */ +return (statfsbuf.f_type == type); +} + +/* +* platform_test_xfs_*() implies that xfsctl will succeed on the file; +* on Linux, at least, special files don't get xfs file ops, +* so return 0 for those +*/ +static __inline__int platform_test_xfs_fd (int fd) +{
Quoted 8 lines
include/linux.h static __inline__ int platform_test_xfs_fd(int fd) · line 72
Quoted 6 lines
include/linux.h +6
-0+return platform_test_fs_fd (fd,0x58465342 );/* XFSB */ +} + +static __inline__int platform_test_ext4_fd (int fd) +{ +return platform_test_fs_fd (fd,0xef53 );/* EXT4 magic number */ Should this be pulling
EXT4_SUPER_MAGICfromlinux/magic.h?
Oh right, thanks for pointing out. I'll use that for the magic numbers.
Thanks,
ojaswin
Quoted 10 lines
--D
Quoted 7 lines
include/linux.h +0
-0}static __inline__int platform_test_xfs_path (const char *path)Signature · 1 line
2.43.5
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Dave ChinnerQuoted 46 lines · Dave Chinner
Quoted 31 lines · Darrick J. Wong
Quoted 30 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +4
-4@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) { -struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd);Quoted 8 lines
io/stat.c print_file_info(void) · line 107
Quoted 4 lines
io/stat.c +2
-2-if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {Urgh... perhaps we should call
FS_IOC_FSGETXATTRand if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?Yes, please.
FS_IOC_FSGETXATTRhas been generic functionality for some time, we should treat it the same way for all filesystems.Quoted 2 lines
e.g.
Quoted 7 lines
Quoted 4 lines
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("FS_IOC_GETXATTR");Why do we even need "
is_xfs_fd" there? XFS will never give a EOPNOTSUPP or ENOTTY error to this or theFS_IOC_GETXATTRAioctl...
Yeah, in hindsight I don't think it's needed for FS_IOC_FSGETXATTR, but it's definitely nice for XFS_IOC_FSGETXATTRA (which is not implemented outside xfs) so that you don't get unnecessary error messages on ext4.
--D
Quoted 5 lines
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [RFC 3/3] xfs_io: add extsize command support
In reply to Ojaswin MujooQuoted 5 lines · Ojaswin Mujoo
Quoted 4 lines · Darrick J. Wong
Quoted 3 lines · Ojaswin Mujoo
extsize command is currently only supported with XFS filesystem. Lift this restriction now that ext4 is also supporting extsize hints.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Ojaswin Mujoo <[email protected]>Seems pretty straightforward to me. Are you planning to add an extsize option to chattr?
Do you mean e2fsprogs? If so, then yes we'll add it there eventually however for now I only have
xfs_iopatches since I was working on them to make the extsize xfstests work with ext4.
Yep, and good to know (about adding chattr support eventually).
--D
Quoted 29 lines
Regards,
ojaswinQuoted 26 lines
Reviewed-by "Darrick J. Wong" <[email protected]>--D
Quoted 21 lines
--- io/open.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)io/open.c +1
-1@@ -997,7 +997,7 @@ open_init(void) extsize_cmd.args =_ ("[-D | -R] [extsize]" ); extsize_cmd.argmin =0 ; extsize_cmd.argmax =-1 ; - extsize_cmd.flags =CMD_NOMAP_OK ; + extsize_cmd.flags =CMD_NOMAP_OK |CMD_FOREIGN_OK ; extsize_cmd.oneline =_ ("get/set preferred extent size (in bytes) for the open file" ); extsize_cmd.help = extsize_help;Signature · 1 line
2.43.5
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Darrick J. WongQuoted 52 lines · Darrick J. Wong
Quoted 51 lines · Dave Chinner
Quoted 31 lines · Darrick J. Wong
Quoted 30 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +4
-4@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) { -struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd);Quoted 8 lines
io/stat.c print_file_info(void) · line 107
Quoted 4 lines
io/stat.c +2
-2-if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {Urgh... perhaps we should call
FS_IOC_FSGETXATTRand if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?Yes, please.
FS_IOC_FSGETXATTRhas been generic functionality for some time, we should treat it the same way for all filesystems.Quoted 6 lines
e.g.
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("FS_IOC_GETXATTR");
Quoted 4 lines
Quoted 2 lines
Why do we even need "
is_xfs_fd" there? XFS will never give a EOPNOTSUPP or ENOTTY error to this or theFS_IOC_GETXATTRAioctl...Yeah, in hindsight I don't think it's needed for
FS_IOC_FSGETXATTR, but
*nod*
Quoted 2 lines
it's definitely nice for
XFS_IOC_FSGETXATTRA(which is not implemented outside xfs) so that you don't get unnecessary error messages on ext4.
I don't think we even need it for FS_IOC_GETXATTRA - if the filesystem does not support that ioctl, we don't print the fields, nor do we output an error.
After all, this "extended info" and it's only ever been printed for XFS, so we can define whatever semantics we want for foreign filesystem output right now. As long as XFS always prints the same info as it always has (i.e. all of it), we can do whatever we want with the foreign filesystem stuff.
Keep in mind that we don't need platform tests for XFS files - that has already been done when the file was opened and the state stored in file->flags via the IO_FOREIGN flag. We already use that in the stat_f() to determine whether we print the "xfs info" or not.
IOWs, I think all we need to do is move where we check the IO_FOREIGN flag. i.e.:
print_extented_info(file)
{
struct dioattr dio = {};
struct fsxattr fsx = {}, fsxa = {};
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) {
perror("FS_IOC_GETXATTR");
exitcode = 1;
return;
}
printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
/* Only XFS supports FS_IOC_FSGETXATTRA and XFS_IOC_DIOINFO */
if (file->flags & IO_FOREIGN)
return;
if (ioctl(file->fd, FS_IOC_FSGETXATTRA, &fsxa)) < 0) {
perror("FS_IOC_GETXATTRA");
exitcode = 1;
return;
}
if ((xfsctl(file->name, file->fd, XFS_IOC_DIOINFO, &dio)) < 0) {
perror("XFS_IOC_DIOINFO");
exitcode = 1;
return;
}
printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
}Thoughts?
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
Quoted 120 lines · Dave Chinner
Quoted 52 lines · Darrick J. Wong
Quoted 51 lines · Dave Chinner
Quoted 31 lines · Darrick J. Wong
Quoted 30 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +4
-4@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) { -struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd);Quoted 8 lines
io/stat.c print_file_info(void) · line 107
Quoted 4 lines
io/stat.c +2
-2-if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {Urgh... perhaps we should call
FS_IOC_FSGETXATTRand if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?Yes, please.
FS_IOC_FSGETXATTRhas been generic functionality for some time, we should treat it the same way for all filesystems.Quoted 6 lines
e.g.
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("FS_IOC_GETXATTR");Quoted 4 lines
Quoted 2 lines
Why do we even need "
is_xfs_fd" there? XFS will never give a EOPNOTSUPP or ENOTTY error to this or theFS_IOC_GETXATTRAioctl...Yeah, in hindsight I don't think it's needed for
FS_IOC_FSGETXATTR, but*nod*
Quoted 2 lines
it's definitely nice for
XFS_IOC_FSGETXATTRA(which is not implemented outside xfs) so that you don't get unnecessary error messages on ext4.I don't think we even need it for
FS_IOC_GETXATTRA- if the filesystem does not support that ioctl, we don't print the fields, nor do we output an error.After all, this "extended info" and it's only ever been printed for XFS, so we can define whatever semantics we want for foreign filesystem output right now. As long as XFS always prints the same info as it always has (i.e. all of it), we can do whatever we want with the foreign filesystem stuff.
Keep in mind that we don't need platform tests for XFS files - that has already been done when the file was opened and the state stored in
file->flagsvia theIO_FOREIGNflag. We already use that in thestat_f()to determine whether we print the "xfs info" or not.IOWs, I think all we need to do is move where we check the
IO_FOREIGNflag. i.e.:
print_extented_info(file){ struct dioattr dio = {}; struct fsxattr fsx = {}, fsxa = {}; if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { perror("FS_IOC_GETXATTR"); exitcode = 1; return; } printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags); printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1); printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid); printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize); printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize); printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents); /* Only XFS supports FS_IOC_FSGETXATTRA and XFS_IOC_DIOINFO */ if (file->flags & IO_FOREIGN) return; if (ioctl(file->fd, FS_IOC_FSGETXATTRA, &fsxa)) < 0) { perror("FS_IOC_GETXATTRA"); exitcode = 1; return; } if ((xfsctl(file->name, file->fd, XFS_IOC_DIOINFO, &dio)) < 0) { perror("XFS_IOC_DIOINFO"); exitcode = 1; return; } printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents); printf(_("dioattr.mem = 0x%x\n"), dio.d_mem); printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz); printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz); }
Quoted 1 line
Thoughts?
Seems fine to me, though I'd print the fsxa before trying to call DIOINFO.
--D
Quoted 5 lines
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [RFC 1/3] include/linux.h: Factor out generic platform_test_fs_fd() helper
In reply to Darrick J. WongQuoted 6 lines · Darrick J. Wong
include/linux.h static __inline__ int platform_test_xfs_fd(int fd) · line 72
Quoted 4 lines
include/linux.h +4
-0+ +static __inline__int platform_test_ext4_fd (int fd) +{ +return platform_test_fs_fd (fd,0xef53 );/* EXT4 magic number */ Should this be pulling
EXT4_SUPER_MAGICfromlinux/magic.h?
I think we can just drop adding platform_test_ext4_fd with the suggested patches later in the series? Having ext4-specific code in xfsprogs would seem pretty odd in xfsprogs anyway over our previous categories of xfs only or generic.
Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details
In reply to Darrick J. WongQuoted 121 lines · Darrick J. Wong
Quoted 120 lines · Dave Chinner
Quoted 52 lines · Darrick J. Wong
Quoted 51 lines · Dave Chinner
Quoted 31 lines · Darrick J. Wong
Quoted 30 lines · Ojaswin Mujoo
Currently with stat we only show
FS_IOC_FSGETXATTRdetails if the filesystem is XFS. With extsize support also coming to ext4 make sure to show these details when -c "stat" or "statx" is used.No functional changes for filesystems other than ext4.
Signed-off-by Ojaswin Mujoo <[email protected]>--- io/stat.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)io/stat.c +4
-4@@ -97,14 +97,14 @@ print_file_info(void) file->flags & IO_TMPFILE ? _(",tmpfile") : ""); } -static void -print_xfs_info (int verbose) +static void print_extended_info (int verbose) { -struct dioattr dio; -struct fsxattr fsx, fsxa; +struct dioattr dio; +struct fsxattr fsx, fsxa; +bool is_xfs_fd =platform_test_xfs_fd (file->fd);Quoted 8 lines
io/stat.c print_file_info(void) · line 107
Quoted 4 lines
io/stat.c +2
-2-if ((xfsctl (file->name, file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || - (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa)) <0 ) { +if ((ioctl (file->fd,FS_IOC_FSGETXATTR , &fsx)) <0 || + (is_xfs_fd && (xfsctl (file->name, file->fd,XFS_IOC_FSGETXATTRA , &fsxa) <0 ))) {Urgh... perhaps we should call
FS_IOC_FSGETXATTRand if it returns zero print whatever is returned, no matter what filesystem we think is feeding us information?Yes, please.
FS_IOC_FSGETXATTRhas been generic functionality for some time, we should treat it the same way for all filesystems.Quoted 6 lines
e.g.
if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { if (is_xfs_fd || (errno != EOPNOTSUPP && errno != ENOTTY)) perror("FS_IOC_GETXATTR");Quoted 4 lines
Quoted 2 lines
Why do we even need "
is_xfs_fd" there? XFS will never give a EOPNOTSUPP or ENOTTY error to this or theFS_IOC_GETXATTRAioctl...Yeah, in hindsight I don't think it's needed for
FS_IOC_FSGETXATTR, but*nod*
Quoted 2 lines
it's definitely nice for
XFS_IOC_FSGETXATTRA(which is not implemented outside xfs) so that you don't get unnecessary error messages on ext4.I don't think we even need it for
FS_IOC_GETXATTRA- if the filesystem does not support that ioctl, we don't print the fields, nor do we output an error.After all, this "extended info" and it's only ever been printed for XFS, so we can define whatever semantics we want for foreign filesystem output right now. As long as XFS always prints the same info as it always has (i.e. all of it), we can do whatever we want with the foreign filesystem stuff.
Keep in mind that we don't need platform tests for XFS files - that has already been done when the file was opened and the state stored in
file->flagsvia theIO_FOREIGNflag. We already use that in thestat_f()to determine whether we print the "xfs info" or not.IOWs, I think all we need to do is move where we check the
IO_FOREIGNflag. i.e.:
print_extented_info(file){ struct dioattr dio = {}; struct fsxattr fsx = {}, fsxa = {}; if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { perror("FS_IOC_GETXATTR"); exitcode = 1; return; } printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags); printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1); printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid); printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize); printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize); printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents); /* Only XFS supports FS_IOC_FSGETXATTRA and XFS_IOC_DIOINFO */ if (file->flags & IO_FOREIGN) return; if (ioctl(file->fd, FS_IOC_FSGETXATTRA, &fsxa)) < 0) { perror("FS_IOC_GETXATTRA"); exitcode = 1; return; } if ((xfsctl(file->name, file->fd, XFS_IOC_DIOINFO, &dio)) < 0) { perror("XFS_IOC_DIOINFO"); exitcode = 1; return; } printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents); printf(_("dioattr.mem = 0x%x\n"), dio.d_mem); printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz); printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz); }
Quoted 6 lines
Quoted 1 line
Thoughts?
Seems fine to me, though I'd print the fsxa before trying to call DIOINFO.
--D
Got it, this makes sense to me as well. I'll do something like this in v2. Thanks!
Regards,
ojaswin
Quoted 6 lines
Quoted 6 lines
-Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [RFC 1/3] include/linux.h: Factor out generic platform_test_fs_fd() helper
In reply to Christoph HellwigQuoted 8 lines · Christoph Hellwig
Quoted 6 lines · Darrick J. Wong
include/linux.h static __inline__ int platform_test_xfs_fd(int fd) · line 72
Quoted 4 lines
include/linux.h +4
-0+ +static __inline__int platform_test_ext4_fd (int fd) +{ +return platform_test_fs_fd (fd,0xef53 );/* EXT4 magic number */ Should this be pulling
EXT4_SUPER_MAGICfromlinux/magic.h?
Quoted 5 lines
I think we can just drop adding
platform_test_ext4_fdwith the suggested patches later in the series? Having ext4-specific code in xfsprogs would seem pretty odd in xfsprogs anyway over our previous categories of xfs only or generic.
Oh right, xfs_io is so widely used across FSes that I missed to consider it might not be okay to add special code specific to ext4 :)
Anyways, I agree we don't need to separately handle ext4 anymore. I'll drop this patch. (Maybe still get XFS_SUPER_MAGIC from linux/magic.h as that seems like the right thing to do)
Regards,
ojaswin