* Tests for file_getattr()/file_setattr() and xfsprogs update @ 2025-08-08 19:28 Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 0/3] Test file_getattr and file_setattr syscalls Andrey Albershteyn 0 siblings, 2 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:28 UTC (permalink / raw) To: linux-xfs, linux-fsdevel, fstests Hi all, This two patchsets are update to xfsprogs to utilize recently added file_getattr() and file_setattr() syscalls. The second patchset adds two tests to fstests, one generic one on these syscals and second one is for XFS's original usecase for these syscalls (projects quotas). -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() 2025-08-08 19:28 Tests for file_getattr()/file_setattr() and xfsprogs update Andrey Albershteyn @ 2025-08-08 19:30 ` Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls Andrey Albershteyn ` (3 more replies) 2025-08-08 19:31 ` [PATCH 0/3] Test file_getattr and file_setattr syscalls Andrey Albershteyn 1 sibling, 4 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:30 UTC (permalink / raw) To: aalbersh, linux-fsdevel, linux-xfs; +Cc: Andrey Albershteyn Hi all, This patchset updates libfrog, xfs_db, xfs_quota to use recently introduced syscalls file_getattr()/file_setattr(). I haven't replaced all the calls to ioctls() with a syscalls, just a few places where syscalls are necessary. If there's more places it would be suitable to update, let me know. Cc: linux-fsdevel@vger.kernel.org Cc: linux-xfs@vger.kernel.org --- Andrey Albershteyn (4): libfrog: add wrappers for file_getattr/file_setattr syscalls xfs_quota: utilize file_setattr to set prjid on special files xfs_io: make ls/chattr work with special files xfs_db: use file_setattr to copy attributes on special files with rdump configure.ac | 1 + db/rdump.c | 24 +++++++-- include/builddefs.in | 5 ++ include/linux.h | 20 +++++++ io/attr.c | 130 ++++++++++++++++++++++++++------------------- libfrog/Makefile | 2 + libfrog/file_attr.c | 105 ++++++++++++++++++++++++++++++++++++ libfrog/file_attr.h | 35 ++++++++++++ m4/package_libcdev.m4 | 19 +++++++ quota/project.c | 144 ++++++++++++++++++++++++++------------------------ 10 files changed, 357 insertions(+), 128 deletions(-) --- base-commit: d0884c436c82dddbf5f5ef57acfbf784ff7f7832 change-id: 20250317-xattrat-syscall-839fb2bb0c63 Best regards, -- Andrey Albershteyn <aalbersh@kernel.org> ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn @ 2025-08-08 19:30 ` Andrey Albershteyn 2025-08-11 15:02 ` Darrick J. Wong 2025-08-08 19:30 ` [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files Andrey Albershteyn ` (2 subsequent siblings) 3 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:30 UTC (permalink / raw) To: aalbersh, linux-fsdevel, linux-xfs Add wrappers for new file_getattr/file_setattr inode syscalls which will be used by xfs_quota and xfs_io. Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- configure.ac | 1 + include/builddefs.in | 5 +++ include/linux.h | 20 ++++++++++ libfrog/Makefile | 2 + libfrog/file_attr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++ libfrog/file_attr.h | 35 +++++++++++++++++ m4/package_libcdev.m4 | 19 +++++++++ 7 files changed, 187 insertions(+) diff --git a/configure.ac b/configure.ac index 9a3309bcdfd1..40a44c571e7b 100644 --- a/configure.ac +++ b/configure.ac @@ -156,6 +156,7 @@ AC_PACKAGE_NEED_RCU_INIT AC_HAVE_PWRITEV2 AC_HAVE_COPY_FILE_RANGE AC_HAVE_CACHESTAT +AC_HAVE_FILE_ATTR AC_NEED_INTERNAL_FSXATTR AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG AC_NEED_INTERNAL_FSCRYPT_POLICY_V2 diff --git a/include/builddefs.in b/include/builddefs.in index 04b4e0880a84..d727b55b854f 100644 --- a/include/builddefs.in +++ b/include/builddefs.in @@ -97,6 +97,7 @@ HAVE_ZIPPED_MANPAGES = @have_zipped_manpages@ HAVE_PWRITEV2 = @have_pwritev2@ HAVE_COPY_FILE_RANGE = @have_copy_file_range@ HAVE_CACHESTAT = @have_cachestat@ +HAVE_FILE_ATTR = @have_file_attr@ NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@ NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@ NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@ @@ -169,6 +170,10 @@ ifeq ($(ENABLE_GETTEXT),yes) GCFLAGS += -DENABLE_GETTEXT endif +ifeq ($(HAVE_FILE_ATTR),yes) +LCFLAGS += -DHAVE_FILE_ATTR +endif + # Override these if C++ needs other options SANITIZER_CXXFLAGS = $(SANITIZER_CFLAGS) GCXXFLAGS = $(GCFLAGS) diff --git a/include/linux.h b/include/linux.h index 6e83e073aa2e..018cc78960e3 100644 --- a/include/linux.h +++ b/include/linux.h @@ -16,6 +16,7 @@ #include <sys/param.h> #include <sys/sysmacros.h> #include <sys/stat.h> +#include <sys/syscall.h> #include <inttypes.h> #include <malloc.h> #include <getopt.h> @@ -202,6 +203,25 @@ struct fsxattr { }; #endif +/* + * Use __NR_file_getattr instead of build system HAVE_FILE_ATTR as this header + * could be included in other places where HAVE_FILE_ATTR is not defined (e.g. + * xfstests's conftest.c in ./configure) + */ +#ifndef __NR_file_getattr +/* + * We need to define file_attr if it's missing to know how to convert it to + * fsxattr + */ +struct file_attr { + __u32 fa_xflags; + __u32 fa_extsize; + __u32 fa_nextents; + __u32 fa_projid; + __u32 fa_cowextsize; +}; +#endif + #ifndef FS_IOC_FSGETXATTR /* * Flags for the fsx_xflags field diff --git a/libfrog/Makefile b/libfrog/Makefile index b64ca4597f4e..7d49fd0fe6cc 100644 --- a/libfrog/Makefile +++ b/libfrog/Makefile @@ -24,6 +24,7 @@ fsproperties.c \ fsprops.c \ getparents.c \ histogram.c \ +file_attr.c \ list_sort.c \ linux.c \ logging.c \ @@ -55,6 +56,7 @@ fsprops.h \ getparents.h \ handle_priv.h \ histogram.h \ +file_attr.h \ logging.h \ paths.h \ projects.h \ diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c new file mode 100644 index 000000000000..8592d775f554 --- /dev/null +++ b/libfrog/file_attr.c @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2024 Red Hat, Inc. + * All Rights Reserved. + */ + +#include "file_attr.h" +#include <stdio.h> +#include <errno.h> +#include <string.h> +#include <sys/syscall.h> +#include <asm/types.h> +#include <fcntl.h> + +static void +file_attr_to_fsxattr( + const struct file_attr *fa, + struct fsxattr *fsxa) +{ + memset(fsxa, 0, sizeof(struct fsxattr)); + + fsxa->fsx_xflags = fa->fa_xflags; + fsxa->fsx_extsize = fa->fa_extsize; + fsxa->fsx_nextents = fa->fa_nextents; + fsxa->fsx_projid = fa->fa_projid; + fsxa->fsx_cowextsize = fa->fa_cowextsize; + +} + +static void +fsxattr_to_file_attr( + const struct fsxattr *fsxa, + struct file_attr *fa) +{ + memset(fa, 0, sizeof(struct file_attr)); + + fa->fa_xflags = fsxa->fsx_xflags; + fa->fa_extsize = fsxa->fsx_extsize; + fa->fa_nextents = fsxa->fsx_nextents; + fa->fa_projid = fsxa->fsx_projid; + fa->fa_cowextsize = fsxa->fsx_cowextsize; +} + +int +file_getattr( + const int dfd, + const char *path, + const struct stat *stat, + struct file_attr *fa, + const unsigned int at_flags) +{ + int error; + int fd; + struct fsxattr fsxa; + +#ifdef HAVE_FILE_ATTR + return syscall(__NR_file_getattr, dfd, path, fa, + sizeof(struct file_attr), at_flags); +#else + if (SPECIAL_FILE(stat->st_mode)) + return 0; +#endif + + fd = open(path, O_RDONLY|O_NOCTTY); + if (fd == -1) + return errno; + + error = ioctl(fd, FS_IOC_FSGETXATTR, &fsxa); + close(fd); + + fsxattr_to_file_attr(&fsxa, fa); + + return error; +} + +int +file_setattr( + const int dfd, + const char *path, + const struct stat *stat, + struct file_attr *fa, + const unsigned int at_flags) +{ + int error; + int fd; + struct fsxattr fsxa; + +#ifdef HAVE_FILE_ATTR + return syscall(__NR_file_setattr, dfd, path, fa, + sizeof(struct file_attr), at_flags); +#else + if (SPECIAL_FILE(stat->st_mode)) + return 0; +#endif + + fd = open(path, O_RDONLY|O_NOCTTY); + if (fd == -1) + return errno; + + file_attr_to_fsxattr(fa, &fsxa); + error = ioctl(fd, FS_IOC_FSSETXATTR, fa); + close(fd); + + return error; +} diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h new file mode 100644 index 000000000000..3e56e80a6f95 --- /dev/null +++ b/libfrog/file_attr.h @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2024 Red Hat, Inc. + * All Rights Reserved. + */ +#ifndef __LIBFROG_IXATTR_H__ +#define __LIBFROG_IXATTR_H__ + +#include "linux.h" +#include <sys/stat.h> + +#define SPECIAL_FILE(x) \ + (S_ISCHR((x)) \ + || S_ISBLK((x)) \ + || S_ISFIFO((x)) \ + || S_ISLNK((x)) \ + || S_ISSOCK((x))) + +int +file_getattr( + const int dfd, + const char *path, + const struct stat *stat, + struct file_attr *fa, + const unsigned int at_flags); + +int +file_setattr( + const int dfd, + const char *path, + const struct stat *stat, + struct file_attr *fa, + const unsigned int at_flags); + +#endif /* __LIBFROG_IXATTR_H__ */ diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 index 61353d0aa9d5..cb8ff1576d01 100644 --- a/m4/package_libcdev.m4 +++ b/m4/package_libcdev.m4 @@ -274,3 +274,22 @@ AC_DEFUN([AC_PACKAGE_CHECK_LTO], AC_SUBST(lto_cflags) AC_SUBST(lto_ldflags) ]) + +# +# Check if we have a file_getattr/file_setattr system call (Linux) +# +AC_DEFUN([AC_HAVE_FILE_ATTR], + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) + AC_LINK_IFELSE( + [ AC_LANG_PROGRAM([[ +#define _GNU_SOURCE +#include <sys/syscall.h> +#include <unistd.h> + ]], [[ +syscall(__NR_file_getattr, 0, 0, 0, 0, 0); + ]]) + ], have_file_attr=yes + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + AC_SUBST(have_file_attr) + ]) -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls 2025-08-08 19:30 ` [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls Andrey Albershteyn @ 2025-08-11 15:02 ` Darrick J. Wong 2025-08-11 17:44 ` Andrey Albershteyn 0 siblings, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:02 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: aalbersh, linux-fsdevel, linux-xfs On Fri, Aug 08, 2025 at 09:30:16PM +0200, Andrey Albershteyn wrote: > Add wrappers for new file_getattr/file_setattr inode syscalls which will > be used by xfs_quota and xfs_io. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > configure.ac | 1 + > include/builddefs.in | 5 +++ > include/linux.h | 20 ++++++++++ > libfrog/Makefile | 2 + > libfrog/file_attr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++ > libfrog/file_attr.h | 35 +++++++++++++++++ > m4/package_libcdev.m4 | 19 +++++++++ > 7 files changed, 187 insertions(+) > > diff --git a/configure.ac b/configure.ac > index 9a3309bcdfd1..40a44c571e7b 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -156,6 +156,7 @@ AC_PACKAGE_NEED_RCU_INIT > AC_HAVE_PWRITEV2 > AC_HAVE_COPY_FILE_RANGE > AC_HAVE_CACHESTAT > +AC_HAVE_FILE_ATTR > AC_NEED_INTERNAL_FSXATTR > AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG > AC_NEED_INTERNAL_FSCRYPT_POLICY_V2 > diff --git a/include/builddefs.in b/include/builddefs.in > index 04b4e0880a84..d727b55b854f 100644 > --- a/include/builddefs.in > +++ b/include/builddefs.in > @@ -97,6 +97,7 @@ HAVE_ZIPPED_MANPAGES = @have_zipped_manpages@ > HAVE_PWRITEV2 = @have_pwritev2@ > HAVE_COPY_FILE_RANGE = @have_copy_file_range@ > HAVE_CACHESTAT = @have_cachestat@ > +HAVE_FILE_ATTR = @have_file_attr@ > NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@ > NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@ > NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@ > @@ -169,6 +170,10 @@ ifeq ($(ENABLE_GETTEXT),yes) > GCFLAGS += -DENABLE_GETTEXT > endif > > +ifeq ($(HAVE_FILE_ATTR),yes) > +LCFLAGS += -DHAVE_FILE_ATTR > +endif > + > # Override these if C++ needs other options > SANITIZER_CXXFLAGS = $(SANITIZER_CFLAGS) > GCXXFLAGS = $(GCFLAGS) > diff --git a/include/linux.h b/include/linux.h > index 6e83e073aa2e..018cc78960e3 100644 > --- a/include/linux.h > +++ b/include/linux.h > @@ -16,6 +16,7 @@ > #include <sys/param.h> > #include <sys/sysmacros.h> > #include <sys/stat.h> > +#include <sys/syscall.h> > #include <inttypes.h> > #include <malloc.h> > #include <getopt.h> > @@ -202,6 +203,25 @@ struct fsxattr { > }; > #endif > > +/* > + * Use __NR_file_getattr instead of build system HAVE_FILE_ATTR as this header > + * could be included in other places where HAVE_FILE_ATTR is not defined (e.g. > + * xfstests's conftest.c in ./configure) > + */ > +#ifndef __NR_file_getattr Seeing as uapi fs.h now has: #define FILE_ATTR_SIZE_VER0 24 #define FILE_ATTR_SIZE_LATEST FILE_ATTR_SIZE_VER0 I wonder if you'd be better off gating on one of those defines rather than the presence of the syscall number? > +/* > + * We need to define file_attr if it's missing to know how to convert it to > + * fsxattr > + */ > +struct file_attr { > + __u32 fa_xflags; > + __u32 fa_extsize; > + __u32 fa_nextents; > + __u32 fa_projid; > + __u32 fa_cowextsize; > +}; > +#endif > + > #ifndef FS_IOC_FSGETXATTR > /* > * Flags for the fsx_xflags field > diff --git a/libfrog/Makefile b/libfrog/Makefile > index b64ca4597f4e..7d49fd0fe6cc 100644 > --- a/libfrog/Makefile > +++ b/libfrog/Makefile > @@ -24,6 +24,7 @@ fsproperties.c \ > fsprops.c \ > getparents.c \ > histogram.c \ > +file_attr.c \ > list_sort.c \ > linux.c \ > logging.c \ > @@ -55,6 +56,7 @@ fsprops.h \ > getparents.h \ > handle_priv.h \ > histogram.h \ > +file_attr.h \ > logging.h \ > paths.h \ > projects.h \ > diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c > new file mode 100644 > index 000000000000..8592d775f554 > --- /dev/null > +++ b/libfrog/file_attr.c > @@ -0,0 +1,105 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2024 Red Hat, Inc. > + * All Rights Reserved. > + */ > + > +#include "file_attr.h" > +#include <stdio.h> > +#include <errno.h> > +#include <string.h> > +#include <sys/syscall.h> > +#include <asm/types.h> > +#include <fcntl.h> > + > +static void > +file_attr_to_fsxattr( > + const struct file_attr *fa, > + struct fsxattr *fsxa) > +{ > + memset(fsxa, 0, sizeof(struct fsxattr)); > + > + fsxa->fsx_xflags = fa->fa_xflags; > + fsxa->fsx_extsize = fa->fa_extsize; > + fsxa->fsx_nextents = fa->fa_nextents; > + fsxa->fsx_projid = fa->fa_projid; > + fsxa->fsx_cowextsize = fa->fa_cowextsize; > + > +} > + > +static void > +fsxattr_to_file_attr( > + const struct fsxattr *fsxa, > + struct file_attr *fa) > +{ > + memset(fa, 0, sizeof(struct file_attr)); > + > + fa->fa_xflags = fsxa->fsx_xflags; > + fa->fa_extsize = fsxa->fsx_extsize; > + fa->fa_nextents = fsxa->fsx_nextents; > + fa->fa_projid = fsxa->fsx_projid; > + fa->fa_cowextsize = fsxa->fsx_cowextsize; > +} > + > +int > +file_getattr( > + const int dfd, > + const char *path, > + const struct stat *stat, > + struct file_attr *fa, > + const unsigned int at_flags) > +{ Will this cause a naming conflict when libc wraps the new syscall? > + int error; > + int fd; > + struct fsxattr fsxa; > + > +#ifdef HAVE_FILE_ATTR > + return syscall(__NR_file_getattr, dfd, path, fa, > + sizeof(struct file_attr), at_flags); What happens if we build xfsprogs on new userspace but it then gets run on an old kernel that doesn't support file_getattr(2)? Shouldn't we fall back to the old ioctl on ENOSYS? > +#else > + if (SPECIAL_FILE(stat->st_mode)) > + return 0; Why does it return 0 without filling out @fa? Shouldn't this be EOPNOTSUPP or something? > +#endif > + > + fd = open(path, O_RDONLY|O_NOCTTY); > + if (fd == -1) > + return errno; > + > + error = ioctl(fd, FS_IOC_FSGETXATTR, &fsxa); > + close(fd); > + > + fsxattr_to_file_attr(&fsxa, fa); Er... if the ioctl errors out, fsxa will still be uninitialized stack garbage, which is (pointlessly) copied to the caller's fa structure. > + > + return error; I'm confused about the return value of this function. If the syscall or the ioctl fail we'll pass the -1 to the caller and let them access errno, but if the open fails we return errno directly? > +} > + > +int > +file_setattr( > + const int dfd, > + const char *path, > + const struct stat *stat, > + struct file_attr *fa, > + const unsigned int at_flags) > +{ > + int error; > + int fd; > + struct fsxattr fsxa; > + > +#ifdef HAVE_FILE_ATTR > + return syscall(__NR_file_setattr, dfd, path, fa, > + sizeof(struct file_attr), at_flags); > +#else > + if (SPECIAL_FILE(stat->st_mode)) > + return 0; > +#endif > + > + fd = open(path, O_RDONLY|O_NOCTTY); > + if (fd == -1) > + return errno; Same comments that I had about file_getattr. > + > + file_attr_to_fsxattr(fa, &fsxa); > + error = ioctl(fd, FS_IOC_FSSETXATTR, fa); > + close(fd); > + > + return error; > +} > diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h > new file mode 100644 > index 000000000000..3e56e80a6f95 > --- /dev/null > +++ b/libfrog/file_attr.h > @@ -0,0 +1,35 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2024 Red Hat, Inc. > + * All Rights Reserved. > + */ > +#ifndef __LIBFROG_IXATTR_H__ > +#define __LIBFROG_IXATTR_H__ __LIBFROG_FILE_ATTR_H__ ? --D > + > +#include "linux.h" > +#include <sys/stat.h> > + > +#define SPECIAL_FILE(x) \ > + (S_ISCHR((x)) \ > + || S_ISBLK((x)) \ > + || S_ISFIFO((x)) \ > + || S_ISLNK((x)) \ > + || S_ISSOCK((x))) > + > +int > +file_getattr( > + const int dfd, > + const char *path, > + const struct stat *stat, > + struct file_attr *fa, > + const unsigned int at_flags); > + > +int > +file_setattr( > + const int dfd, > + const char *path, > + const struct stat *stat, > + struct file_attr *fa, > + const unsigned int at_flags); > + > +#endif /* __LIBFROG_IXATTR_H__ */ > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > index 61353d0aa9d5..cb8ff1576d01 100644 > --- a/m4/package_libcdev.m4 > +++ b/m4/package_libcdev.m4 > @@ -274,3 +274,22 @@ AC_DEFUN([AC_PACKAGE_CHECK_LTO], > AC_SUBST(lto_cflags) > AC_SUBST(lto_ldflags) > ]) > + > +# > +# Check if we have a file_getattr/file_setattr system call (Linux) > +# > +AC_DEFUN([AC_HAVE_FILE_ATTR], > + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) > + AC_LINK_IFELSE( > + [ AC_LANG_PROGRAM([[ > +#define _GNU_SOURCE > +#include <sys/syscall.h> > +#include <unistd.h> > + ]], [[ > +syscall(__NR_file_getattr, 0, 0, 0, 0, 0); > + ]]) > + ], have_file_attr=yes > + AC_MSG_RESULT(yes), > + AC_MSG_RESULT(no)) > + AC_SUBST(have_file_attr) > + ]) > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls 2025-08-11 15:02 ` Darrick J. Wong @ 2025-08-11 17:44 ` Andrey Albershteyn 2025-08-12 14:53 ` Darrick J. Wong 0 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 17:44 UTC (permalink / raw) To: Darrick J. Wong; +Cc: aalbersh, linux-fsdevel, linux-xfs On 2025-08-11 08:02:42, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:30:16PM +0200, Andrey Albershteyn wrote: > > Add wrappers for new file_getattr/file_setattr inode syscalls which will > > be used by xfs_quota and xfs_io. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > configure.ac | 1 + > > include/builddefs.in | 5 +++ > > include/linux.h | 20 ++++++++++ > > libfrog/Makefile | 2 + > > libfrog/file_attr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > libfrog/file_attr.h | 35 +++++++++++++++++ > > m4/package_libcdev.m4 | 19 +++++++++ > > 7 files changed, 187 insertions(+) > > > > diff --git a/configure.ac b/configure.ac > > index 9a3309bcdfd1..40a44c571e7b 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -156,6 +156,7 @@ AC_PACKAGE_NEED_RCU_INIT > > AC_HAVE_PWRITEV2 > > AC_HAVE_COPY_FILE_RANGE > > AC_HAVE_CACHESTAT > > +AC_HAVE_FILE_ATTR > > AC_NEED_INTERNAL_FSXATTR > > AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG > > AC_NEED_INTERNAL_FSCRYPT_POLICY_V2 > > diff --git a/include/builddefs.in b/include/builddefs.in > > index 04b4e0880a84..d727b55b854f 100644 > > --- a/include/builddefs.in > > +++ b/include/builddefs.in > > @@ -97,6 +97,7 @@ HAVE_ZIPPED_MANPAGES = @have_zipped_manpages@ > > HAVE_PWRITEV2 = @have_pwritev2@ > > HAVE_COPY_FILE_RANGE = @have_copy_file_range@ > > HAVE_CACHESTAT = @have_cachestat@ > > +HAVE_FILE_ATTR = @have_file_attr@ > > NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@ > > NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@ > > NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@ > > @@ -169,6 +170,10 @@ ifeq ($(ENABLE_GETTEXT),yes) > > GCFLAGS += -DENABLE_GETTEXT > > endif > > > > +ifeq ($(HAVE_FILE_ATTR),yes) > > +LCFLAGS += -DHAVE_FILE_ATTR > > +endif > > + > > # Override these if C++ needs other options > > SANITIZER_CXXFLAGS = $(SANITIZER_CFLAGS) > > GCXXFLAGS = $(GCFLAGS) > > diff --git a/include/linux.h b/include/linux.h > > index 6e83e073aa2e..018cc78960e3 100644 > > --- a/include/linux.h > > +++ b/include/linux.h > > @@ -16,6 +16,7 @@ > > #include <sys/param.h> > > #include <sys/sysmacros.h> > > #include <sys/stat.h> > > +#include <sys/syscall.h> > > #include <inttypes.h> > > #include <malloc.h> > > #include <getopt.h> > > @@ -202,6 +203,25 @@ struct fsxattr { > > }; > > #endif > > > > +/* > > + * Use __NR_file_getattr instead of build system HAVE_FILE_ATTR as this header > > + * could be included in other places where HAVE_FILE_ATTR is not defined (e.g. > > + * xfstests's conftest.c in ./configure) > > + */ > > +#ifndef __NR_file_getattr > > Seeing as uapi fs.h now has: > > #define FILE_ATTR_SIZE_VER0 24 > #define FILE_ATTR_SIZE_LATEST FILE_ATTR_SIZE_VER0 > > I wonder if you'd be better off gating on one of those defines rather > than the presence of the syscall number? Hmm, yeah, should work > > > +/* > > + * We need to define file_attr if it's missing to know how to convert it to > > + * fsxattr > > + */ > > +struct file_attr { > > + __u32 fa_xflags; > > + __u32 fa_extsize; > > + __u32 fa_nextents; > > + __u32 fa_projid; > > + __u32 fa_cowextsize; > > +}; > > +#endif > > + > > #ifndef FS_IOC_FSGETXATTR > > /* > > * Flags for the fsx_xflags field > > diff --git a/libfrog/Makefile b/libfrog/Makefile > > index b64ca4597f4e..7d49fd0fe6cc 100644 > > --- a/libfrog/Makefile > > +++ b/libfrog/Makefile > > @@ -24,6 +24,7 @@ fsproperties.c \ > > fsprops.c \ > > getparents.c \ > > histogram.c \ > > +file_attr.c \ > > list_sort.c \ > > linux.c \ > > logging.c \ > > @@ -55,6 +56,7 @@ fsprops.h \ > > getparents.h \ > > handle_priv.h \ > > histogram.h \ > > +file_attr.h \ > > logging.h \ > > paths.h \ > > projects.h \ > > diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c > > new file mode 100644 > > index 000000000000..8592d775f554 > > --- /dev/null > > +++ b/libfrog/file_attr.c > > @@ -0,0 +1,105 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2024 Red Hat, Inc. > > + * All Rights Reserved. > > + */ > > + > > +#include "file_attr.h" > > +#include <stdio.h> > > +#include <errno.h> > > +#include <string.h> > > +#include <sys/syscall.h> > > +#include <asm/types.h> > > +#include <fcntl.h> > > + > > +static void > > +file_attr_to_fsxattr( > > + const struct file_attr *fa, > > + struct fsxattr *fsxa) > > +{ > > + memset(fsxa, 0, sizeof(struct fsxattr)); > > + > > + fsxa->fsx_xflags = fa->fa_xflags; > > + fsxa->fsx_extsize = fa->fa_extsize; > > + fsxa->fsx_nextents = fa->fa_nextents; > > + fsxa->fsx_projid = fa->fa_projid; > > + fsxa->fsx_cowextsize = fa->fa_cowextsize; > > + > > +} > > + > > +static void > > +fsxattr_to_file_attr( > > + const struct fsxattr *fsxa, > > + struct file_attr *fa) > > +{ > > + memset(fa, 0, sizeof(struct file_attr)); > > + > > + fa->fa_xflags = fsxa->fsx_xflags; > > + fa->fa_extsize = fsxa->fsx_extsize; > > + fa->fa_nextents = fsxa->fsx_nextents; > > + fa->fa_projid = fsxa->fsx_projid; > > + fa->fa_cowextsize = fsxa->fsx_cowextsize; > > +} > > + > > +int > > +file_getattr( > > + const int dfd, > > + const char *path, > > + const struct stat *stat, > > + struct file_attr *fa, > > + const unsigned int at_flags) > > +{ > > Will this cause a naming conflict when libc wraps the new syscall? xfrog_file_getattr? > > > + int error; > > + int fd; > > + struct fsxattr fsxa; > > + > > +#ifdef HAVE_FILE_ATTR > > + return syscall(__NR_file_getattr, dfd, path, fa, > > + sizeof(struct file_attr), at_flags); > > What happens if we build xfsprogs on new userspace but it then gets run > on an old kernel that doesn't support file_getattr(2)? Shouldn't we > fall back to the old ioctl on ENOSYS? oh right, missed that. I can add this check. Is it something common in general? I suppose booting into older kernel when xfsprogs was compiled with the "current" one is one case but it's expected that kernel can miss some features > > > +#else > > + if (SPECIAL_FILE(stat->st_mode)) > > + return 0; > > Why does it return 0 without filling out @fa? Shouldn't this be > EOPNOTSUPP or something? > > > +#endif > > + > > + fd = open(path, O_RDONLY|O_NOCTTY); > > + if (fd == -1) > > + return errno; > > + > > + error = ioctl(fd, FS_IOC_FSGETXATTR, &fsxa); > > + close(fd); > > + > > + fsxattr_to_file_attr(&fsxa, fa); > > Er... if the ioctl errors out, fsxa will still be uninitialized stack > garbage, which is (pointlessly) copied to the caller's fa structure. > > > + > > + return error; > > I'm confused about the return value of this function. If the syscall > or the ioctl fail we'll pass the -1 to the caller and let them access > errno, but if the open fails we return errno directly? I was trying to just wrap the old code without changing the output, I haven't thought too hard about design of this function. I will apply your suggestion including EOPNOSUPP mentioned in other mail. > > > +} > > + > > +int > > +file_setattr( > > + const int dfd, > > + const char *path, > > + const struct stat *stat, > > + struct file_attr *fa, > > + const unsigned int at_flags) > > +{ > > + int error; > > + int fd; > > + struct fsxattr fsxa; > > + > > +#ifdef HAVE_FILE_ATTR > > + return syscall(__NR_file_setattr, dfd, path, fa, > > + sizeof(struct file_attr), at_flags); > > +#else > > + if (SPECIAL_FILE(stat->st_mode)) > > + return 0; > > +#endif > > + > > + fd = open(path, O_RDONLY|O_NOCTTY); > > + if (fd == -1) > > + return errno; > > Same comments that I had about file_getattr. > > > + > > + file_attr_to_fsxattr(fa, &fsxa); > > + error = ioctl(fd, FS_IOC_FSSETXATTR, fa); > > + close(fd); > > + > > + return error; > > +} > > diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h > > new file mode 100644 > > index 000000000000..3e56e80a6f95 > > --- /dev/null > > +++ b/libfrog/file_attr.h > > @@ -0,0 +1,35 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2024 Red Hat, Inc. > > + * All Rights Reserved. > > + */ > > +#ifndef __LIBFROG_IXATTR_H__ > > +#define __LIBFROG_IXATTR_H__ > > __LIBFROG_FILE_ATTR_H__ ? ops, right > > --D > > > + > > +#include "linux.h" > > +#include <sys/stat.h> > > + > > +#define SPECIAL_FILE(x) \ > > + (S_ISCHR((x)) \ > > + || S_ISBLK((x)) \ > > + || S_ISFIFO((x)) \ > > + || S_ISLNK((x)) \ > > + || S_ISSOCK((x))) > > + > > +int > > +file_getattr( > > + const int dfd, > > + const char *path, > > + const struct stat *stat, > > + struct file_attr *fa, > > + const unsigned int at_flags); > > + > > +int > > +file_setattr( > > + const int dfd, > > + const char *path, > > + const struct stat *stat, > > + struct file_attr *fa, > > + const unsigned int at_flags); > > + > > +#endif /* __LIBFROG_IXATTR_H__ */ > > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > > index 61353d0aa9d5..cb8ff1576d01 100644 > > --- a/m4/package_libcdev.m4 > > +++ b/m4/package_libcdev.m4 > > @@ -274,3 +274,22 @@ AC_DEFUN([AC_PACKAGE_CHECK_LTO], > > AC_SUBST(lto_cflags) > > AC_SUBST(lto_ldflags) > > ]) > > + > > +# > > +# Check if we have a file_getattr/file_setattr system call (Linux) > > +# > > +AC_DEFUN([AC_HAVE_FILE_ATTR], > > + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) > > + AC_LINK_IFELSE( > > + [ AC_LANG_PROGRAM([[ > > +#define _GNU_SOURCE > > +#include <sys/syscall.h> > > +#include <unistd.h> > > + ]], [[ > > +syscall(__NR_file_getattr, 0, 0, 0, 0, 0); > > + ]]) > > + ], have_file_attr=yes > > + AC_MSG_RESULT(yes), > > + AC_MSG_RESULT(no)) > > + AC_SUBST(have_file_attr) > > + ]) > > > > -- > > 2.49.0 > > > > > -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls 2025-08-11 17:44 ` Andrey Albershteyn @ 2025-08-12 14:53 ` Darrick J. Wong 0 siblings, 0 replies; 33+ messages in thread From: Darrick J. Wong @ 2025-08-12 14:53 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: aalbersh, linux-fsdevel, linux-xfs On Mon, Aug 11, 2025 at 07:44:14PM +0200, Andrey Albershteyn wrote: > On 2025-08-11 08:02:42, Darrick J. Wong wrote: > > On Fri, Aug 08, 2025 at 09:30:16PM +0200, Andrey Albershteyn wrote: > > > Add wrappers for new file_getattr/file_setattr inode syscalls which will > > > be used by xfs_quota and xfs_io. > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > > --- > > > configure.ac | 1 + > > > include/builddefs.in | 5 +++ > > > include/linux.h | 20 ++++++++++ > > > libfrog/Makefile | 2 + > > > libfrog/file_attr.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > libfrog/file_attr.h | 35 +++++++++++++++++ > > > m4/package_libcdev.m4 | 19 +++++++++ > > > 7 files changed, 187 insertions(+) > > > > > > diff --git a/configure.ac b/configure.ac > > > index 9a3309bcdfd1..40a44c571e7b 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -156,6 +156,7 @@ AC_PACKAGE_NEED_RCU_INIT > > > AC_HAVE_PWRITEV2 > > > AC_HAVE_COPY_FILE_RANGE > > > AC_HAVE_CACHESTAT > > > +AC_HAVE_FILE_ATTR > > > AC_NEED_INTERNAL_FSXATTR > > > AC_NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG > > > AC_NEED_INTERNAL_FSCRYPT_POLICY_V2 > > > diff --git a/include/builddefs.in b/include/builddefs.in > > > index 04b4e0880a84..d727b55b854f 100644 > > > --- a/include/builddefs.in > > > +++ b/include/builddefs.in > > > @@ -97,6 +97,7 @@ HAVE_ZIPPED_MANPAGES = @have_zipped_manpages@ > > > HAVE_PWRITEV2 = @have_pwritev2@ > > > HAVE_COPY_FILE_RANGE = @have_copy_file_range@ > > > HAVE_CACHESTAT = @have_cachestat@ > > > +HAVE_FILE_ATTR = @have_file_attr@ > > > NEED_INTERNAL_FSXATTR = @need_internal_fsxattr@ > > > NEED_INTERNAL_FSCRYPT_ADD_KEY_ARG = @need_internal_fscrypt_add_key_arg@ > > > NEED_INTERNAL_FSCRYPT_POLICY_V2 = @need_internal_fscrypt_policy_v2@ > > > @@ -169,6 +170,10 @@ ifeq ($(ENABLE_GETTEXT),yes) > > > GCFLAGS += -DENABLE_GETTEXT > > > endif > > > > > > +ifeq ($(HAVE_FILE_ATTR),yes) > > > +LCFLAGS += -DHAVE_FILE_ATTR > > > +endif > > > + > > > # Override these if C++ needs other options > > > SANITIZER_CXXFLAGS = $(SANITIZER_CFLAGS) > > > GCXXFLAGS = $(GCFLAGS) > > > diff --git a/include/linux.h b/include/linux.h > > > index 6e83e073aa2e..018cc78960e3 100644 > > > --- a/include/linux.h > > > +++ b/include/linux.h > > > @@ -16,6 +16,7 @@ > > > #include <sys/param.h> > > > #include <sys/sysmacros.h> > > > #include <sys/stat.h> > > > +#include <sys/syscall.h> > > > #include <inttypes.h> > > > #include <malloc.h> > > > #include <getopt.h> > > > @@ -202,6 +203,25 @@ struct fsxattr { > > > }; > > > #endif > > > > > > +/* > > > + * Use __NR_file_getattr instead of build system HAVE_FILE_ATTR as this header > > > + * could be included in other places where HAVE_FILE_ATTR is not defined (e.g. > > > + * xfstests's conftest.c in ./configure) > > > + */ > > > +#ifndef __NR_file_getattr > > > > Seeing as uapi fs.h now has: > > > > #define FILE_ATTR_SIZE_VER0 24 > > #define FILE_ATTR_SIZE_LATEST FILE_ATTR_SIZE_VER0 > > > > I wonder if you'd be better off gating on one of those defines rather > > than the presence of the syscall number? > > Hmm, yeah, should work > > > > > > +/* > > > + * We need to define file_attr if it's missing to know how to convert it to > > > + * fsxattr > > > + */ > > > +struct file_attr { > > > + __u32 fa_xflags; > > > + __u32 fa_extsize; > > > + __u32 fa_nextents; > > > + __u32 fa_projid; > > > + __u32 fa_cowextsize; > > > +}; > > > +#endif > > > + > > > #ifndef FS_IOC_FSGETXATTR > > > /* > > > * Flags for the fsx_xflags field > > > diff --git a/libfrog/Makefile b/libfrog/Makefile > > > index b64ca4597f4e..7d49fd0fe6cc 100644 > > > --- a/libfrog/Makefile > > > +++ b/libfrog/Makefile > > > @@ -24,6 +24,7 @@ fsproperties.c \ > > > fsprops.c \ > > > getparents.c \ > > > histogram.c \ > > > +file_attr.c \ > > > list_sort.c \ > > > linux.c \ > > > logging.c \ > > > @@ -55,6 +56,7 @@ fsprops.h \ > > > getparents.h \ > > > handle_priv.h \ > > > histogram.h \ > > > +file_attr.h \ > > > logging.h \ > > > paths.h \ > > > projects.h \ > > > diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c > > > new file mode 100644 > > > index 000000000000..8592d775f554 > > > --- /dev/null > > > +++ b/libfrog/file_attr.c > > > @@ -0,0 +1,105 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (c) 2024 Red Hat, Inc. > > > + * All Rights Reserved. > > > + */ > > > + > > > +#include "file_attr.h" > > > +#include <stdio.h> > > > +#include <errno.h> > > > +#include <string.h> > > > +#include <sys/syscall.h> > > > +#include <asm/types.h> > > > +#include <fcntl.h> > > > + > > > +static void > > > +file_attr_to_fsxattr( > > > + const struct file_attr *fa, > > > + struct fsxattr *fsxa) > > > +{ > > > + memset(fsxa, 0, sizeof(struct fsxattr)); > > > + > > > + fsxa->fsx_xflags = fa->fa_xflags; > > > + fsxa->fsx_extsize = fa->fa_extsize; > > > + fsxa->fsx_nextents = fa->fa_nextents; > > > + fsxa->fsx_projid = fa->fa_projid; > > > + fsxa->fsx_cowextsize = fa->fa_cowextsize; > > > + > > > +} > > > + > > > +static void > > > +fsxattr_to_file_attr( > > > + const struct fsxattr *fsxa, > > > + struct file_attr *fa) > > > +{ > > > + memset(fa, 0, sizeof(struct file_attr)); > > > + > > > + fa->fa_xflags = fsxa->fsx_xflags; > > > + fa->fa_extsize = fsxa->fsx_extsize; > > > + fa->fa_nextents = fsxa->fsx_nextents; > > > + fa->fa_projid = fsxa->fsx_projid; > > > + fa->fa_cowextsize = fsxa->fsx_cowextsize; > > > +} > > > + > > > +int > > > +file_getattr( > > > + const int dfd, > > > + const char *path, > > > + const struct stat *stat, > > > + struct file_attr *fa, > > > + const unsigned int at_flags) > > > +{ > > > > Will this cause a naming conflict when libc wraps the new syscall? > > xfrog_file_getattr? > > > > > > + int error; > > > + int fd; > > > + struct fsxattr fsxa; > > > + > > > +#ifdef HAVE_FILE_ATTR > > > + return syscall(__NR_file_getattr, dfd, path, fa, > > > + sizeof(struct file_attr), at_flags); > > > > What happens if we build xfsprogs on new userspace but it then gets run > > on an old kernel that doesn't support file_getattr(2)? Shouldn't we > > fall back to the old ioctl on ENOSYS? > > oh right, missed that. I can add this check. > > Is it something common in general? I suppose booting into older > kernel when xfsprogs was compiled with the "current" one is one case > but it's expected that kernel can miss some features I don't think it's common among packaged distributions, but developers (or at least myself) regularly do things like that. > > > > > +#else > > > + if (SPECIAL_FILE(stat->st_mode)) > > > + return 0; > > > > Why does it return 0 without filling out @fa? Shouldn't this be > > EOPNOTSUPP or something? > > > > > +#endif > > > + > > > + fd = open(path, O_RDONLY|O_NOCTTY); > > > + if (fd == -1) > > > + return errno; > > > + > > > + error = ioctl(fd, FS_IOC_FSGETXATTR, &fsxa); > > > + close(fd); > > > + > > > + fsxattr_to_file_attr(&fsxa, fa); > > > > Er... if the ioctl errors out, fsxa will still be uninitialized stack > > garbage, which is (pointlessly) copied to the caller's fa structure. > > > > > + > > > + return error; > > > > I'm confused about the return value of this function. If the syscall > > or the ioctl fail we'll pass the -1 to the caller and let them access > > errno, but if the open fails we return errno directly? > > I was trying to just wrap the old code without changing the output, > I haven't thought too hard about design of this function. I will > apply your suggestion including EOPNOSUPP mentioned in other mail. <nod> --D > > > > > +} > > > + > > > +int > > > +file_setattr( > > > + const int dfd, > > > + const char *path, > > > + const struct stat *stat, > > > + struct file_attr *fa, > > > + const unsigned int at_flags) > > > +{ > > > + int error; > > > + int fd; > > > + struct fsxattr fsxa; > > > + > > > +#ifdef HAVE_FILE_ATTR > > > + return syscall(__NR_file_setattr, dfd, path, fa, > > > + sizeof(struct file_attr), at_flags); > > > +#else > > > + if (SPECIAL_FILE(stat->st_mode)) > > > + return 0; > > > +#endif > > > + > > > + fd = open(path, O_RDONLY|O_NOCTTY); > > > + if (fd == -1) > > > + return errno; > > > > Same comments that I had about file_getattr. > > > > > + > > > + file_attr_to_fsxattr(fa, &fsxa); > > > + error = ioctl(fd, FS_IOC_FSSETXATTR, fa); > > > + close(fd); > > > + > > > + return error; > > > +} > > > diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h > > > new file mode 100644 > > > index 000000000000..3e56e80a6f95 > > > --- /dev/null > > > +++ b/libfrog/file_attr.h > > > @@ -0,0 +1,35 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (c) 2024 Red Hat, Inc. > > > + * All Rights Reserved. > > > + */ > > > +#ifndef __LIBFROG_IXATTR_H__ > > > +#define __LIBFROG_IXATTR_H__ > > > > __LIBFROG_FILE_ATTR_H__ ? > > ops, right > > > > > --D > > > > > + > > > +#include "linux.h" > > > +#include <sys/stat.h> > > > + > > > +#define SPECIAL_FILE(x) \ > > > + (S_ISCHR((x)) \ > > > + || S_ISBLK((x)) \ > > > + || S_ISFIFO((x)) \ > > > + || S_ISLNK((x)) \ > > > + || S_ISSOCK((x))) > > > + > > > +int > > > +file_getattr( > > > + const int dfd, > > > + const char *path, > > > + const struct stat *stat, > > > + struct file_attr *fa, > > > + const unsigned int at_flags); > > > + > > > +int > > > +file_setattr( > > > + const int dfd, > > > + const char *path, > > > + const struct stat *stat, > > > + struct file_attr *fa, > > > + const unsigned int at_flags); > > > + > > > +#endif /* __LIBFROG_IXATTR_H__ */ > > > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > > > index 61353d0aa9d5..cb8ff1576d01 100644 > > > --- a/m4/package_libcdev.m4 > > > +++ b/m4/package_libcdev.m4 > > > @@ -274,3 +274,22 @@ AC_DEFUN([AC_PACKAGE_CHECK_LTO], > > > AC_SUBST(lto_cflags) > > > AC_SUBST(lto_ldflags) > > > ]) > > > + > > > +# > > > +# Check if we have a file_getattr/file_setattr system call (Linux) > > > +# > > > +AC_DEFUN([AC_HAVE_FILE_ATTR], > > > + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) > > > + AC_LINK_IFELSE( > > > + [ AC_LANG_PROGRAM([[ > > > +#define _GNU_SOURCE > > > +#include <sys/syscall.h> > > > +#include <unistd.h> > > > + ]], [[ > > > +syscall(__NR_file_getattr, 0, 0, 0, 0, 0); > > > + ]]) > > > + ], have_file_attr=yes > > > + AC_MSG_RESULT(yes), > > > + AC_MSG_RESULT(no)) > > > + AC_SUBST(have_file_attr) > > > + ]) > > > > > > -- > > > 2.49.0 > > > > > > > > > > -- > - Andrey > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls Andrey Albershteyn @ 2025-08-08 19:30 ` Andrey Albershteyn 2025-08-11 15:07 ` Darrick J. Wong 2025-08-08 19:30 ` [PATCH 3/4] xfs_io: make ls/chattr work with " Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump Andrey Albershteyn 3 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:30 UTC (permalink / raw) To: aalbersh, linux-fsdevel, linux-xfs; +Cc: Andrey Albershteyn From: Andrey Albershteyn <aalbersh@redhat.com> Utilize new file_getattr/file_setattr syscalls to set project ID on special files. Previously, special files were skipped due to lack of the way to call FS_IOC_SETFSXATTR ioctl on them. The quota accounting was therefore missing these inodes (special files created before project setup). The ones created after porject initialization did inherit the projid flag from the parent. Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> --- quota/project.c | 144 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 74 insertions(+), 70 deletions(-) diff --git a/quota/project.c b/quota/project.c index adb26945fa57..93d7ace0e11b 100644 --- a/quota/project.c +++ b/quota/project.c @@ -4,14 +4,17 @@ * All Rights Reserved. */ +#include <unistd.h> #include "command.h" #include "input.h" #include "init.h" +#include "libfrog/file_attr.h" #include "quota.h" static cmdinfo_t project_cmd; static prid_t prid; static int recurse_depth = -1; +static int dfd; enum { CHECK_PROJECT = 0x1, @@ -19,13 +22,6 @@ enum { CLEAR_PROJECT = 0x4, }; -#define EXCLUDED_FILE_TYPES(x) \ - (S_ISCHR((x)) \ - || S_ISBLK((x)) \ - || S_ISFIFO((x)) \ - || S_ISLNK((x)) \ - || S_ISSOCK((x))) - static void project_help(void) { @@ -85,8 +81,8 @@ check_project( int flag, struct FTW *data) { - struct fsxattr fsx; - int fd; + int error; + struct file_attr fa = { 0 }; if (recurse_depth >= 0 && data->level > recurse_depth) return 0; @@ -96,30 +92,30 @@ check_project( fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); return 0; } - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); - return 0; - } - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { - exitcode = 1; - fprintf(stderr, _("%s: cannot open %s: %s\n"), - progname, path, strerror(errno)); - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { - exitcode = 1; + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { +#ifndef HAVE_FILE_ATTR + if (SPECIAL_FILE(stat->st_mode)) { + fprintf(stderr, _("%s: skipping special file %s\n"), + progname, path); + return 0; + } +#endif fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), - progname, path, strerror(errno)); - } else { - if (fsx.fsx_projid != prid) - printf(_("%s - project identifier is not set" - " (inode=%u, tree=%u)\n"), - path, fsx.fsx_projid, (unsigned int)prid); - if (!(fsx.fsx_xflags & FS_XFLAG_PROJINHERIT) && S_ISDIR(stat->st_mode)) - printf(_("%s - project inheritance flag is not set\n"), - path); + progname, path, strerror(errno)); + exitcode = 1; + return 0; } - if (fd != -1) - close(fd); + + if (fa.fa_projid != prid) + printf(_("%s - project identifier is not set" + " (inode=%u, tree=%u)\n"), + path, fa.fa_projid, (unsigned int)prid); + if (!(fa.fa_xflags & FS_XFLAG_PROJINHERIT) && S_ISDIR(stat->st_mode)) + printf(_("%s - project inheritance flag is not set\n"), + path); + return 0; } @@ -130,8 +126,8 @@ clear_project( int flag, struct FTW *data) { - struct fsxattr fsx; - int fd; + int error; + struct file_attr fa; if (recurse_depth >= 0 && data->level > recurse_depth) return 0; @@ -141,32 +137,31 @@ clear_project( fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); return 0; } - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); - return 0; - } - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { - exitcode = 1; - fprintf(stderr, _("%s: cannot open %s: %s\n"), - progname, path, strerror(errno)); - return 0; - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx) < 0) { - exitcode = 1; + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { +#ifndef HAVE_FILE_ATTR + if (SPECIAL_FILE(stat->st_mode)) { + fprintf(stderr, _("%s: skipping special file %s\n"), + progname, path); + return 0; + } +#endif fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), - progname, path, strerror(errno)); - close(fd); + progname, path, strerror(errno)); + exitcode = 1; return 0; } - fsx.fsx_projid = 0; - fsx.fsx_xflags &= ~FS_XFLAG_PROJINHERIT; - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &fsx) < 0) { - exitcode = 1; + fa.fa_projid = 0; + fa.fa_xflags &= ~FS_XFLAG_PROJINHERIT; + + error = file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { fprintf(stderr, _("%s: cannot clear project on %s: %s\n"), progname, path, strerror(errno)); + exitcode = 1; } - close(fd); return 0; } @@ -177,8 +172,8 @@ setup_project( int flag, struct FTW *data) { - struct fsxattr fsx; - int fd; + struct file_attr fa; + int error; if (recurse_depth >= 0 && data->level > recurse_depth) return 0; @@ -188,32 +183,32 @@ setup_project( fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); return 0; } - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); - return 0; - } - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { - exitcode = 1; - fprintf(stderr, _("%s: cannot open %s: %s\n"), - progname, path, strerror(errno)); - return 0; - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx) < 0) { - exitcode = 1; + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { +#ifndef HAVE_FILE_ATTR + if (SPECIAL_FILE(stat->st_mode)) { + fprintf(stderr, _("%s: skipping special file %s\n"), + progname, path); + return 0; + } +#endif fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), - progname, path, strerror(errno)); - close(fd); + progname, path, strerror(errno)); + exitcode = 1; return 0; } - fsx.fsx_projid = prid; - fsx.fsx_xflags |= FS_XFLAG_PROJINHERIT; - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &fsx) < 0) { - exitcode = 1; + fa.fa_projid = prid; + if (S_ISDIR(stat->st_mode)) + fa.fa_xflags |= FS_XFLAG_PROJINHERIT; + + error = file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { fprintf(stderr, _("%s: cannot set project on %s: %s\n"), progname, path, strerror(errno)); + exitcode = 1; } - close(fd); return 0; } @@ -223,6 +218,13 @@ project_operations( char *dir, int type) { + dfd = open(dir, O_RDONLY|O_NOCTTY); + if (dfd < -1) { + printf(_("Error opening dir %s for project %s...\n"), dir, + project); + return; + } + switch (type) { case CHECK_PROJECT: printf(_("Checking project %s (path %s)...\n"), project, dir); @@ -237,6 +239,8 @@ project_operations( nftw(dir, clear_project, 100, FTW_PHYS|FTW_MOUNT); break; } + + close(dfd); } static void -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files 2025-08-08 19:30 ` [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files Andrey Albershteyn @ 2025-08-11 15:07 ` Darrick J. Wong 2025-08-11 17:51 ` Andrey Albershteyn 0 siblings, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:07 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: aalbersh, linux-fsdevel, linux-xfs On Fri, Aug 08, 2025 at 09:30:17PM +0200, Andrey Albershteyn wrote: > From: Andrey Albershteyn <aalbersh@redhat.com> > > Utilize new file_getattr/file_setattr syscalls to set project ID on > special files. Previously, special files were skipped due to lack of the > way to call FS_IOC_SETFSXATTR ioctl on them. The quota accounting was > therefore missing these inodes (special files created before project > setup). The ones created after porject initialization did inherit the > projid flag from the parent. > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> > --- > quota/project.c | 144 +++++++++++++++++++++++++++++--------------------------- > 1 file changed, 74 insertions(+), 70 deletions(-) > > diff --git a/quota/project.c b/quota/project.c > index adb26945fa57..93d7ace0e11b 100644 > --- a/quota/project.c > +++ b/quota/project.c > @@ -4,14 +4,17 @@ > * All Rights Reserved. > */ > > +#include <unistd.h> > #include "command.h" > #include "input.h" > #include "init.h" > +#include "libfrog/file_attr.h" > #include "quota.h" > > static cmdinfo_t project_cmd; > static prid_t prid; > static int recurse_depth = -1; > +static int dfd; Ew, global scope variables, can we pass that through to check_project? > enum { > CHECK_PROJECT = 0x1, > @@ -19,13 +22,6 @@ enum { > CLEAR_PROJECT = 0x4, > }; > > -#define EXCLUDED_FILE_TYPES(x) \ > - (S_ISCHR((x)) \ > - || S_ISBLK((x)) \ > - || S_ISFIFO((x)) \ > - || S_ISLNK((x)) \ > - || S_ISSOCK((x))) > - > static void > project_help(void) > { > @@ -85,8 +81,8 @@ check_project( > int flag, > struct FTW *data) > { > - struct fsxattr fsx; > - int fd; > + int error; > + struct file_attr fa = { 0 }; > > if (recurse_depth >= 0 && data->level > recurse_depth) > return 0; > @@ -96,30 +92,30 @@ check_project( > fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); > return 0; > } > - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { > - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); > - return 0; > - } > > - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { > - exitcode = 1; > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > - progname, path, strerror(errno)); > - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > - exitcode = 1; > + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > +#ifndef HAVE_FILE_ATTR > + if (SPECIAL_FILE(stat->st_mode)) { > + fprintf(stderr, _("%s: skipping special file %s\n"), > + progname, path); > + return 0; > + } > +#endif Yeah, file_getattr really ought to return some error code for "not supported" and then this becomes: error = file_getattr(...); if (error && errno == EOPNOTSUPP) { fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); return 0; } if (error) { fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), progname, path, strerror(errno)); exitcode = 1; return 0; } > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > - progname, path, strerror(errno)); > - } else { > - if (fsx.fsx_projid != prid) > - printf(_("%s - project identifier is not set" > - " (inode=%u, tree=%u)\n"), > - path, fsx.fsx_projid, (unsigned int)prid); > - if (!(fsx.fsx_xflags & FS_XFLAG_PROJINHERIT) && S_ISDIR(stat->st_mode)) > - printf(_("%s - project inheritance flag is not set\n"), > - path); > + progname, path, strerror(errno)); > + exitcode = 1; > + return 0; > } > - if (fd != -1) > - close(fd); > + > + if (fa.fa_projid != prid) > + printf(_("%s - project identifier is not set" > + " (inode=%u, tree=%u)\n"), > + path, fa.fa_projid, (unsigned int)prid); > + if (!(fa.fa_xflags & FS_XFLAG_PROJINHERIT) && S_ISDIR(stat->st_mode)) > + printf(_("%s - project inheritance flag is not set\n"), > + path); > + > return 0; > } > > @@ -130,8 +126,8 @@ clear_project( > int flag, > struct FTW *data) > { > - struct fsxattr fsx; > - int fd; > + int error; > + struct file_attr fa; > > if (recurse_depth >= 0 && data->level > recurse_depth) > return 0; > @@ -141,32 +137,31 @@ clear_project( > fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); > return 0; > } > - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { > - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); > - return 0; > - } > > - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { > - exitcode = 1; > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > - progname, path, strerror(errno)); > - return 0; > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx) < 0) { > - exitcode = 1; > + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > +#ifndef HAVE_FILE_ATTR > + if (SPECIAL_FILE(stat->st_mode)) { > + fprintf(stderr, _("%s: skipping special file %s\n"), > + progname, path); > + return 0; > + } > +#endif > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > - progname, path, strerror(errno)); > - close(fd); > + progname, path, strerror(errno)); > + exitcode = 1; > return 0; > } > > - fsx.fsx_projid = 0; > - fsx.fsx_xflags &= ~FS_XFLAG_PROJINHERIT; > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &fsx) < 0) { > - exitcode = 1; > + fa.fa_projid = 0; > + fa.fa_xflags &= ~FS_XFLAG_PROJINHERIT; > + > + error = file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > fprintf(stderr, _("%s: cannot clear project on %s: %s\n"), > progname, path, strerror(errno)); > + exitcode = 1; > } > - close(fd); > return 0; > } > > @@ -177,8 +172,8 @@ setup_project( > int flag, > struct FTW *data) > { > - struct fsxattr fsx; > - int fd; > + struct file_attr fa; > + int error; > > if (recurse_depth >= 0 && data->level > recurse_depth) > return 0; > @@ -188,32 +183,32 @@ setup_project( > fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); > return 0; > } > - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { > - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); > - return 0; > - } > > - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { > - exitcode = 1; > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > - progname, path, strerror(errno)); > - return 0; > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx) < 0) { > - exitcode = 1; > + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > +#ifndef HAVE_FILE_ATTR > + if (SPECIAL_FILE(stat->st_mode)) { > + fprintf(stderr, _("%s: skipping special file %s\n"), > + progname, path); > + return 0; > + } > +#endif > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > - progname, path, strerror(errno)); > - close(fd); > + progname, path, strerror(errno)); > + exitcode = 1; > return 0; > } > > - fsx.fsx_projid = prid; > - fsx.fsx_xflags |= FS_XFLAG_PROJINHERIT; > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &fsx) < 0) { > - exitcode = 1; > + fa.fa_projid = prid; > + if (S_ISDIR(stat->st_mode)) > + fa.fa_xflags |= FS_XFLAG_PROJINHERIT; > + > + error = file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > fprintf(stderr, _("%s: cannot set project on %s: %s\n"), > progname, path, strerror(errno)); > + exitcode = 1; > } > - close(fd); > return 0; > } > > @@ -223,6 +218,13 @@ project_operations( > char *dir, > int type) > { > + dfd = open(dir, O_RDONLY|O_NOCTTY); > + if (dfd < -1) { > + printf(_("Error opening dir %s for project %s...\n"), dir, > + project); > + return; > + } > + > switch (type) { > case CHECK_PROJECT: > printf(_("Checking project %s (path %s)...\n"), project, dir); > @@ -237,6 +239,8 @@ project_operations( > nftw(dir, clear_project, 100, FTW_PHYS|FTW_MOUNT); > break; > } > + > + close(dfd); > } > > static void > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files 2025-08-11 15:07 ` Darrick J. Wong @ 2025-08-11 17:51 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 17:51 UTC (permalink / raw) To: Darrick J. Wong; +Cc: aalbersh, linux-fsdevel, linux-xfs On 2025-08-11 08:07:47, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:30:17PM +0200, Andrey Albershteyn wrote: > > From: Andrey Albershteyn <aalbersh@redhat.com> > > > > Utilize new file_getattr/file_setattr syscalls to set project ID on > > special files. Previously, special files were skipped due to lack of the > > way to call FS_IOC_SETFSXATTR ioctl on them. The quota accounting was > > therefore missing these inodes (special files created before project > > setup). The ones created after porject initialization did inherit the > > projid flag from the parent. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> > > --- > > quota/project.c | 144 +++++++++++++++++++++++++++++--------------------------- > > 1 file changed, 74 insertions(+), 70 deletions(-) > > > > diff --git a/quota/project.c b/quota/project.c > > index adb26945fa57..93d7ace0e11b 100644 > > --- a/quota/project.c > > +++ b/quota/project.c > > @@ -4,14 +4,17 @@ > > * All Rights Reserved. > > */ > > > > +#include <unistd.h> > > #include "command.h" > > #include "input.h" > > #include "init.h" > > +#include "libfrog/file_attr.h" > > #include "quota.h" > > > > static cmdinfo_t project_cmd; > > static prid_t prid; > > static int recurse_depth = -1; > > +static int dfd; > > Ew, global scope variables, can we pass that through to check_project? it's used in all ops (setup, check and clear), this is opened in project_operations() which then calls nftw which doesn't seem to have any context for callbacks. So, I think in anyway we will need some global state. > > > enum { > > CHECK_PROJECT = 0x1, > > @@ -19,13 +22,6 @@ enum { > > CLEAR_PROJECT = 0x4, > > }; > > > > -#define EXCLUDED_FILE_TYPES(x) \ > > - (S_ISCHR((x)) \ > > - || S_ISBLK((x)) \ > > - || S_ISFIFO((x)) \ > > - || S_ISLNK((x)) \ > > - || S_ISSOCK((x))) > > - > > static void > > project_help(void) > > { > > @@ -85,8 +81,8 @@ check_project( > > int flag, > > struct FTW *data) > > { > > - struct fsxattr fsx; > > - int fd; > > + int error; > > + struct file_attr fa = { 0 }; > > > > if (recurse_depth >= 0 && data->level > recurse_depth) > > return 0; > > @@ -96,30 +92,30 @@ check_project( > > fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); > > return 0; > > } > > - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { > > - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); > > - return 0; > > - } > > > > - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { > > - exitcode = 1; > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > - progname, path, strerror(errno)); > > - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > > - exitcode = 1; > > + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > +#ifndef HAVE_FILE_ATTR > > + if (SPECIAL_FILE(stat->st_mode)) { > > + fprintf(stderr, _("%s: skipping special file %s\n"), > > + progname, path); > > + return 0; > > + } > > +#endif > > Yeah, file_getattr really ought to return some error code for "not > supported" and then this becomes: > > error = file_getattr(...); > if (error && errno == EOPNOTSUPP) { > fprintf(stderr, _("%s: skipping special file %s\n"), > progname, path); > return 0; > } > if (error) { > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > progname, path, strerror(errno)); > exitcode = 1; > return 0; > } Sure, that's better, will redo file_getattr() > > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > - progname, path, strerror(errno)); > > - } else { > > - if (fsx.fsx_projid != prid) > > - printf(_("%s - project identifier is not set" > > - " (inode=%u, tree=%u)\n"), > > - path, fsx.fsx_projid, (unsigned int)prid); > > - if (!(fsx.fsx_xflags & FS_XFLAG_PROJINHERIT) && S_ISDIR(stat->st_mode)) > > - printf(_("%s - project inheritance flag is not set\n"), > > - path); > > + progname, path, strerror(errno)); > > + exitcode = 1; > > + return 0; > > } > > - if (fd != -1) > > - close(fd); > > + > > + if (fa.fa_projid != prid) > > + printf(_("%s - project identifier is not set" > > + " (inode=%u, tree=%u)\n"), > > + path, fa.fa_projid, (unsigned int)prid); > > + if (!(fa.fa_xflags & FS_XFLAG_PROJINHERIT) && S_ISDIR(stat->st_mode)) > > + printf(_("%s - project inheritance flag is not set\n"), > > + path); > > + > > return 0; > > } > > > > @@ -130,8 +126,8 @@ clear_project( > > int flag, > > struct FTW *data) > > { > > - struct fsxattr fsx; > > - int fd; > > + int error; > > + struct file_attr fa; > > > > if (recurse_depth >= 0 && data->level > recurse_depth) > > return 0; > > @@ -141,32 +137,31 @@ clear_project( > > fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); > > return 0; > > } > > - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { > > - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); > > - return 0; > > - } > > > > - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { > > - exitcode = 1; > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > - progname, path, strerror(errno)); > > - return 0; > > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx) < 0) { > > - exitcode = 1; > > + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > +#ifndef HAVE_FILE_ATTR > > + if (SPECIAL_FILE(stat->st_mode)) { > > + fprintf(stderr, _("%s: skipping special file %s\n"), > > + progname, path); > > + return 0; > > + } > > +#endif > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > - progname, path, strerror(errno)); > > - close(fd); > > + progname, path, strerror(errno)); > > + exitcode = 1; > > return 0; > > } > > > > - fsx.fsx_projid = 0; > > - fsx.fsx_xflags &= ~FS_XFLAG_PROJINHERIT; > > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &fsx) < 0) { > > - exitcode = 1; > > + fa.fa_projid = 0; > > + fa.fa_xflags &= ~FS_XFLAG_PROJINHERIT; > > + > > + error = file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > fprintf(stderr, _("%s: cannot clear project on %s: %s\n"), > > progname, path, strerror(errno)); > > + exitcode = 1; > > } > > - close(fd); > > return 0; > > } > > > > @@ -177,8 +172,8 @@ setup_project( > > int flag, > > struct FTW *data) > > { > > - struct fsxattr fsx; > > - int fd; > > + struct file_attr fa; > > + int error; > > > > if (recurse_depth >= 0 && data->level > recurse_depth) > > return 0; > > @@ -188,32 +183,32 @@ setup_project( > > fprintf(stderr, _("%s: cannot stat file %s\n"), progname, path); > > return 0; > > } > > - if (EXCLUDED_FILE_TYPES(stat->st_mode)) { > > - fprintf(stderr, _("%s: skipping special file %s\n"), progname, path); > > - return 0; > > - } > > > > - if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) { > > - exitcode = 1; > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > - progname, path, strerror(errno)); > > - return 0; > > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx) < 0) { > > - exitcode = 1; > > + error = file_getattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > +#ifndef HAVE_FILE_ATTR > > + if (SPECIAL_FILE(stat->st_mode)) { > > + fprintf(stderr, _("%s: skipping special file %s\n"), > > + progname, path); > > + return 0; > > + } > > +#endif > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > - progname, path, strerror(errno)); > > - close(fd); > > + progname, path, strerror(errno)); > > + exitcode = 1; > > return 0; > > } > > > > - fsx.fsx_projid = prid; > > - fsx.fsx_xflags |= FS_XFLAG_PROJINHERIT; > > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &fsx) < 0) { > > - exitcode = 1; > > + fa.fa_projid = prid; > > + if (S_ISDIR(stat->st_mode)) > > + fa.fa_xflags |= FS_XFLAG_PROJINHERIT; > > + > > + error = file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > fprintf(stderr, _("%s: cannot set project on %s: %s\n"), > > progname, path, strerror(errno)); > > + exitcode = 1; > > } > > - close(fd); > > return 0; > > } > > > > @@ -223,6 +218,13 @@ project_operations( > > char *dir, > > int type) > > { > > + dfd = open(dir, O_RDONLY|O_NOCTTY); > > + if (dfd < -1) { > > + printf(_("Error opening dir %s for project %s...\n"), dir, > > + project); > > + return; > > + } > > + > > switch (type) { > > case CHECK_PROJECT: > > printf(_("Checking project %s (path %s)...\n"), project, dir); > > @@ -237,6 +239,8 @@ project_operations( > > nftw(dir, clear_project, 100, FTW_PHYS|FTW_MOUNT); > > break; > > } > > + > > + close(dfd); > > } > > > > static void > > > > -- > > 2.49.0 > > > > > -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/4] xfs_io: make ls/chattr work with special files 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files Andrey Albershteyn @ 2025-08-08 19:30 ` Andrey Albershteyn 2025-08-11 15:12 ` Darrick J. Wong 2025-08-08 19:30 ` [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump Andrey Albershteyn 3 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:30 UTC (permalink / raw) To: aalbersh, linux-fsdevel, linux-xfs; +Cc: Andrey Albershteyn From: Andrey Albershteyn <aalbersh@redhat.com> With new file_getattr/file_setattr syscalls we can now list/change file attributes on special files instead for ignoring them. Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- io/attr.c | 130 ++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 75 insertions(+), 55 deletions(-) diff --git a/io/attr.c b/io/attr.c index fd82a2e73801..1cce602074f4 100644 --- a/io/attr.c +++ b/io/attr.c @@ -8,6 +8,7 @@ #include "input.h" #include "init.h" #include "io.h" +#include "libfrog/file_attr.h" static cmdinfo_t chattr_cmd; static cmdinfo_t lsattr_cmd; @@ -156,36 +157,35 @@ lsattr_callback( int status, struct FTW *data) { - struct fsxattr fsx; - int fd; + struct file_attr fa; + int error; if (recurse_dir && !S_ISDIR(stat->st_mode)) return 0; - if ((fd = open(path, O_RDONLY)) == -1) { - fprintf(stderr, _("%s: cannot open %s: %s\n"), - progname, path, strerror(errno)); - exitcode = 1; - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { + error = file_getattr(AT_FDCWD, path, stat, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), progname, path, strerror(errno)); exitcode = 1; - } else - printxattr(fsx.fsx_xflags, 0, 1, path, 0, 1); + return 0; + } + + printxattr(fa.fa_xflags, 0, 1, path, 0, 1); - if (fd != -1) - close(fd); return 0; } static int lsattr_f( - int argc, - char **argv) + int argc, + char **argv) { - struct fsxattr fsx; - char *name = file->name; - int c, aflag = 0, vflag = 0; + struct file_attr fa; + char *name = file->name; + int c, aflag = 0, vflag = 0; + struct stat st; + int error; recurse_all = recurse_dir = 0; while ((c = getopt(argc, argv, "DRav")) != EOF) { @@ -211,17 +211,27 @@ lsattr_f( if (recurse_all || recurse_dir) { nftw(name, lsattr_callback, 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); - } else if ((xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { + return 0; + } + + error = stat(name, &st); + if (error) + return error; + + error = file_getattr(AT_FDCWD, name, &st, &fa, AT_SYMLINK_NOFOLLOW); + if (error) { fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), progname, name, strerror(errno)); exitcode = 1; - } else { - printxattr(fsx.fsx_xflags, vflag, !aflag, name, vflag, !aflag); - if (aflag) { - fputs("/", stdout); - printxattr(-1, 0, 1, name, 0, 1); - } + return 0; } + + printxattr(fa.fa_xflags, vflag, !aflag, name, vflag, !aflag); + if (aflag) { + fputs("/", stdout); + printxattr(-1, 0, 1, name, 0, 1); + } + return 0; } @@ -232,44 +242,43 @@ chattr_callback( int status, struct FTW *data) { - struct fsxattr attr; - int fd; + struct file_attr attr; + int error; if (recurse_dir && !S_ISDIR(stat->st_mode)) return 0; - if ((fd = open(path, O_RDONLY)) == -1) { - fprintf(stderr, _("%s: cannot open %s: %s\n"), - progname, path, strerror(errno)); - exitcode = 1; - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &attr) < 0) { + error = file_getattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); + if (error) { fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), progname, path, strerror(errno)); exitcode = 1; - } else { - attr.fsx_xflags |= orflags; - attr.fsx_xflags &= ~andflags; - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &attr) < 0) { - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), - progname, path, strerror(errno)); - exitcode = 1; - } + return 0; + } + + attr.fa_xflags |= orflags; + attr.fa_xflags &= ~andflags; + error = file_setattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); + if (error) { + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), + progname, path, strerror(errno)); + exitcode = 1; } - if (fd != -1) - close(fd); return 0; } static int chattr_f( - int argc, - char **argv) + int argc, + char **argv) { - struct fsxattr attr; - struct xflags *p; - unsigned int i = 0; - char *c, *name = file->name; + struct file_attr attr; + struct xflags *p; + unsigned int i = 0; + char *c, *name = file->name; + struct stat st; + int error; orflags = andflags = 0; recurse_all = recurse_dir = 0; @@ -326,19 +335,30 @@ chattr_f( if (recurse_all || recurse_dir) { nftw(name, chattr_callback, 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); - } else if (xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &attr) < 0) { + return 0; + } + + error = stat(name, &st); + if (error) + return error; + + error = file_getattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); + if (error) { fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), progname, name, strerror(errno)); exitcode = 1; - } else { - attr.fsx_xflags |= orflags; - attr.fsx_xflags &= ~andflags; - if (xfsctl(name, file->fd, FS_IOC_FSSETXATTR, &attr) < 0) { - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), - progname, name, strerror(errno)); - exitcode = 1; - } + return 0; } + + attr.fa_xflags |= orflags; + attr.fa_xflags &= ~andflags; + error = file_setattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); + if (error) { + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), + progname, name, strerror(errno)); + exitcode = 1; + } + return 0; } -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4] xfs_io: make ls/chattr work with special files 2025-08-08 19:30 ` [PATCH 3/4] xfs_io: make ls/chattr work with " Andrey Albershteyn @ 2025-08-11 15:12 ` Darrick J. Wong 2025-08-11 17:57 ` Andrey Albershteyn 0 siblings, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:12 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: aalbersh, linux-fsdevel, linux-xfs On Fri, Aug 08, 2025 at 09:30:18PM +0200, Andrey Albershteyn wrote: > From: Andrey Albershteyn <aalbersh@redhat.com> > > With new file_getattr/file_setattr syscalls we can now list/change file > attributes on special files instead for ignoring them. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > io/attr.c | 130 ++++++++++++++++++++++++++++++++++++-------------------------- > 1 file changed, 75 insertions(+), 55 deletions(-) > > diff --git a/io/attr.c b/io/attr.c > index fd82a2e73801..1cce602074f4 100644 > --- a/io/attr.c > +++ b/io/attr.c > @@ -8,6 +8,7 @@ > #include "input.h" > #include "init.h" > #include "io.h" > +#include "libfrog/file_attr.h" > > static cmdinfo_t chattr_cmd; > static cmdinfo_t lsattr_cmd; > @@ -156,36 +157,35 @@ lsattr_callback( > int status, > struct FTW *data) > { > - struct fsxattr fsx; > - int fd; > + struct file_attr fa; > + int error; > > if (recurse_dir && !S_ISDIR(stat->st_mode)) > return 0; > > - if ((fd = open(path, O_RDONLY)) == -1) { > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > - progname, path, strerror(errno)); > - exitcode = 1; > - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > + error = file_getattr(AT_FDCWD, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > progname, path, strerror(errno)); > exitcode = 1; > - } else > - printxattr(fsx.fsx_xflags, 0, 1, path, 0, 1); > + return 0; > + } > + > + printxattr(fa.fa_xflags, 0, 1, path, 0, 1); Maybe it's time to rename this printxflags or something that's less easily confused with extended attributes... > > - if (fd != -1) > - close(fd); > return 0; > } > > static int > lsattr_f( > - int argc, > - char **argv) > + int argc, > + char **argv) > { > - struct fsxattr fsx; > - char *name = file->name; > - int c, aflag = 0, vflag = 0; > + struct file_attr fa; > + char *name = file->name; > + int c, aflag = 0, vflag = 0; > + struct stat st; > + int error; > > recurse_all = recurse_dir = 0; > while ((c = getopt(argc, argv, "DRav")) != EOF) { > @@ -211,17 +211,27 @@ lsattr_f( > if (recurse_all || recurse_dir) { > nftw(name, lsattr_callback, > 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); > - } else if ((xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > + return 0; > + } > + > + error = stat(name, &st); > + if (error) > + return error; > + > + error = file_getattr(AT_FDCWD, name, &st, &fa, AT_SYMLINK_NOFOLLOW); > + if (error) { > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > progname, name, strerror(errno)); > exitcode = 1; > - } else { > - printxattr(fsx.fsx_xflags, vflag, !aflag, name, vflag, !aflag); > - if (aflag) { > - fputs("/", stdout); > - printxattr(-1, 0, 1, name, 0, 1); > - } > + return 0; > } > + > + printxattr(fa.fa_xflags, vflag, !aflag, name, vflag, !aflag); > + if (aflag) { > + fputs("/", stdout); > + printxattr(-1, 0, 1, name, 0, 1); > + } > + > return 0; > } > > @@ -232,44 +242,43 @@ chattr_callback( > int status, > struct FTW *data) > { > - struct fsxattr attr; > - int fd; > + struct file_attr attr; > + int error; > > if (recurse_dir && !S_ISDIR(stat->st_mode)) > return 0; > > - if ((fd = open(path, O_RDONLY)) == -1) { > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > - progname, path, strerror(errno)); > - exitcode = 1; > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &attr) < 0) { > + error = file_getattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); > + if (error) { > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > progname, path, strerror(errno)); > exitcode = 1; > - } else { > - attr.fsx_xflags |= orflags; > - attr.fsx_xflags &= ~andflags; > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &attr) < 0) { > - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > - progname, path, strerror(errno)); > - exitcode = 1; > - } > + return 0; > + } > + > + attr.fa_xflags |= orflags; > + attr.fa_xflags &= ~andflags; > + error = file_setattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); > + if (error) { > + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > + progname, path, strerror(errno)); > + exitcode = 1; > } > > - if (fd != -1) > - close(fd); > return 0; > } > > static int > chattr_f( > - int argc, > - char **argv) > + int argc, > + char **argv) > { > - struct fsxattr attr; > - struct xflags *p; > - unsigned int i = 0; > - char *c, *name = file->name; > + struct file_attr attr; > + struct xflags *p; > + unsigned int i = 0; > + char *c, *name = file->name; > + struct stat st; > + int error; > > orflags = andflags = 0; > recurse_all = recurse_dir = 0; > @@ -326,19 +335,30 @@ chattr_f( > if (recurse_all || recurse_dir) { > nftw(name, chattr_callback, > 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); > - } else if (xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &attr) < 0) { > + return 0; > + } > + > + error = stat(name, &st); > + if (error) > + return error; > + > + error = file_getattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); > + if (error) { > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > progname, name, strerror(errno)); > exitcode = 1; > - } else { > - attr.fsx_xflags |= orflags; > - attr.fsx_xflags &= ~andflags; > - if (xfsctl(name, file->fd, FS_IOC_FSSETXATTR, &attr) < 0) { > - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > - progname, name, strerror(errno)); > - exitcode = 1; > - } > + return 0; > } > + > + attr.fa_xflags |= orflags; > + attr.fa_xflags &= ~andflags; > + error = file_setattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); For my own curiosity, if you wanted to do the get/set sequence on a file that's already open, is that just: file_getattr(fd, "", &attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); ... file_setattr(fd, "", &attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); --D > + if (error) { > + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > + progname, name, strerror(errno)); > + exitcode = 1; > + } > + > return 0; > } > > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4] xfs_io: make ls/chattr work with special files 2025-08-11 15:12 ` Darrick J. Wong @ 2025-08-11 17:57 ` Andrey Albershteyn 2025-08-12 17:28 ` Darrick J. Wong 0 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 17:57 UTC (permalink / raw) To: Darrick J. Wong; +Cc: aalbersh, linux-fsdevel, linux-xfs On 2025-08-11 08:12:17, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:30:18PM +0200, Andrey Albershteyn wrote: > > From: Andrey Albershteyn <aalbersh@redhat.com> > > > > With new file_getattr/file_setattr syscalls we can now list/change file > > attributes on special files instead for ignoring them. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > io/attr.c | 130 ++++++++++++++++++++++++++++++++++++-------------------------- > > 1 file changed, 75 insertions(+), 55 deletions(-) > > > > diff --git a/io/attr.c b/io/attr.c > > index fd82a2e73801..1cce602074f4 100644 > > --- a/io/attr.c > > +++ b/io/attr.c > > @@ -8,6 +8,7 @@ > > #include "input.h" > > #include "init.h" > > #include "io.h" > > +#include "libfrog/file_attr.h" > > > > static cmdinfo_t chattr_cmd; > > static cmdinfo_t lsattr_cmd; > > @@ -156,36 +157,35 @@ lsattr_callback( > > int status, > > struct FTW *data) > > { > > - struct fsxattr fsx; > > - int fd; > > + struct file_attr fa; > > + int error; > > > > if (recurse_dir && !S_ISDIR(stat->st_mode)) > > return 0; > > > > - if ((fd = open(path, O_RDONLY)) == -1) { > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > - progname, path, strerror(errno)); > > - exitcode = 1; > > - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > > + error = file_getattr(AT_FDCWD, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > progname, path, strerror(errno)); > > exitcode = 1; > > - } else > > - printxattr(fsx.fsx_xflags, 0, 1, path, 0, 1); > > + return 0; > > + } > > + > > + printxattr(fa.fa_xflags, 0, 1, path, 0, 1); > > Maybe it's time to rename this printxflags or something that's less > easily confused with extended attributes... print_xflags()? > > > > > - if (fd != -1) > > - close(fd); > > return 0; > > } > > > > static int > > lsattr_f( > > - int argc, > > - char **argv) > > + int argc, > > + char **argv) > > { > > - struct fsxattr fsx; > > - char *name = file->name; > > - int c, aflag = 0, vflag = 0; > > + struct file_attr fa; > > + char *name = file->name; > > + int c, aflag = 0, vflag = 0; > > + struct stat st; > > + int error; > > > > recurse_all = recurse_dir = 0; > > while ((c = getopt(argc, argv, "DRav")) != EOF) { > > @@ -211,17 +211,27 @@ lsattr_f( > > if (recurse_all || recurse_dir) { > > nftw(name, lsattr_callback, > > 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); > > - } else if ((xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > > + return 0; > > + } > > + > > + error = stat(name, &st); > > + if (error) > > + return error; > > + > > + error = file_getattr(AT_FDCWD, name, &st, &fa, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > progname, name, strerror(errno)); > > exitcode = 1; > > - } else { > > - printxattr(fsx.fsx_xflags, vflag, !aflag, name, vflag, !aflag); > > - if (aflag) { > > - fputs("/", stdout); > > - printxattr(-1, 0, 1, name, 0, 1); > > - } > > + return 0; > > } > > + > > + printxattr(fa.fa_xflags, vflag, !aflag, name, vflag, !aflag); > > + if (aflag) { > > + fputs("/", stdout); > > + printxattr(-1, 0, 1, name, 0, 1); > > + } > > + > > return 0; > > } > > > > @@ -232,44 +242,43 @@ chattr_callback( > > int status, > > struct FTW *data) > > { > > - struct fsxattr attr; > > - int fd; > > + struct file_attr attr; > > + int error; > > > > if (recurse_dir && !S_ISDIR(stat->st_mode)) > > return 0; > > > > - if ((fd = open(path, O_RDONLY)) == -1) { > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > - progname, path, strerror(errno)); > > - exitcode = 1; > > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &attr) < 0) { > > + error = file_getattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > progname, path, strerror(errno)); > > exitcode = 1; > > - } else { > > - attr.fsx_xflags |= orflags; > > - attr.fsx_xflags &= ~andflags; > > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &attr) < 0) { > > - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > - progname, path, strerror(errno)); > > - exitcode = 1; > > - } > > + return 0; > > + } > > + > > + attr.fa_xflags |= orflags; > > + attr.fa_xflags &= ~andflags; > > + error = file_setattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > + progname, path, strerror(errno)); > > + exitcode = 1; > > } > > > > - if (fd != -1) > > - close(fd); > > return 0; > > } > > > > static int > > chattr_f( > > - int argc, > > - char **argv) > > + int argc, > > + char **argv) > > { > > - struct fsxattr attr; > > - struct xflags *p; > > - unsigned int i = 0; > > - char *c, *name = file->name; > > + struct file_attr attr; > > + struct xflags *p; > > + unsigned int i = 0; > > + char *c, *name = file->name; > > + struct stat st; > > + int error; > > > > orflags = andflags = 0; > > recurse_all = recurse_dir = 0; > > @@ -326,19 +335,30 @@ chattr_f( > > if (recurse_all || recurse_dir) { > > nftw(name, chattr_callback, > > 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); > > - } else if (xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &attr) < 0) { > > + return 0; > > + } > > + > > + error = stat(name, &st); > > + if (error) > > + return error; > > + > > + error = file_getattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); > > + if (error) { > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > progname, name, strerror(errno)); > > exitcode = 1; > > - } else { > > - attr.fsx_xflags |= orflags; > > - attr.fsx_xflags &= ~andflags; > > - if (xfsctl(name, file->fd, FS_IOC_FSSETXATTR, &attr) < 0) { > > - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > - progname, name, strerror(errno)); > > - exitcode = 1; > > - } > > + return 0; > > } > > + > > + attr.fa_xflags |= orflags; > > + attr.fa_xflags &= ~andflags; > > + error = file_setattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); > > For my own curiosity, if you wanted to do the get/set sequence on a file > that's already open, is that just: > > file_getattr(fd, "", &attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); > ... > file_setattr(fd, "", &attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); yeah, that should work > > --D > > > + if (error) { > > + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > + progname, name, strerror(errno)); > > + exitcode = 1; > > + } > > + > > return 0; > > } > > > > > > -- > > 2.49.0 > > > > > -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4] xfs_io: make ls/chattr work with special files 2025-08-11 17:57 ` Andrey Albershteyn @ 2025-08-12 17:28 ` Darrick J. Wong 0 siblings, 0 replies; 33+ messages in thread From: Darrick J. Wong @ 2025-08-12 17:28 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: aalbersh, linux-fsdevel, linux-xfs On Mon, Aug 11, 2025 at 07:57:20PM +0200, Andrey Albershteyn wrote: > On 2025-08-11 08:12:17, Darrick J. Wong wrote: > > On Fri, Aug 08, 2025 at 09:30:18PM +0200, Andrey Albershteyn wrote: > > > From: Andrey Albershteyn <aalbersh@redhat.com> > > > > > > With new file_getattr/file_setattr syscalls we can now list/change file > > > attributes on special files instead for ignoring them. > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > > --- > > > io/attr.c | 130 ++++++++++++++++++++++++++++++++++++-------------------------- > > > 1 file changed, 75 insertions(+), 55 deletions(-) > > > > > > diff --git a/io/attr.c b/io/attr.c > > > index fd82a2e73801..1cce602074f4 100644 > > > --- a/io/attr.c > > > +++ b/io/attr.c > > > @@ -8,6 +8,7 @@ > > > #include "input.h" > > > #include "init.h" > > > #include "io.h" > > > +#include "libfrog/file_attr.h" > > > > > > static cmdinfo_t chattr_cmd; > > > static cmdinfo_t lsattr_cmd; > > > @@ -156,36 +157,35 @@ lsattr_callback( > > > int status, > > > struct FTW *data) > > > { > > > - struct fsxattr fsx; > > > - int fd; > > > + struct file_attr fa; > > > + int error; > > > > > > if (recurse_dir && !S_ISDIR(stat->st_mode)) > > > return 0; > > > > > > - if ((fd = open(path, O_RDONLY)) == -1) { > > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > > - progname, path, strerror(errno)); > > > - exitcode = 1; > > > - } else if ((xfsctl(path, fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > > > + error = file_getattr(AT_FDCWD, path, stat, &fa, AT_SYMLINK_NOFOLLOW); > > > + if (error) { > > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > > progname, path, strerror(errno)); > > > exitcode = 1; > > > - } else > > > - printxattr(fsx.fsx_xflags, 0, 1, path, 0, 1); > > > + return 0; > > > + } > > > + > > > + printxattr(fa.fa_xflags, 0, 1, path, 0, 1); > > > > Maybe it's time to rename this printxflags or something that's less > > easily confused with extended attributes... > > print_xflags()? Sounds good to me. > > > > > > > > - if (fd != -1) > > > - close(fd); > > > return 0; > > > } > > > > > > static int > > > lsattr_f( > > > - int argc, > > > - char **argv) > > > + int argc, > > > + char **argv) > > > { > > > - struct fsxattr fsx; > > > - char *name = file->name; > > > - int c, aflag = 0, vflag = 0; > > > + struct file_attr fa; > > > + char *name = file->name; > > > + int c, aflag = 0, vflag = 0; > > > + struct stat st; > > > + int error; > > > > > > recurse_all = recurse_dir = 0; > > > while ((c = getopt(argc, argv, "DRav")) != EOF) { > > > @@ -211,17 +211,27 @@ lsattr_f( > > > if (recurse_all || recurse_dir) { > > > nftw(name, lsattr_callback, > > > 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); > > > - } else if ((xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) { > > > + return 0; > > > + } > > > + > > > + error = stat(name, &st); > > > + if (error) > > > + return error; > > > + > > > + error = file_getattr(AT_FDCWD, name, &st, &fa, AT_SYMLINK_NOFOLLOW); > > > + if (error) { > > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > > progname, name, strerror(errno)); > > > exitcode = 1; > > > - } else { > > > - printxattr(fsx.fsx_xflags, vflag, !aflag, name, vflag, !aflag); > > > - if (aflag) { > > > - fputs("/", stdout); > > > - printxattr(-1, 0, 1, name, 0, 1); > > > - } > > > + return 0; > > > } > > > + > > > + printxattr(fa.fa_xflags, vflag, !aflag, name, vflag, !aflag); > > > + if (aflag) { > > > + fputs("/", stdout); > > > + printxattr(-1, 0, 1, name, 0, 1); > > > + } > > > + > > > return 0; > > > } > > > > > > @@ -232,44 +242,43 @@ chattr_callback( > > > int status, > > > struct FTW *data) > > > { > > > - struct fsxattr attr; > > > - int fd; > > > + struct file_attr attr; > > > + int error; > > > > > > if (recurse_dir && !S_ISDIR(stat->st_mode)) > > > return 0; > > > > > > - if ((fd = open(path, O_RDONLY)) == -1) { > > > - fprintf(stderr, _("%s: cannot open %s: %s\n"), > > > - progname, path, strerror(errno)); > > > - exitcode = 1; > > > - } else if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &attr) < 0) { > > > + error = file_getattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); > > > + if (error) { > > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > > progname, path, strerror(errno)); > > > exitcode = 1; > > > - } else { > > > - attr.fsx_xflags |= orflags; > > > - attr.fsx_xflags &= ~andflags; > > > - if (xfsctl(path, fd, FS_IOC_FSSETXATTR, &attr) < 0) { > > > - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > > - progname, path, strerror(errno)); > > > - exitcode = 1; > > > - } > > > + return 0; > > > + } > > > + > > > + attr.fa_xflags |= orflags; > > > + attr.fa_xflags &= ~andflags; > > > + error = file_setattr(AT_FDCWD, path, stat, &attr, AT_SYMLINK_NOFOLLOW); > > > + if (error) { > > > + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > > + progname, path, strerror(errno)); > > > + exitcode = 1; > > > } > > > > > > - if (fd != -1) > > > - close(fd); > > > return 0; > > > } > > > > > > static int > > > chattr_f( > > > - int argc, > > > - char **argv) > > > + int argc, > > > + char **argv) > > > { > > > - struct fsxattr attr; > > > - struct xflags *p; > > > - unsigned int i = 0; > > > - char *c, *name = file->name; > > > + struct file_attr attr; > > > + struct xflags *p; > > > + unsigned int i = 0; > > > + char *c, *name = file->name; > > > + struct stat st; > > > + int error; > > > > > > orflags = andflags = 0; > > > recurse_all = recurse_dir = 0; > > > @@ -326,19 +335,30 @@ chattr_f( > > > if (recurse_all || recurse_dir) { > > > nftw(name, chattr_callback, > > > 100, FTW_PHYS | FTW_MOUNT | FTW_DEPTH); > > > - } else if (xfsctl(name, file->fd, FS_IOC_FSGETXATTR, &attr) < 0) { > > > + return 0; > > > + } > > > + > > > + error = stat(name, &st); > > > + if (error) > > > + return error; > > > + > > > + error = file_getattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); > > > + if (error) { > > > fprintf(stderr, _("%s: cannot get flags on %s: %s\n"), > > > progname, name, strerror(errno)); > > > exitcode = 1; > > > - } else { > > > - attr.fsx_xflags |= orflags; > > > - attr.fsx_xflags &= ~andflags; > > > - if (xfsctl(name, file->fd, FS_IOC_FSSETXATTR, &attr) < 0) { > > > - fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > > - progname, name, strerror(errno)); > > > - exitcode = 1; > > > - } > > > + return 0; > > > } > > > + > > > + attr.fa_xflags |= orflags; > > > + attr.fa_xflags &= ~andflags; > > > + error = file_setattr(AT_FDCWD, name, &st, &attr, AT_SYMLINK_NOFOLLOW); > > > > For my own curiosity, if you wanted to do the get/set sequence on a file > > that's already open, is that just: > > > > file_getattr(fd, "", &attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); > > ... > > file_setattr(fd, "", &attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW); > > yeah, that should work Cool, thanks for confirming my understanding. :) --D > > > > --D > > > > > + if (error) { > > > + fprintf(stderr, _("%s: cannot set flags on %s: %s\n"), > > > + progname, name, strerror(errno)); > > > + exitcode = 1; > > > + } > > > + > > > return 0; > > > } > > > > > > > > > -- > > > 2.49.0 > > > > > > > > > > -- > - Andrey > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn ` (2 preceding siblings ...) 2025-08-08 19:30 ` [PATCH 3/4] xfs_io: make ls/chattr work with " Andrey Albershteyn @ 2025-08-08 19:30 ` Andrey Albershteyn 2025-08-11 15:14 ` Darrick J. Wong 3 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:30 UTC (permalink / raw) To: aalbersh, linux-fsdevel, linux-xfs rdump just skipped file attributes on special files as copying wasn't possible. Let's use new file_getattr/file_setattr syscalls to copy attributes even for special files. Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- db/rdump.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/db/rdump.c b/db/rdump.c index 9ff833553ccb..5b9458e6bc94 100644 --- a/db/rdump.c +++ b/db/rdump.c @@ -17,6 +17,7 @@ #include "field.h" #include "inode.h" #include "listxattr.h" +#include "libfrog/file_attr.h" #include <sys/xattr.h> #include <linux/xattr.h> @@ -152,10 +153,17 @@ rdump_fileattrs_path( const struct destdir *destdir, const struct pathbuf *pbuf) { + struct file_attr fa = { + .fa_extsize = ip->i_extsize, + .fa_projid = ip->i_projid, + .fa_cowextsize = ip->i_cowextsize, + .fa_xflags = xfs_ip2xflags(ip), + }; int ret; + int at_flags = AT_SYMLINK_NOFOLLOW; ret = fchmodat(destdir->fd, pbuf->path, VFS_I(ip)->i_mode & ~S_IFMT, - AT_SYMLINK_NOFOLLOW); + at_flags); if (ret) { /* fchmodat on a symlink is not supported */ if (errno == EPERM || errno == EOPNOTSUPP) @@ -169,7 +177,7 @@ rdump_fileattrs_path( } ret = fchownat(destdir->fd, pbuf->path, i_uid_read(VFS_I(ip)), - i_gid_read(VFS_I(ip)), AT_SYMLINK_NOFOLLOW); + i_gid_read(VFS_I(ip)), at_flags); if (ret) { if (errno == EPERM) lost_mask |= LOST_OWNER; @@ -181,7 +189,17 @@ rdump_fileattrs_path( return 1; } - /* Cannot copy fsxattrs until setfsxattrat gets merged */ + ret = file_setattr(destdir->fd, pbuf->path, NULL, &fa, at_flags); + if (ret) { + if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY) + lost_mask |= LOST_FSXATTR; + else + dbprintf(_("%s%s%s: file_setattr %s\n"), destdir->path, + destdir->sep, pbuf->path, + strerror(errno)); + if (strict_errors) + return 1; + } return 0; } -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump 2025-08-08 19:30 ` [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump Andrey Albershteyn @ 2025-08-11 15:14 ` Darrick J. Wong 2025-08-11 17:59 ` Andrey Albershteyn 0 siblings, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:14 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: aalbersh, linux-fsdevel, linux-xfs On Fri, Aug 08, 2025 at 09:30:19PM +0200, Andrey Albershteyn wrote: > rdump just skipped file attributes on special files as copying wasn't > possible. Let's use new file_getattr/file_setattr syscalls to copy > attributes even for special files. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > db/rdump.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/db/rdump.c b/db/rdump.c > index 9ff833553ccb..5b9458e6bc94 100644 > --- a/db/rdump.c > +++ b/db/rdump.c > @@ -17,6 +17,7 @@ > #include "field.h" > #include "inode.h" > #include "listxattr.h" > +#include "libfrog/file_attr.h" > #include <sys/xattr.h> > #include <linux/xattr.h> > > @@ -152,10 +153,17 @@ rdump_fileattrs_path( > const struct destdir *destdir, > const struct pathbuf *pbuf) > { > + struct file_attr fa = { > + .fa_extsize = ip->i_extsize, > + .fa_projid = ip->i_projid, > + .fa_cowextsize = ip->i_cowextsize, > + .fa_xflags = xfs_ip2xflags(ip), > + }; > int ret; > + int at_flags = AT_SYMLINK_NOFOLLOW; Why does this become a mutable variable? AFAICT it doesn't change? Otherwise things look good here. --D > > ret = fchmodat(destdir->fd, pbuf->path, VFS_I(ip)->i_mode & ~S_IFMT, > - AT_SYMLINK_NOFOLLOW); > + at_flags); > if (ret) { > /* fchmodat on a symlink is not supported */ > if (errno == EPERM || errno == EOPNOTSUPP) > @@ -169,7 +177,7 @@ rdump_fileattrs_path( > } > > ret = fchownat(destdir->fd, pbuf->path, i_uid_read(VFS_I(ip)), > - i_gid_read(VFS_I(ip)), AT_SYMLINK_NOFOLLOW); > + i_gid_read(VFS_I(ip)), at_flags); > if (ret) { > if (errno == EPERM) > lost_mask |= LOST_OWNER; > @@ -181,7 +189,17 @@ rdump_fileattrs_path( > return 1; > } > > - /* Cannot copy fsxattrs until setfsxattrat gets merged */ > + ret = file_setattr(destdir->fd, pbuf->path, NULL, &fa, at_flags); > + if (ret) { > + if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY) > + lost_mask |= LOST_FSXATTR; > + else > + dbprintf(_("%s%s%s: file_setattr %s\n"), destdir->path, > + destdir->sep, pbuf->path, > + strerror(errno)); > + if (strict_errors) > + return 1; > + } > > return 0; > } > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump 2025-08-11 15:14 ` Darrick J. Wong @ 2025-08-11 17:59 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 17:59 UTC (permalink / raw) To: Darrick J. Wong; +Cc: aalbersh, linux-fsdevel, linux-xfs On 2025-08-11 08:14:27, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:30:19PM +0200, Andrey Albershteyn wrote: > > rdump just skipped file attributes on special files as copying wasn't > > possible. Let's use new file_getattr/file_setattr syscalls to copy > > attributes even for special files. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > db/rdump.c | 24 +++++++++++++++++++++--- > > 1 file changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/db/rdump.c b/db/rdump.c > > index 9ff833553ccb..5b9458e6bc94 100644 > > --- a/db/rdump.c > > +++ b/db/rdump.c > > @@ -17,6 +17,7 @@ > > #include "field.h" > > #include "inode.h" > > #include "listxattr.h" > > +#include "libfrog/file_attr.h" > > #include <sys/xattr.h> > > #include <linux/xattr.h> > > > > @@ -152,10 +153,17 @@ rdump_fileattrs_path( > > const struct destdir *destdir, > > const struct pathbuf *pbuf) > > { > > + struct file_attr fa = { > > + .fa_extsize = ip->i_extsize, > > + .fa_projid = ip->i_projid, > > + .fa_cowextsize = ip->i_cowextsize, > > + .fa_xflags = xfs_ip2xflags(ip), > > + }; > > int ret; > > + int at_flags = AT_SYMLINK_NOFOLLOW; > > Why does this become a mutable variable? AFAICT it doesn't change? > > Otherwise things look good here. ops, leftover from older version, will pass it in place > > --D > > > > > ret = fchmodat(destdir->fd, pbuf->path, VFS_I(ip)->i_mode & ~S_IFMT, > > - AT_SYMLINK_NOFOLLOW); > > + at_flags); > > if (ret) { > > /* fchmodat on a symlink is not supported */ > > if (errno == EPERM || errno == EOPNOTSUPP) > > @@ -169,7 +177,7 @@ rdump_fileattrs_path( > > } > > > > ret = fchownat(destdir->fd, pbuf->path, i_uid_read(VFS_I(ip)), > > - i_gid_read(VFS_I(ip)), AT_SYMLINK_NOFOLLOW); > > + i_gid_read(VFS_I(ip)), at_flags); > > if (ret) { > > if (errno == EPERM) > > lost_mask |= LOST_OWNER; > > @@ -181,7 +189,17 @@ rdump_fileattrs_path( > > return 1; > > } > > > > - /* Cannot copy fsxattrs until setfsxattrat gets merged */ > > + ret = file_setattr(destdir->fd, pbuf->path, NULL, &fa, at_flags); > > + if (ret) { > > + if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY) > > + lost_mask |= LOST_FSXATTR; > > + else > > + dbprintf(_("%s%s%s: file_setattr %s\n"), destdir->path, > > + destdir->sep, pbuf->path, > > + strerror(errno)); > > + if (strict_errors) > > + return 1; > > + } > > > > return 0; > > } > > > > -- > > 2.49.0 > > > > > -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 0/3] Test file_getattr and file_setattr syscalls 2025-08-08 19:28 Tests for file_getattr()/file_setattr() and xfsprogs update Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn @ 2025-08-08 19:31 ` Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 1/3] file_attr: introduce program to set/get fsxattr Andrey Albershteyn ` (2 more replies) 1 sibling, 3 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:31 UTC (permalink / raw) To: fstests Cc: zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn, Andrey Albershteyn Add a test to check basic functionallity of file_getattr() and file_setattr() syscalls. These syscalls are used to get/set filesystem inode attributes (think of FS_IOC_SETFSXATTR ioctl()). The difference from ioctl() is that these syscalls use *at() semantics and can be called on any file without opening it, including special ones. For XFS, with the use of these syscalls, xfs_quota now can manipulate quota on special files such as sockets. Add a test to check that special files are counted, which wasn't true before. To: fstests@vger.kernel.org Cc: zlang@redhat.com Cc: linux-fsdevel@vger.kernel.org Cc: linux-xfs@vger.kernel.org Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- Andrey Albershteyn (3): file_attr: introduce program to set/get fsxattr generic: introduce test to test file_getattr/file_setattr syscalls xfs: test quota's project ID on special files .gitignore | 1 + configure.ac | 1 + include/builddefs.in | 1 + m4/package_libcdev.m4 | 16 +++ src/Makefile | 5 + src/file_attr.c | 277 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/2000 | 113 ++++++++++++++++++++ tests/generic/2000.out | 37 +++++++ tests/xfs/2000 | 77 ++++++++++++++ tests/xfs/2000.out | 17 +++ 10 files changed, 545 insertions(+) --- base-commit: 2cc8c822f864e272251460e05b0cba5bada0f9ee change-id: 20250317-xattrat-syscall-ee8f5e2d6e18 Best regards, -- Andrey Albershteyn <aalbersh@kernel.org> ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/3] file_attr: introduce program to set/get fsxattr 2025-08-08 19:31 ` [PATCH 0/3] Test file_getattr and file_setattr syscalls Andrey Albershteyn @ 2025-08-08 19:31 ` Andrey Albershteyn 2025-08-11 15:23 ` Darrick J. Wong 2025-08-11 17:51 ` Zorro Lang 2025-08-08 19:31 ` [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 3/3] xfs: test quota's project ID on special files Andrey Albershteyn 2 siblings, 2 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:31 UTC (permalink / raw) To: fstests; +Cc: zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn This programs uses newly introduced file_getattr and file_setattr syscalls. This program is partially a test of invalid options. This will be used further in the test. Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- .gitignore | 1 + configure.ac | 1 + include/builddefs.in | 1 + m4/package_libcdev.m4 | 16 +++ src/Makefile | 5 + src/file_attr.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 301 insertions(+) diff --git a/.gitignore b/.gitignore index 4fd817243dca..1a578eab1ea0 100644 --- a/.gitignore +++ b/.gitignore @@ -210,6 +210,7 @@ tags /src/fiemap-fault /src/min_dio_alignment /src/dio-writeback-race +/src/file_attr # Symlinked files /tests/generic/035.out diff --git a/configure.ac b/configure.ac index f3c8c643f0eb..6fe54e8e1d54 100644 --- a/configure.ac +++ b/configure.ac @@ -73,6 +73,7 @@ AC_HAVE_RLIMIT_NOFILE AC_NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE AC_HAVE_FICLONE AC_HAVE_TRIVIAL_AUTO_VAR_INIT +AC_HAVE_FILE_ATTR AC_CHECK_FUNCS([renameat2]) AC_CHECK_FUNCS([reallocarray]) diff --git a/include/builddefs.in b/include/builddefs.in index 96d5ed25b3e2..821237339cc7 100644 --- a/include/builddefs.in +++ b/include/builddefs.in @@ -74,6 +74,7 @@ HAVE_BMV_OF_SHARED = @have_bmv_of_shared@ HAVE_RLIMIT_NOFILE = @have_rlimit_nofile@ NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE = @need_internal_xfs_ioc_exchange_range@ HAVE_FICLONE = @have_ficlone@ +HAVE_FILE_ATTR = @have_file_attr@ GCCFLAGS = -std=gnu11 -funsigned-char -fno-strict-aliasing -Wall SANITIZER_CFLAGS += @autovar_init_cflags@ diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 index ed8fe6e32ae0..e68a70f7d87e 100644 --- a/m4/package_libcdev.m4 +++ b/m4/package_libcdev.m4 @@ -86,3 +86,19 @@ AC_DEFUN([AC_HAVE_TRIVIAL_AUTO_VAR_INIT], CFLAGS="${OLD_CFLAGS}" AC_SUBST(autovar_init_cflags) ]) + +# +# Check if we have a file_getattr/file_setattr system call (Linux) +# +AC_DEFUN([AC_HAVE_FILE_ATTR], + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) + AC_LINK_IFELSE([AC_LANG_PROGRAM([[ +#define _GNU_SOURCE +#include <sys/syscall.h> +#include <unistd.h> + ]], [[ + syscall(__NR_file_getattr, 0, 0, 0, 0, 0, 0); + ]])],[have_file_attr=yes + AC_MSG_RESULT(yes)],[AC_MSG_RESULT(no)]) + AC_SUBST(have_file_attr) + ]) diff --git a/src/Makefile b/src/Makefile index 6ac72b366257..f3137acf687f 100644 --- a/src/Makefile +++ b/src/Makefile @@ -61,6 +61,11 @@ ifeq ($(HAVE_FALLOCATE), true) LCFLAGS += -DHAVE_FALLOCATE endif +ifeq ($(HAVE_FILE_ATTR), yes) +LINUX_TARGETS += file_attr +LCFLAGS += -DHAVE_FILE_ATTR +endif + ifeq ($(PKG_PLATFORM),linux) TARGETS += $(LINUX_TARGETS) endif diff --git a/src/file_attr.c b/src/file_attr.c new file mode 100644 index 000000000000..9756ab265a57 --- /dev/null +++ b/src/file_attr.c @@ -0,0 +1,277 @@ +#include "global.h" +#include <sys/syscall.h> +#include <getopt.h> +#include <errno.h> +#include <linux/fs.h> +#include <sys/stat.h> +#include <string.h> +#include <getopt.h> +#include <stdlib.h> +#include <unistd.h> + +#ifndef HAVE_FILE_ATTR +#define __NR_file_getattr 468 +#define __NR_file_setattr 469 + +struct file_attr { + __u32 fa_xflags; /* xflags field value (get/set) */ + __u32 fa_extsize; /* extsize field value (get/set)*/ + __u32 fa_nextents; /* nextents field value (get) */ + __u32 fa_projid; /* project identifier (get/set) */ + __u32 fa_cowextsize; /* CoW extsize field value (get/set) */ +}; + +#endif + +#define SPECIAL_FILE(x) \ + (S_ISCHR((x)) \ + || S_ISBLK((x)) \ + || S_ISFIFO((x)) \ + || S_ISLNK((x)) \ + || S_ISSOCK((x))) + +static struct option long_options[] = { + {"set", no_argument, 0, 's' }, + {"get", no_argument, 0, 'g' }, + {"no-follow", no_argument, 0, 'n' }, + {"at-cwd", no_argument, 0, 'a' }, + {"set-nodump", no_argument, 0, 'd' }, + {"invalid-at", no_argument, 0, 'i' }, + {"too-big-arg", no_argument, 0, 'b' }, + {"too-small-arg", no_argument, 0, 'm' }, + {"new-fsx-flag", no_argument, 0, 'x' }, + {0, 0, 0, 0 } +}; + +static struct xflags { + uint flag; + char *shortname; + char *longname; +} xflags[] = { + { FS_XFLAG_REALTIME, "r", "realtime" }, + { FS_XFLAG_PREALLOC, "p", "prealloc" }, + { FS_XFLAG_IMMUTABLE, "i", "immutable" }, + { FS_XFLAG_APPEND, "a", "append-only" }, + { FS_XFLAG_SYNC, "s", "sync" }, + { FS_XFLAG_NOATIME, "A", "no-atime" }, + { FS_XFLAG_NODUMP, "d", "no-dump" }, + { FS_XFLAG_RTINHERIT, "t", "rt-inherit" }, + { FS_XFLAG_PROJINHERIT, "P", "proj-inherit" }, + { FS_XFLAG_NOSYMLINKS, "n", "nosymlinks" }, + { FS_XFLAG_EXTSIZE, "e", "extsize" }, + { FS_XFLAG_EXTSZINHERIT, "E", "extsz-inherit" }, + { FS_XFLAG_NODEFRAG, "f", "no-defrag" }, + { FS_XFLAG_FILESTREAM, "S", "filestream" }, + { FS_XFLAG_DAX, "x", "dax" }, + { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, + { FS_XFLAG_HASATTR, "X", "has-xattr" }, + { 0, NULL, NULL } +}; + +static int +file_getattr( + int dfd, + const char *filename, + struct file_attr *fsx, + size_t usize, + unsigned int at_flags) +{ + return syscall(__NR_file_getattr, dfd, filename, fsx, usize, at_flags); +} + +static int +file_setattr( + int dfd, + const char *filename, + struct file_attr *fsx, + size_t usize, + unsigned int at_flags) +{ + return syscall(__NR_file_setattr, dfd, filename, fsx, usize, at_flags); +} + +void +printxattr( + uint flags, + int verbose, + int dofname, + const char *fname, + int dobraces, + int doeol) +{ + struct xflags *p; + int first = 1; + + if (dobraces) + fputs("[", stdout); + for (p = xflags; p->flag; p++) { + if (flags & p->flag) { + if (verbose) { + if (first) + first = 0; + else + fputs(", ", stdout); + fputs(p->longname, stdout); + } else { + fputs(p->shortname, stdout); + } + } else if (!verbose) { + fputs("-", stdout); + } + } + if (dobraces) + fputs("]", stdout); + if (dofname) + printf(" %s ", fname); + if (doeol) + fputs("\n", stdout); +} + +int main(int argc, char *argv[]) +{ + int error; + int c; + const char *path = NULL; + const char *path1 = NULL; + const char *path2 = NULL; + unsigned int at_flags = 0; + unsigned int fa_xflags = 0; + int action = 0; /* 0 get; 1 set */ + struct file_attr fsx = { }; + int fa_size = sizeof(struct file_attr); + struct stat status; + int fd; + int at_fdcwd = 0; + int unknwon_fa_flag = 0; + + while (1) { + int option_index = 0; + + c = getopt_long_only(argc, argv, "", long_options, &option_index); + if (c == -1) + break; + + switch (c) { + case 's': + action = 1; + break; + case 'g': + action = 0; + break; + case 'n': + at_flags |= AT_SYMLINK_NOFOLLOW; + break; + case 'a': + at_fdcwd = 1; + break; + case 'd': + fa_xflags |= FS_XFLAG_NODUMP; + break; + case 'i': + at_flags |= (1 << 25); + break; + case 'b': + fa_size = getpagesize() + 1; /* max size if page size */ + break; + case 'm': + fa_size = 19; /* VER0 size of fsxattr is 20 */ + break; + case 'x': + unknwon_fa_flag = (1 << 27); + break; + default: + goto usage; + } + } + + if (!path1 && optind < argc) + path1 = argv[optind++]; + if (!path2 && optind < argc) + path2 = argv[optind++]; + + if (at_fdcwd) { + fd = AT_FDCWD; + path = path1; + } else if (!path2) { + error = stat(path1, &status); + if (error) { + fprintf(stderr, +"Can not get file status of %s: %s\n", path1, strerror(errno)); + return error; + } + + if (SPECIAL_FILE(status.st_mode)) { + fprintf(stderr, +"Can not open special file %s without parent dir: %s\n", path1, strerror(errno)); + return errno; + } + + fd = open(path1, O_RDONLY); + if (fd == -1) { + fprintf(stderr, "Can not open %s: %s\n", path1, + strerror(errno)); + return errno; + } + } else { + fd = open(path1, O_RDONLY); + if (fd == -1) { + fprintf(stderr, "Can not open %s: %s\n", path1, + strerror(errno)); + return errno; + } + path = path2; + } + + if (!path) + at_flags |= AT_EMPTY_PATH; + + if (action) { + error = file_getattr(fd, path, &fsx, fa_size, + at_flags); + if (error) { + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, + strerror(errno)); + return error; + } + + fsx.fa_xflags |= (fa_xflags | unknwon_fa_flag); + + error = file_setattr(fd, path, &fsx, fa_size, + at_flags); + if (error) { + fprintf(stderr, "Can not set fsxattr on %s: %s\n", path, + strerror(errno)); + return error; + } + } else { + error = file_getattr(fd, path, &fsx, fa_size, + at_flags); + if (error) { + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, + strerror(errno)); + return error; + } + + if (path2) + printxattr(fsx.fa_xflags, 0, 1, path, 0, 1); + else + printxattr(fsx.fa_xflags, 0, 1, path1, 0, 1); + } + + return error; + +usage: + printf("Usage: %s [options]\n", argv[0]); + printf("Options:\n"); + printf("\t--get\t\tget filesystem inode attributes\n"); + printf("\t--set\t\tset filesystem inode attributes\n"); + printf("\t--at-cwd\t\topen file at current working directory\n"); + printf("\t--no-follow\t\tdon't follow symlinks\n"); + printf("\t--set-nodump\t\tset FS_XFLAG_NODUMP on an inode\n"); + printf("\t--invalid-at\t\tUse invalida AT_* flag\n"); + printf("\t--too-big-arg\t\tSet fsxattr size bigger than PAGE_SIZE\n"); + printf("\t--too-small-arg\t\tSet fsxattr size to 27 bytes\n"); + printf("\t--new-fsx-flag\t\tUse unknown fa_flags flag\n"); + + return 1; +} -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] file_attr: introduce program to set/get fsxattr 2025-08-08 19:31 ` [PATCH 1/3] file_attr: introduce program to set/get fsxattr Andrey Albershteyn @ 2025-08-11 15:23 ` Darrick J. Wong 2025-08-11 18:06 ` Andrey Albershteyn 2025-08-11 17:51 ` Zorro Lang 1 sibling, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:23 UTC (permalink / raw) To: Andrey Albershteyn Cc: fstests, zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn On Fri, Aug 08, 2025 at 09:31:56PM +0200, Andrey Albershteyn wrote: > This programs uses newly introduced file_getattr and file_setattr > syscalls. This program is partially a test of invalid options. This will > be used further in the test. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > .gitignore | 1 + > configure.ac | 1 + > include/builddefs.in | 1 + > m4/package_libcdev.m4 | 16 +++ > src/Makefile | 5 + > src/file_attr.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 301 insertions(+) > > diff --git a/.gitignore b/.gitignore > index 4fd817243dca..1a578eab1ea0 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -210,6 +210,7 @@ tags > /src/fiemap-fault > /src/min_dio_alignment > /src/dio-writeback-race > +/src/file_attr > > # Symlinked files > /tests/generic/035.out > diff --git a/configure.ac b/configure.ac > index f3c8c643f0eb..6fe54e8e1d54 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -73,6 +73,7 @@ AC_HAVE_RLIMIT_NOFILE > AC_NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE > AC_HAVE_FICLONE > AC_HAVE_TRIVIAL_AUTO_VAR_INIT > +AC_HAVE_FILE_ATTR > > AC_CHECK_FUNCS([renameat2]) > AC_CHECK_FUNCS([reallocarray]) > diff --git a/include/builddefs.in b/include/builddefs.in > index 96d5ed25b3e2..821237339cc7 100644 > --- a/include/builddefs.in > +++ b/include/builddefs.in > @@ -74,6 +74,7 @@ HAVE_BMV_OF_SHARED = @have_bmv_of_shared@ > HAVE_RLIMIT_NOFILE = @have_rlimit_nofile@ > NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE = @need_internal_xfs_ioc_exchange_range@ > HAVE_FICLONE = @have_ficlone@ > +HAVE_FILE_ATTR = @have_file_attr@ > > GCCFLAGS = -std=gnu11 -funsigned-char -fno-strict-aliasing -Wall > SANITIZER_CFLAGS += @autovar_init_cflags@ > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > index ed8fe6e32ae0..e68a70f7d87e 100644 > --- a/m4/package_libcdev.m4 > +++ b/m4/package_libcdev.m4 > @@ -86,3 +86,19 @@ AC_DEFUN([AC_HAVE_TRIVIAL_AUTO_VAR_INIT], > CFLAGS="${OLD_CFLAGS}" > AC_SUBST(autovar_init_cflags) > ]) > + > +# > +# Check if we have a file_getattr/file_setattr system call (Linux) > +# > +AC_DEFUN([AC_HAVE_FILE_ATTR], > + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) > + AC_LINK_IFELSE([AC_LANG_PROGRAM([[ > +#define _GNU_SOURCE > +#include <sys/syscall.h> > +#include <unistd.h> > + ]], [[ > + syscall(__NR_file_getattr, 0, 0, 0, 0, 0, 0); > + ]])],[have_file_attr=yes > + AC_MSG_RESULT(yes)],[AC_MSG_RESULT(no)]) > + AC_SUBST(have_file_attr) > + ]) > diff --git a/src/Makefile b/src/Makefile > index 6ac72b366257..f3137acf687f 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -61,6 +61,11 @@ ifeq ($(HAVE_FALLOCATE), true) > LCFLAGS += -DHAVE_FALLOCATE > endif > > +ifeq ($(HAVE_FILE_ATTR), yes) > +LINUX_TARGETS += file_attr > +LCFLAGS += -DHAVE_FILE_ATTR > +endif > + > ifeq ($(PKG_PLATFORM),linux) > TARGETS += $(LINUX_TARGETS) > endif > diff --git a/src/file_attr.c b/src/file_attr.c > new file mode 100644 > index 000000000000..9756ab265a57 > --- /dev/null > +++ b/src/file_attr.c > @@ -0,0 +1,277 @@ > +#include "global.h" > +#include <sys/syscall.h> > +#include <getopt.h> > +#include <errno.h> > +#include <linux/fs.h> > +#include <sys/stat.h> > +#include <string.h> > +#include <getopt.h> > +#include <stdlib.h> > +#include <unistd.h> > + > +#ifndef HAVE_FILE_ATTR > +#define __NR_file_getattr 468 > +#define __NR_file_setattr 469 > + > +struct file_attr { > + __u32 fa_xflags; /* xflags field value (get/set) */ > + __u32 fa_extsize; /* extsize field value (get/set)*/ > + __u32 fa_nextents; /* nextents field value (get) */ > + __u32 fa_projid; /* project identifier (get/set) */ > + __u32 fa_cowextsize; /* CoW extsize field value (get/set) */ > +}; > + > +#endif > + > +#define SPECIAL_FILE(x) \ > + (S_ISCHR((x)) \ > + || S_ISBLK((x)) \ > + || S_ISFIFO((x)) \ > + || S_ISLNK((x)) \ > + || S_ISSOCK((x))) > + > +static struct option long_options[] = { > + {"set", no_argument, 0, 's' }, > + {"get", no_argument, 0, 'g' }, > + {"no-follow", no_argument, 0, 'n' }, > + {"at-cwd", no_argument, 0, 'a' }, > + {"set-nodump", no_argument, 0, 'd' }, > + {"invalid-at", no_argument, 0, 'i' }, > + {"too-big-arg", no_argument, 0, 'b' }, > + {"too-small-arg", no_argument, 0, 'm' }, > + {"new-fsx-flag", no_argument, 0, 'x' }, > + {0, 0, 0, 0 } > +}; > + > +static struct xflags { > + uint flag; > + char *shortname; > + char *longname; > +} xflags[] = { > + { FS_XFLAG_REALTIME, "r", "realtime" }, > + { FS_XFLAG_PREALLOC, "p", "prealloc" }, > + { FS_XFLAG_IMMUTABLE, "i", "immutable" }, > + { FS_XFLAG_APPEND, "a", "append-only" }, > + { FS_XFLAG_SYNC, "s", "sync" }, > + { FS_XFLAG_NOATIME, "A", "no-atime" }, > + { FS_XFLAG_NODUMP, "d", "no-dump" }, > + { FS_XFLAG_RTINHERIT, "t", "rt-inherit" }, > + { FS_XFLAG_PROJINHERIT, "P", "proj-inherit" }, > + { FS_XFLAG_NOSYMLINKS, "n", "nosymlinks" }, > + { FS_XFLAG_EXTSIZE, "e", "extsize" }, > + { FS_XFLAG_EXTSZINHERIT, "E", "extsz-inherit" }, > + { FS_XFLAG_NODEFRAG, "f", "no-defrag" }, > + { FS_XFLAG_FILESTREAM, "S", "filestream" }, > + { FS_XFLAG_DAX, "x", "dax" }, > + { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > + { 0, NULL, NULL } > +}; > + > +static int > +file_getattr( > + int dfd, > + const char *filename, > + struct file_attr *fsx, > + size_t usize, > + unsigned int at_flags) > +{ > + return syscall(__NR_file_getattr, dfd, filename, fsx, usize, at_flags); > +} > + > +static int > +file_setattr( > + int dfd, > + const char *filename, > + struct file_attr *fsx, > + size_t usize, > + unsigned int at_flags) > +{ > + return syscall(__NR_file_setattr, dfd, filename, fsx, usize, at_flags); > +} > + > +void > +printxattr( > + uint flags, > + int verbose, > + int dofname, > + const char *fname, > + int dobraces, > + int doeol) > +{ > + struct xflags *p; > + int first = 1; > + > + if (dobraces) > + fputs("[", stdout); > + for (p = xflags; p->flag; p++) { > + if (flags & p->flag) { > + if (verbose) { > + if (first) > + first = 0; > + else > + fputs(", ", stdout); > + fputs(p->longname, stdout); > + } else { > + fputs(p->shortname, stdout); > + } > + } else if (!verbose) { > + fputs("-", stdout); > + } > + } > + if (dobraces) > + fputs("]", stdout); > + if (dofname) > + printf(" %s ", fname); > + if (doeol) > + fputs("\n", stdout); > +} > + > +int main(int argc, char *argv[]) > +{ > + int error; > + int c; > + const char *path = NULL; > + const char *path1 = NULL; > + const char *path2 = NULL; > + unsigned int at_flags = 0; > + unsigned int fa_xflags = 0; > + int action = 0; /* 0 get; 1 set */ > + struct file_attr fsx = { }; > + int fa_size = sizeof(struct file_attr); > + struct stat status; > + int fd; > + int at_fdcwd = 0; > + int unknwon_fa_flag = 0; > + > + while (1) { > + int option_index = 0; > + > + c = getopt_long_only(argc, argv, "", long_options, &option_index); > + if (c == -1) > + break; > + > + switch (c) { > + case 's': > + action = 1; > + break; > + case 'g': > + action = 0; > + break; > + case 'n': > + at_flags |= AT_SYMLINK_NOFOLLOW; > + break; > + case 'a': > + at_fdcwd = 1; > + break; > + case 'd': > + fa_xflags |= FS_XFLAG_NODUMP; > + break; > + case 'i': > + at_flags |= (1 << 25); > + break; > + case 'b': > + fa_size = getpagesize() + 1; /* max size if page size */ > + break; > + case 'm': > + fa_size = 19; /* VER0 size of fsxattr is 20 */ > + break; > + case 'x': > + unknwon_fa_flag = (1 << 27); > + break; > + default: > + goto usage; > + } > + } > + > + if (!path1 && optind < argc) > + path1 = argv[optind++]; > + if (!path2 && optind < argc) > + path2 = argv[optind++]; > + > + if (at_fdcwd) { > + fd = AT_FDCWD; > + path = path1; > + } else if (!path2) { > + error = stat(path1, &status); > + if (error) { > + fprintf(stderr, > +"Can not get file status of %s: %s\n", path1, strerror(errno)); > + return error; > + } > + > + if (SPECIAL_FILE(status.st_mode)) { > + fprintf(stderr, > +"Can not open special file %s without parent dir: %s\n", path1, strerror(errno)); > + return errno; > + } > + > + fd = open(path1, O_RDONLY); > + if (fd == -1) { > + fprintf(stderr, "Can not open %s: %s\n", path1, > + strerror(errno)); > + return errno; > + } > + } else { > + fd = open(path1, O_RDONLY); > + if (fd == -1) { > + fprintf(stderr, "Can not open %s: %s\n", path1, > + strerror(errno)); > + return errno; > + } > + path = path2; > + } > + > + if (!path) > + at_flags |= AT_EMPTY_PATH; > + > + if (action) { > + error = file_getattr(fd, path, &fsx, fa_size, > + at_flags); > + if (error) { > + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, > + strerror(errno)); > + return error; > + } > + > + fsx.fa_xflags |= (fa_xflags | unknwon_fa_flag); > + > + error = file_setattr(fd, path, &fsx, fa_size, > + at_flags); > + if (error) { > + fprintf(stderr, "Can not set fsxattr on %s: %s\n", path, > + strerror(errno)); > + return error; > + } > + } else { > + error = file_getattr(fd, path, &fsx, fa_size, > + at_flags); > + if (error) { > + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, > + strerror(errno)); > + return error; > + } Can the file_getattr be lifted above the if (action) ? > + > + if (path2) > + printxattr(fsx.fa_xflags, 0, 1, path, 0, 1); > + else > + printxattr(fsx.fa_xflags, 0, 1, path1, 0, 1); > + } > + > + return error; > + > +usage: > + printf("Usage: %s [options]\n", argv[0]); > + printf("Options:\n"); > + printf("\t--get\t\tget filesystem inode attributes\n"); > + printf("\t--set\t\tset filesystem inode attributes\n"); > + printf("\t--at-cwd\t\topen file at current working directory\n"); > + printf("\t--no-follow\t\tdon't follow symlinks\n"); > + printf("\t--set-nodump\t\tset FS_XFLAG_NODUMP on an inode\n"); > + printf("\t--invalid-at\t\tUse invalida AT_* flag\n"); > + printf("\t--too-big-arg\t\tSet fsxattr size bigger than PAGE_SIZE\n"); > + printf("\t--too-small-arg\t\tSet fsxattr size to 27 bytes\n"); 27? I thought you put in 19 above? Also it'd be nice if the help listed the short and long versions. (Or skip the short cli switches ;)) --D > + printf("\t--new-fsx-flag\t\tUse unknown fa_flags flag\n"); > + > + return 1; > +} > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] file_attr: introduce program to set/get fsxattr 2025-08-11 15:23 ` Darrick J. Wong @ 2025-08-11 18:06 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 18:06 UTC (permalink / raw) To: Darrick J. Wong Cc: fstests, zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn On 2025-08-11 08:23:40, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:31:56PM +0200, Andrey Albershteyn wrote: > > This programs uses newly introduced file_getattr and file_setattr > > syscalls. This program is partially a test of invalid options. This will > > be used further in the test. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > .gitignore | 1 + > > configure.ac | 1 + > > include/builddefs.in | 1 + > > m4/package_libcdev.m4 | 16 +++ > > src/Makefile | 5 + > > src/file_attr.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 6 files changed, 301 insertions(+) > > > > diff --git a/.gitignore b/.gitignore > > index 4fd817243dca..1a578eab1ea0 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -210,6 +210,7 @@ tags > > /src/fiemap-fault > > /src/min_dio_alignment > > /src/dio-writeback-race > > +/src/file_attr > > > > # Symlinked files > > /tests/generic/035.out > > diff --git a/configure.ac b/configure.ac > > index f3c8c643f0eb..6fe54e8e1d54 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -73,6 +73,7 @@ AC_HAVE_RLIMIT_NOFILE > > AC_NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE > > AC_HAVE_FICLONE > > AC_HAVE_TRIVIAL_AUTO_VAR_INIT > > +AC_HAVE_FILE_ATTR > > > > AC_CHECK_FUNCS([renameat2]) > > AC_CHECK_FUNCS([reallocarray]) > > diff --git a/include/builddefs.in b/include/builddefs.in > > index 96d5ed25b3e2..821237339cc7 100644 > > --- a/include/builddefs.in > > +++ b/include/builddefs.in > > @@ -74,6 +74,7 @@ HAVE_BMV_OF_SHARED = @have_bmv_of_shared@ > > HAVE_RLIMIT_NOFILE = @have_rlimit_nofile@ > > NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE = @need_internal_xfs_ioc_exchange_range@ > > HAVE_FICLONE = @have_ficlone@ > > +HAVE_FILE_ATTR = @have_file_attr@ > > > > GCCFLAGS = -std=gnu11 -funsigned-char -fno-strict-aliasing -Wall > > SANITIZER_CFLAGS += @autovar_init_cflags@ > > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > > index ed8fe6e32ae0..e68a70f7d87e 100644 > > --- a/m4/package_libcdev.m4 > > +++ b/m4/package_libcdev.m4 > > @@ -86,3 +86,19 @@ AC_DEFUN([AC_HAVE_TRIVIAL_AUTO_VAR_INIT], > > CFLAGS="${OLD_CFLAGS}" > > AC_SUBST(autovar_init_cflags) > > ]) > > + > > +# > > +# Check if we have a file_getattr/file_setattr system call (Linux) > > +# > > +AC_DEFUN([AC_HAVE_FILE_ATTR], > > + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) > > + AC_LINK_IFELSE([AC_LANG_PROGRAM([[ > > +#define _GNU_SOURCE > > +#include <sys/syscall.h> > > +#include <unistd.h> > > + ]], [[ > > + syscall(__NR_file_getattr, 0, 0, 0, 0, 0, 0); > > + ]])],[have_file_attr=yes > > + AC_MSG_RESULT(yes)],[AC_MSG_RESULT(no)]) > > + AC_SUBST(have_file_attr) > > + ]) > > diff --git a/src/Makefile b/src/Makefile > > index 6ac72b366257..f3137acf687f 100644 > > --- a/src/Makefile > > +++ b/src/Makefile > > @@ -61,6 +61,11 @@ ifeq ($(HAVE_FALLOCATE), true) > > LCFLAGS += -DHAVE_FALLOCATE > > endif > > > > +ifeq ($(HAVE_FILE_ATTR), yes) > > +LINUX_TARGETS += file_attr > > +LCFLAGS += -DHAVE_FILE_ATTR > > +endif > > + > > ifeq ($(PKG_PLATFORM),linux) > > TARGETS += $(LINUX_TARGETS) > > endif > > diff --git a/src/file_attr.c b/src/file_attr.c > > new file mode 100644 > > index 000000000000..9756ab265a57 > > --- /dev/null > > +++ b/src/file_attr.c > > @@ -0,0 +1,277 @@ > > +#include "global.h" > > +#include <sys/syscall.h> > > +#include <getopt.h> > > +#include <errno.h> > > +#include <linux/fs.h> > > +#include <sys/stat.h> > > +#include <string.h> > > +#include <getopt.h> > > +#include <stdlib.h> > > +#include <unistd.h> > > + > > +#ifndef HAVE_FILE_ATTR > > +#define __NR_file_getattr 468 > > +#define __NR_file_setattr 469 > > + > > +struct file_attr { > > + __u32 fa_xflags; /* xflags field value (get/set) */ > > + __u32 fa_extsize; /* extsize field value (get/set)*/ > > + __u32 fa_nextents; /* nextents field value (get) */ > > + __u32 fa_projid; /* project identifier (get/set) */ > > + __u32 fa_cowextsize; /* CoW extsize field value (get/set) */ > > +}; > > + > > +#endif > > + > > +#define SPECIAL_FILE(x) \ > > + (S_ISCHR((x)) \ > > + || S_ISBLK((x)) \ > > + || S_ISFIFO((x)) \ > > + || S_ISLNK((x)) \ > > + || S_ISSOCK((x))) > > + > > +static struct option long_options[] = { > > + {"set", no_argument, 0, 's' }, > > + {"get", no_argument, 0, 'g' }, > > + {"no-follow", no_argument, 0, 'n' }, > > + {"at-cwd", no_argument, 0, 'a' }, > > + {"set-nodump", no_argument, 0, 'd' }, > > + {"invalid-at", no_argument, 0, 'i' }, > > + {"too-big-arg", no_argument, 0, 'b' }, > > + {"too-small-arg", no_argument, 0, 'm' }, > > + {"new-fsx-flag", no_argument, 0, 'x' }, > > + {0, 0, 0, 0 } > > +}; > > + > > +static struct xflags { > > + uint flag; > > + char *shortname; > > + char *longname; > > +} xflags[] = { > > + { FS_XFLAG_REALTIME, "r", "realtime" }, > > + { FS_XFLAG_PREALLOC, "p", "prealloc" }, > > + { FS_XFLAG_IMMUTABLE, "i", "immutable" }, > > + { FS_XFLAG_APPEND, "a", "append-only" }, > > + { FS_XFLAG_SYNC, "s", "sync" }, > > + { FS_XFLAG_NOATIME, "A", "no-atime" }, > > + { FS_XFLAG_NODUMP, "d", "no-dump" }, > > + { FS_XFLAG_RTINHERIT, "t", "rt-inherit" }, > > + { FS_XFLAG_PROJINHERIT, "P", "proj-inherit" }, > > + { FS_XFLAG_NOSYMLINKS, "n", "nosymlinks" }, > > + { FS_XFLAG_EXTSIZE, "e", "extsize" }, > > + { FS_XFLAG_EXTSZINHERIT, "E", "extsz-inherit" }, > > + { FS_XFLAG_NODEFRAG, "f", "no-defrag" }, > > + { FS_XFLAG_FILESTREAM, "S", "filestream" }, > > + { FS_XFLAG_DAX, "x", "dax" }, > > + { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > > + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > > + { 0, NULL, NULL } > > +}; > > + > > +static int > > +file_getattr( > > + int dfd, > > + const char *filename, > > + struct file_attr *fsx, > > + size_t usize, > > + unsigned int at_flags) > > +{ > > + return syscall(__NR_file_getattr, dfd, filename, fsx, usize, at_flags); > > +} > > + > > +static int > > +file_setattr( > > + int dfd, > > + const char *filename, > > + struct file_attr *fsx, > > + size_t usize, > > + unsigned int at_flags) > > +{ > > + return syscall(__NR_file_setattr, dfd, filename, fsx, usize, at_flags); > > +} > > + > > +void > > +printxattr( > > + uint flags, > > + int verbose, > > + int dofname, > > + const char *fname, > > + int dobraces, > > + int doeol) > > +{ > > + struct xflags *p; > > + int first = 1; > > + > > + if (dobraces) > > + fputs("[", stdout); > > + for (p = xflags; p->flag; p++) { > > + if (flags & p->flag) { > > + if (verbose) { > > + if (first) > > + first = 0; > > + else > > + fputs(", ", stdout); > > + fputs(p->longname, stdout); > > + } else { > > + fputs(p->shortname, stdout); > > + } > > + } else if (!verbose) { > > + fputs("-", stdout); > > + } > > + } > > + if (dobraces) > > + fputs("]", stdout); > > + if (dofname) > > + printf(" %s ", fname); > > + if (doeol) > > + fputs("\n", stdout); > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + int error; > > + int c; > > + const char *path = NULL; > > + const char *path1 = NULL; > > + const char *path2 = NULL; > > + unsigned int at_flags = 0; > > + unsigned int fa_xflags = 0; > > + int action = 0; /* 0 get; 1 set */ > > + struct file_attr fsx = { }; > > + int fa_size = sizeof(struct file_attr); > > + struct stat status; > > + int fd; > > + int at_fdcwd = 0; > > + int unknwon_fa_flag = 0; > > + > > + while (1) { > > + int option_index = 0; > > + > > + c = getopt_long_only(argc, argv, "", long_options, &option_index); > > + if (c == -1) > > + break; > > + > > + switch (c) { > > + case 's': > > + action = 1; > > + break; > > + case 'g': > > + action = 0; > > + break; > > + case 'n': > > + at_flags |= AT_SYMLINK_NOFOLLOW; > > + break; > > + case 'a': > > + at_fdcwd = 1; > > + break; > > + case 'd': > > + fa_xflags |= FS_XFLAG_NODUMP; > > + break; > > + case 'i': > > + at_flags |= (1 << 25); > > + break; > > + case 'b': > > + fa_size = getpagesize() + 1; /* max size if page size */ > > + break; > > + case 'm': > > + fa_size = 19; /* VER0 size of fsxattr is 20 */ > > + break; > > + case 'x': > > + unknwon_fa_flag = (1 << 27); > > + break; > > + default: > > + goto usage; > > + } > > + } > > + > > + if (!path1 && optind < argc) > > + path1 = argv[optind++]; > > + if (!path2 && optind < argc) > > + path2 = argv[optind++]; > > + > > + if (at_fdcwd) { > > + fd = AT_FDCWD; > > + path = path1; > > + } else if (!path2) { > > + error = stat(path1, &status); > > + if (error) { > > + fprintf(stderr, > > +"Can not get file status of %s: %s\n", path1, strerror(errno)); > > + return error; > > + } > > + > > + if (SPECIAL_FILE(status.st_mode)) { > > + fprintf(stderr, > > +"Can not open special file %s without parent dir: %s\n", path1, strerror(errno)); > > + return errno; > > + } > > + > > + fd = open(path1, O_RDONLY); > > + if (fd == -1) { > > + fprintf(stderr, "Can not open %s: %s\n", path1, > > + strerror(errno)); > > + return errno; > > + } > > + } else { > > + fd = open(path1, O_RDONLY); > > + if (fd == -1) { > > + fprintf(stderr, "Can not open %s: %s\n", path1, > > + strerror(errno)); > > + return errno; > > + } > > + path = path2; > > + } > > + > > + if (!path) > > + at_flags |= AT_EMPTY_PATH; > > + > > + if (action) { > > + error = file_getattr(fd, path, &fsx, fa_size, > > + at_flags); > > + if (error) { > > + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, > > + strerror(errno)); > > + return error; > > + } > > + > > + fsx.fa_xflags |= (fa_xflags | unknwon_fa_flag); > > + > > + error = file_setattr(fd, path, &fsx, fa_size, > > + at_flags); > > + if (error) { > > + fprintf(stderr, "Can not set fsxattr on %s: %s\n", path, > > + strerror(errno)); > > + return error; > > + } > > + } else { > > + error = file_getattr(fd, path, &fsx, fa_size, > > + at_flags); > > + if (error) { > > + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, > > + strerror(errno)); > > + return error; > > + } > > Can the file_getattr be lifted above the if (action) ? yup > > > + > > + if (path2) > > + printxattr(fsx.fa_xflags, 0, 1, path, 0, 1); > > + else > > + printxattr(fsx.fa_xflags, 0, 1, path1, 0, 1); > > + } > > + > > + return error; > > + > > +usage: > > + printf("Usage: %s [options]\n", argv[0]); > > + printf("Options:\n"); > > + printf("\t--get\t\tget filesystem inode attributes\n"); > > + printf("\t--set\t\tset filesystem inode attributes\n"); > > + printf("\t--at-cwd\t\topen file at current working directory\n"); > > + printf("\t--no-follow\t\tdon't follow symlinks\n"); > > + printf("\t--set-nodump\t\tset FS_XFLAG_NODUMP on an inode\n"); > > + printf("\t--invalid-at\t\tUse invalida AT_* flag\n"); > > + printf("\t--too-big-arg\t\tSet fsxattr size bigger than PAGE_SIZE\n"); > > + printf("\t--too-small-arg\t\tSet fsxattr size to 27 bytes\n"); > > 27? I thought you put in 19 above? ops, forgot to update it here > > Also it'd be nice if the help listed the short and long versions. > > (Or skip the short cli switches ;)) will add short versions > > --D > > > + printf("\t--new-fsx-flag\t\tUse unknown fa_flags flag\n"); > > + > > + return 1; > > +} > > > > -- > > 2.49.0 > > > > > -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] file_attr: introduce program to set/get fsxattr 2025-08-08 19:31 ` [PATCH 1/3] file_attr: introduce program to set/get fsxattr Andrey Albershteyn 2025-08-11 15:23 ` Darrick J. Wong @ 2025-08-11 17:51 ` Zorro Lang 2025-08-11 18:12 ` Andrey Albershteyn 1 sibling, 1 reply; 33+ messages in thread From: Zorro Lang @ 2025-08-11 17:51 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On Fri, Aug 08, 2025 at 09:31:56PM +0200, Andrey Albershteyn wrote: > This programs uses newly introduced file_getattr and file_setattr > syscalls. This program is partially a test of invalid options. This will > be used further in the test. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > .gitignore | 1 + > configure.ac | 1 + > include/builddefs.in | 1 + > m4/package_libcdev.m4 | 16 +++ > src/Makefile | 5 + > src/file_attr.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 301 insertions(+) > > diff --git a/.gitignore b/.gitignore > index 4fd817243dca..1a578eab1ea0 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -210,6 +210,7 @@ tags > /src/fiemap-fault > /src/min_dio_alignment > /src/dio-writeback-race > +/src/file_attr I'm wondering if xfsprogs/xfs_io would like to have this command :) Thanks, Zorro > > # Symlinked files > /tests/generic/035.out > diff --git a/configure.ac b/configure.ac > index f3c8c643f0eb..6fe54e8e1d54 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -73,6 +73,7 @@ AC_HAVE_RLIMIT_NOFILE > AC_NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE > AC_HAVE_FICLONE > AC_HAVE_TRIVIAL_AUTO_VAR_INIT > +AC_HAVE_FILE_ATTR > > AC_CHECK_FUNCS([renameat2]) > AC_CHECK_FUNCS([reallocarray]) > diff --git a/include/builddefs.in b/include/builddefs.in > index 96d5ed25b3e2..821237339cc7 100644 > --- a/include/builddefs.in > +++ b/include/builddefs.in > @@ -74,6 +74,7 @@ HAVE_BMV_OF_SHARED = @have_bmv_of_shared@ > HAVE_RLIMIT_NOFILE = @have_rlimit_nofile@ > NEED_INTERNAL_XFS_IOC_EXCHANGE_RANGE = @need_internal_xfs_ioc_exchange_range@ > HAVE_FICLONE = @have_ficlone@ > +HAVE_FILE_ATTR = @have_file_attr@ > > GCCFLAGS = -std=gnu11 -funsigned-char -fno-strict-aliasing -Wall > SANITIZER_CFLAGS += @autovar_init_cflags@ > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > index ed8fe6e32ae0..e68a70f7d87e 100644 > --- a/m4/package_libcdev.m4 > +++ b/m4/package_libcdev.m4 > @@ -86,3 +86,19 @@ AC_DEFUN([AC_HAVE_TRIVIAL_AUTO_VAR_INIT], > CFLAGS="${OLD_CFLAGS}" > AC_SUBST(autovar_init_cflags) > ]) > + > +# > +# Check if we have a file_getattr/file_setattr system call (Linux) > +# > +AC_DEFUN([AC_HAVE_FILE_ATTR], > + [ AC_MSG_CHECKING([for file_getattr/file_setattr syscalls]) > + AC_LINK_IFELSE([AC_LANG_PROGRAM([[ > +#define _GNU_SOURCE > +#include <sys/syscall.h> > +#include <unistd.h> > + ]], [[ > + syscall(__NR_file_getattr, 0, 0, 0, 0, 0, 0); > + ]])],[have_file_attr=yes > + AC_MSG_RESULT(yes)],[AC_MSG_RESULT(no)]) > + AC_SUBST(have_file_attr) > + ]) > diff --git a/src/Makefile b/src/Makefile > index 6ac72b366257..f3137acf687f 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -61,6 +61,11 @@ ifeq ($(HAVE_FALLOCATE), true) > LCFLAGS += -DHAVE_FALLOCATE > endif > > +ifeq ($(HAVE_FILE_ATTR), yes) > +LINUX_TARGETS += file_attr > +LCFLAGS += -DHAVE_FILE_ATTR > +endif > + > ifeq ($(PKG_PLATFORM),linux) > TARGETS += $(LINUX_TARGETS) > endif > diff --git a/src/file_attr.c b/src/file_attr.c > new file mode 100644 > index 000000000000..9756ab265a57 > --- /dev/null > +++ b/src/file_attr.c > @@ -0,0 +1,277 @@ > +#include "global.h" > +#include <sys/syscall.h> > +#include <getopt.h> > +#include <errno.h> > +#include <linux/fs.h> > +#include <sys/stat.h> > +#include <string.h> > +#include <getopt.h> > +#include <stdlib.h> > +#include <unistd.h> > + > +#ifndef HAVE_FILE_ATTR > +#define __NR_file_getattr 468 > +#define __NR_file_setattr 469 > + > +struct file_attr { > + __u32 fa_xflags; /* xflags field value (get/set) */ > + __u32 fa_extsize; /* extsize field value (get/set)*/ > + __u32 fa_nextents; /* nextents field value (get) */ > + __u32 fa_projid; /* project identifier (get/set) */ > + __u32 fa_cowextsize; /* CoW extsize field value (get/set) */ > +}; > + > +#endif > + > +#define SPECIAL_FILE(x) \ > + (S_ISCHR((x)) \ > + || S_ISBLK((x)) \ > + || S_ISFIFO((x)) \ > + || S_ISLNK((x)) \ > + || S_ISSOCK((x))) > + > +static struct option long_options[] = { > + {"set", no_argument, 0, 's' }, > + {"get", no_argument, 0, 'g' }, > + {"no-follow", no_argument, 0, 'n' }, > + {"at-cwd", no_argument, 0, 'a' }, > + {"set-nodump", no_argument, 0, 'd' }, > + {"invalid-at", no_argument, 0, 'i' }, > + {"too-big-arg", no_argument, 0, 'b' }, > + {"too-small-arg", no_argument, 0, 'm' }, > + {"new-fsx-flag", no_argument, 0, 'x' }, > + {0, 0, 0, 0 } > +}; > + > +static struct xflags { > + uint flag; > + char *shortname; > + char *longname; > +} xflags[] = { > + { FS_XFLAG_REALTIME, "r", "realtime" }, > + { FS_XFLAG_PREALLOC, "p", "prealloc" }, > + { FS_XFLAG_IMMUTABLE, "i", "immutable" }, > + { FS_XFLAG_APPEND, "a", "append-only" }, > + { FS_XFLAG_SYNC, "s", "sync" }, > + { FS_XFLAG_NOATIME, "A", "no-atime" }, > + { FS_XFLAG_NODUMP, "d", "no-dump" }, > + { FS_XFLAG_RTINHERIT, "t", "rt-inherit" }, > + { FS_XFLAG_PROJINHERIT, "P", "proj-inherit" }, > + { FS_XFLAG_NOSYMLINKS, "n", "nosymlinks" }, > + { FS_XFLAG_EXTSIZE, "e", "extsize" }, > + { FS_XFLAG_EXTSZINHERIT, "E", "extsz-inherit" }, > + { FS_XFLAG_NODEFRAG, "f", "no-defrag" }, > + { FS_XFLAG_FILESTREAM, "S", "filestream" }, > + { FS_XFLAG_DAX, "x", "dax" }, > + { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > + { 0, NULL, NULL } > +}; > + > +static int > +file_getattr( > + int dfd, > + const char *filename, > + struct file_attr *fsx, > + size_t usize, > + unsigned int at_flags) > +{ > + return syscall(__NR_file_getattr, dfd, filename, fsx, usize, at_flags); > +} > + > +static int > +file_setattr( > + int dfd, > + const char *filename, > + struct file_attr *fsx, > + size_t usize, > + unsigned int at_flags) > +{ > + return syscall(__NR_file_setattr, dfd, filename, fsx, usize, at_flags); > +} > + > +void > +printxattr( > + uint flags, > + int verbose, > + int dofname, > + const char *fname, > + int dobraces, > + int doeol) > +{ > + struct xflags *p; > + int first = 1; > + > + if (dobraces) > + fputs("[", stdout); > + for (p = xflags; p->flag; p++) { > + if (flags & p->flag) { > + if (verbose) { > + if (first) > + first = 0; > + else > + fputs(", ", stdout); > + fputs(p->longname, stdout); > + } else { > + fputs(p->shortname, stdout); > + } > + } else if (!verbose) { > + fputs("-", stdout); > + } > + } > + if (dobraces) > + fputs("]", stdout); > + if (dofname) > + printf(" %s ", fname); > + if (doeol) > + fputs("\n", stdout); > +} > + > +int main(int argc, char *argv[]) > +{ > + int error; > + int c; > + const char *path = NULL; > + const char *path1 = NULL; > + const char *path2 = NULL; > + unsigned int at_flags = 0; > + unsigned int fa_xflags = 0; > + int action = 0; /* 0 get; 1 set */ > + struct file_attr fsx = { }; > + int fa_size = sizeof(struct file_attr); > + struct stat status; > + int fd; > + int at_fdcwd = 0; > + int unknwon_fa_flag = 0; > + > + while (1) { > + int option_index = 0; > + > + c = getopt_long_only(argc, argv, "", long_options, &option_index); > + if (c == -1) > + break; > + > + switch (c) { > + case 's': > + action = 1; > + break; > + case 'g': > + action = 0; > + break; > + case 'n': > + at_flags |= AT_SYMLINK_NOFOLLOW; > + break; > + case 'a': > + at_fdcwd = 1; > + break; > + case 'd': > + fa_xflags |= FS_XFLAG_NODUMP; > + break; > + case 'i': > + at_flags |= (1 << 25); > + break; > + case 'b': > + fa_size = getpagesize() + 1; /* max size if page size */ > + break; > + case 'm': > + fa_size = 19; /* VER0 size of fsxattr is 20 */ > + break; > + case 'x': > + unknwon_fa_flag = (1 << 27); > + break; > + default: > + goto usage; > + } > + } > + > + if (!path1 && optind < argc) > + path1 = argv[optind++]; > + if (!path2 && optind < argc) > + path2 = argv[optind++]; > + > + if (at_fdcwd) { > + fd = AT_FDCWD; > + path = path1; > + } else if (!path2) { > + error = stat(path1, &status); > + if (error) { > + fprintf(stderr, > +"Can not get file status of %s: %s\n", path1, strerror(errno)); > + return error; > + } > + > + if (SPECIAL_FILE(status.st_mode)) { > + fprintf(stderr, > +"Can not open special file %s without parent dir: %s\n", path1, strerror(errno)); > + return errno; > + } > + > + fd = open(path1, O_RDONLY); > + if (fd == -1) { > + fprintf(stderr, "Can not open %s: %s\n", path1, > + strerror(errno)); > + return errno; > + } > + } else { > + fd = open(path1, O_RDONLY); > + if (fd == -1) { > + fprintf(stderr, "Can not open %s: %s\n", path1, > + strerror(errno)); > + return errno; > + } > + path = path2; > + } > + > + if (!path) > + at_flags |= AT_EMPTY_PATH; > + > + if (action) { > + error = file_getattr(fd, path, &fsx, fa_size, > + at_flags); > + if (error) { > + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, > + strerror(errno)); > + return error; > + } > + > + fsx.fa_xflags |= (fa_xflags | unknwon_fa_flag); > + > + error = file_setattr(fd, path, &fsx, fa_size, > + at_flags); > + if (error) { > + fprintf(stderr, "Can not set fsxattr on %s: %s\n", path, > + strerror(errno)); > + return error; > + } > + } else { > + error = file_getattr(fd, path, &fsx, fa_size, > + at_flags); > + if (error) { > + fprintf(stderr, "Can not get fsxattr on %s: %s\n", path, > + strerror(errno)); > + return error; > + } > + > + if (path2) > + printxattr(fsx.fa_xflags, 0, 1, path, 0, 1); > + else > + printxattr(fsx.fa_xflags, 0, 1, path1, 0, 1); > + } > + > + return error; > + > +usage: > + printf("Usage: %s [options]\n", argv[0]); > + printf("Options:\n"); > + printf("\t--get\t\tget filesystem inode attributes\n"); > + printf("\t--set\t\tset filesystem inode attributes\n"); > + printf("\t--at-cwd\t\topen file at current working directory\n"); > + printf("\t--no-follow\t\tdon't follow symlinks\n"); > + printf("\t--set-nodump\t\tset FS_XFLAG_NODUMP on an inode\n"); > + printf("\t--invalid-at\t\tUse invalida AT_* flag\n"); > + printf("\t--too-big-arg\t\tSet fsxattr size bigger than PAGE_SIZE\n"); > + printf("\t--too-small-arg\t\tSet fsxattr size to 27 bytes\n"); > + printf("\t--new-fsx-flag\t\tUse unknown fa_flags flag\n"); > + > + return 1; > +} > > -- > 2.49.0 > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] file_attr: introduce program to set/get fsxattr 2025-08-11 17:51 ` Zorro Lang @ 2025-08-11 18:12 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 18:12 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On 2025-08-12 01:51:07, Zorro Lang wrote: > On Fri, Aug 08, 2025 at 09:31:56PM +0200, Andrey Albershteyn wrote: > > This programs uses newly introduced file_getattr and file_setattr > > syscalls. This program is partially a test of invalid options. This will > > be used further in the test. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > .gitignore | 1 + > > configure.ac | 1 + > > include/builddefs.in | 1 + > > m4/package_libcdev.m4 | 16 +++ > > src/Makefile | 5 + > > src/file_attr.c | 277 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 6 files changed, 301 insertions(+) > > > > diff --git a/.gitignore b/.gitignore > > index 4fd817243dca..1a578eab1ea0 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -210,6 +210,7 @@ tags > > /src/fiemap-fault > > /src/min_dio_alignment > > /src/dio-writeback-race > > +/src/file_attr > > I'm wondering if xfsprogs/xfs_io would like to have this command :) it has chattr/lsattr, but this one also generates quite a few invalid arguments for these syscalls, I don't think it would be useful in xfs_io -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls 2025-08-08 19:31 ` [PATCH 0/3] Test file_getattr and file_setattr syscalls Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 1/3] file_attr: introduce program to set/get fsxattr Andrey Albershteyn @ 2025-08-08 19:31 ` Andrey Albershteyn 2025-08-11 15:17 ` Darrick J. Wong 2025-08-11 17:55 ` Zorro Lang 2025-08-08 19:31 ` [PATCH 3/3] xfs: test quota's project ID on special files Andrey Albershteyn 2 siblings, 2 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:31 UTC (permalink / raw) To: fstests; +Cc: zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn Add a test to test basic functionality of file_getattr() and file_setattr() syscalls. Most of the work is done in file_attr utility. Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- tests/generic/2000 | 113 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/2000.out | 37 ++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/tests/generic/2000 b/tests/generic/2000 new file mode 100755 index 000000000000..b4410628c241 --- /dev/null +++ b/tests/generic/2000 @@ -0,0 +1,113 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved. +# +# FS QA Test No. 2000 +# +# Test file_getattr/file_setattr syscalls +# +. ./common/preamble +_begin_fstest auto + +# Import common functions. +# . ./common/filter + +_wants_kernel_commit xxxxxxxxxxx \ + "fs: introduce file_getattr and file_setattr syscalls" + +# Modify as appropriate. +_require_scratch +_require_test_program "af_unix" +_require_test_program "file_attr" +_require_symlinks +_require_mknod + +_scratch_mkfs >>$seqres.full 2>&1 +_scratch_mount + +file_attr () { + $here/src/file_attr $* +} + +create_af_unix () { + $here/src/af_unix $* || echo af_unix failed +} + +projectdir=$SCRATCH_MNT/prj + +# Create normal files and special files +mkdir $projectdir +mkfifo $projectdir/fifo +mknod $projectdir/chardev c 1 1 +mknod $projectdir/blockdev b 1 1 +create_af_unix $projectdir/socket +touch $projectdir/foo +ln -s $projectdir/foo $projectdir/symlink +touch $projectdir/bar +ln -s $projectdir/bar $projectdir/broken-symlink +rm -f $projectdir/bar + +echo "Error codes" +# wrong AT_ flags +file_attr --get --invalid-at $projectdir ./foo +file_attr --set --invalid-at $projectdir ./foo +# wrong fsxattr size (too big, too small) +file_attr --get --too-big-arg $projectdir ./foo +file_attr --get --too-small-arg $projectdir ./foo +file_attr --set --too-big-arg $projectdir ./foo +file_attr --set --too-small-arg $projectdir ./foo +# out of fsx_xflags mask +file_attr --set --new-fsx-flag $projectdir ./foo + +echo "Initial attributes state" +file_attr --get $projectdir +file_attr --get $projectdir ./fifo +file_attr --get $projectdir ./chardev +file_attr --get $projectdir ./blockdev +file_attr --get $projectdir ./socket +file_attr --get $projectdir ./foo +file_attr --get $projectdir ./symlink + +echo "Set FS_XFLAG_NODUMP (d)" +file_attr --set --set-nodump $projectdir +file_attr --set --set-nodump $projectdir ./fifo +file_attr --set --set-nodump $projectdir ./chardev +file_attr --set --set-nodump $projectdir ./blockdev +file_attr --set --set-nodump $projectdir ./socket +file_attr --set --set-nodump $projectdir ./foo +file_attr --set --set-nodump $projectdir ./symlink + +echo "Read attributes" +file_attr --get $projectdir +file_attr --get $projectdir ./fifo +file_attr --get $projectdir ./chardev +file_attr --get $projectdir ./blockdev +file_attr --get $projectdir ./socket +file_attr --get $projectdir ./foo +file_attr --get $projectdir ./symlink + +echo "Set attribute on broken link with AT_SYMLINK_NOFOLLOW" +file_attr --set --set-nodump $projectdir ./broken-symlink +file_attr --get $projectdir ./broken-symlink + +file_attr --set --no-follow --set-nodump $projectdir ./broken-symlink +file_attr --get --no-follow $projectdir ./broken-symlink + +cd $SCRATCH_MNT +touch ./foo2 +echo "Initial state of foo2" +file_attr --get --at-cwd ./foo2 +echo "Set attribute relative to AT_FDCWD" +file_attr --set --at-cwd --set-nodump ./foo2 +file_attr --get --at-cwd ./foo2 + +echo "Set attribute on AT_FDCWD" +mkdir ./bar +file_attr --get --at-cwd ./bar +cd ./bar +file_attr --set --at-cwd --set-nodump "" +file_attr --get --at-cwd . + +# success, all done +status=0 +exit diff --git a/tests/generic/2000.out b/tests/generic/2000.out new file mode 100644 index 000000000000..51b4d84e2bae --- /dev/null +++ b/tests/generic/2000.out @@ -0,0 +1,37 @@ +QA output created by 2000 +Error codes +Can not get fsxattr on ./foo: Invalid argument +Can not get fsxattr on ./foo: Invalid argument +Can not get fsxattr on ./foo: Argument list too long +Can not get fsxattr on ./foo: Invalid argument +Can not get fsxattr on ./foo: Argument list too long +Can not get fsxattr on ./foo: Invalid argument +Can not set fsxattr on ./foo: Invalid argument +Initial attributes state +----------------- /mnt/scratch/prj +----------------- ./fifo +----------------- ./chardev +----------------- ./blockdev +----------------- ./socket +----------------- ./foo +----------------- ./symlink +Set FS_XFLAG_NODUMP (d) +Read attributes +------d---------- /mnt/scratch/prj +------d---------- ./fifo +------d---------- ./chardev +------d---------- ./blockdev +------d---------- ./socket +------d---------- ./foo +------d---------- ./symlink +Set attribute on broken link with AT_SYMLINK_NOFOLLOW +Can not get fsxattr on ./broken-symlink: No such file or directory +Can not get fsxattr on ./broken-symlink: No such file or directory +------d---------- ./broken-symlink +Initial state of foo2 +----------------- ./foo2 +Set attribute relative to AT_FDCWD +------d---------- ./foo2 +Set attribute on AT_FDCWD +----------------- ./bar +------d---------- . -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls 2025-08-08 19:31 ` [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls Andrey Albershteyn @ 2025-08-11 15:17 ` Darrick J. Wong 2025-08-11 18:13 ` Andrey Albershteyn 2025-08-11 17:55 ` Zorro Lang 1 sibling, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:17 UTC (permalink / raw) To: Andrey Albershteyn Cc: fstests, zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn On Fri, Aug 08, 2025 at 09:31:57PM +0200, Andrey Albershteyn wrote: > Add a test to test basic functionality of file_getattr() and > file_setattr() syscalls. Most of the work is done in file_attr > utility. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > tests/generic/2000 | 113 +++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/2000.out | 37 ++++++++++++++++ > 2 files changed, 150 insertions(+) > > diff --git a/tests/generic/2000 b/tests/generic/2000 > new file mode 100755 > index 000000000000..b4410628c241 > --- /dev/null > +++ b/tests/generic/2000 > @@ -0,0 +1,113 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved. > +# > +# FS QA Test No. 2000 > +# > +# Test file_getattr/file_setattr syscalls > +# > +. ./common/preamble > +_begin_fstest auto > + > +# Import common functions. > +# . ./common/filter > + > +_wants_kernel_commit xxxxxxxxxxx \ > + "fs: introduce file_getattr and file_setattr syscalls" > + > +# Modify as appropriate. > +_require_scratch > +_require_test_program "af_unix" > +_require_test_program "file_attr" > +_require_symlinks > +_require_mknod > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_scratch_mount > + > +file_attr () { > + $here/src/file_attr $* > +} > + > +create_af_unix () { > + $here/src/af_unix $* || echo af_unix failed > +} > + > +projectdir=$SCRATCH_MNT/prj > + > +# Create normal files and special files > +mkdir $projectdir > +mkfifo $projectdir/fifo > +mknod $projectdir/chardev c 1 1 > +mknod $projectdir/blockdev b 1 1 > +create_af_unix $projectdir/socket > +touch $projectdir/foo > +ln -s $projectdir/foo $projectdir/symlink > +touch $projectdir/bar > +ln -s $projectdir/bar $projectdir/broken-symlink > +rm -f $projectdir/bar > + > +echo "Error codes" > +# wrong AT_ flags > +file_attr --get --invalid-at $projectdir ./foo > +file_attr --set --invalid-at $projectdir ./foo > +# wrong fsxattr size (too big, too small) > +file_attr --get --too-big-arg $projectdir ./foo > +file_attr --get --too-small-arg $projectdir ./foo > +file_attr --set --too-big-arg $projectdir ./foo > +file_attr --set --too-small-arg $projectdir ./foo > +# out of fsx_xflags mask > +file_attr --set --new-fsx-flag $projectdir ./foo > + > +echo "Initial attributes state" > +file_attr --get $projectdir > +file_attr --get $projectdir ./fifo > +file_attr --get $projectdir ./chardev > +file_attr --get $projectdir ./blockdev > +file_attr --get $projectdir ./socket > +file_attr --get $projectdir ./foo > +file_attr --get $projectdir ./symlink > + > +echo "Set FS_XFLAG_NODUMP (d)" > +file_attr --set --set-nodump $projectdir > +file_attr --set --set-nodump $projectdir ./fifo > +file_attr --set --set-nodump $projectdir ./chardev > +file_attr --set --set-nodump $projectdir ./blockdev > +file_attr --set --set-nodump $projectdir ./socket > +file_attr --set --set-nodump $projectdir ./foo > +file_attr --set --set-nodump $projectdir ./symlink > + > +echo "Read attributes" > +file_attr --get $projectdir > +file_attr --get $projectdir ./fifo > +file_attr --get $projectdir ./chardev > +file_attr --get $projectdir ./blockdev > +file_attr --get $projectdir ./socket > +file_attr --get $projectdir ./foo > +file_attr --get $projectdir ./symlink > + > +echo "Set attribute on broken link with AT_SYMLINK_NOFOLLOW" > +file_attr --set --set-nodump $projectdir ./broken-symlink > +file_attr --get $projectdir ./broken-symlink > + > +file_attr --set --no-follow --set-nodump $projectdir ./broken-symlink > +file_attr --get --no-follow $projectdir ./broken-symlink > + > +cd $SCRATCH_MNT > +touch ./foo2 > +echo "Initial state of foo2" > +file_attr --get --at-cwd ./foo2 > +echo "Set attribute relative to AT_FDCWD" > +file_attr --set --at-cwd --set-nodump ./foo2 > +file_attr --get --at-cwd ./foo2 > + > +echo "Set attribute on AT_FDCWD" > +mkdir ./bar > +file_attr --get --at-cwd ./bar > +cd ./bar > +file_attr --set --at-cwd --set-nodump "" > +file_attr --get --at-cwd . > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/2000.out b/tests/generic/2000.out > new file mode 100644 > index 000000000000..51b4d84e2bae > --- /dev/null > +++ b/tests/generic/2000.out > @@ -0,0 +1,37 @@ > +QA output created by 2000 > +Error codes > +Can not get fsxattr on ./foo: Invalid argument > +Can not get fsxattr on ./foo: Invalid argument > +Can not get fsxattr on ./foo: Argument list too long > +Can not get fsxattr on ./foo: Invalid argument > +Can not get fsxattr on ./foo: Argument list too long > +Can not get fsxattr on ./foo: Invalid argument > +Can not set fsxattr on ./foo: Invalid argument > +Initial attributes state > +----------------- /mnt/scratch/prj Assuming SCRATCH_DIR=/mnt/scratch on your system, please _filter_scratch the output. (The rest looks reasonable to me.) --D > +----------------- ./fifo > +----------------- ./chardev > +----------------- ./blockdev > +----------------- ./socket > +----------------- ./foo > +----------------- ./symlink > +Set FS_XFLAG_NODUMP (d) > +Read attributes > +------d---------- /mnt/scratch/prj > +------d---------- ./fifo > +------d---------- ./chardev > +------d---------- ./blockdev > +------d---------- ./socket > +------d---------- ./foo > +------d---------- ./symlink > +Set attribute on broken link with AT_SYMLINK_NOFOLLOW > +Can not get fsxattr on ./broken-symlink: No such file or directory > +Can not get fsxattr on ./broken-symlink: No such file or directory > +------d---------- ./broken-symlink > +Initial state of foo2 > +----------------- ./foo2 > +Set attribute relative to AT_FDCWD > +------d---------- ./foo2 > +Set attribute on AT_FDCWD > +----------------- ./bar > +------d---------- . > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls 2025-08-11 15:17 ` Darrick J. Wong @ 2025-08-11 18:13 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 18:13 UTC (permalink / raw) To: Darrick J. Wong Cc: fstests, zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn On 2025-08-11 08:17:40, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:31:57PM +0200, Andrey Albershteyn wrote: > > Add a test to test basic functionality of file_getattr() and > > file_setattr() syscalls. Most of the work is done in file_attr > > utility. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > tests/generic/2000 | 113 +++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/2000.out | 37 ++++++++++++++++ > > 2 files changed, 150 insertions(+) > > > > diff --git a/tests/generic/2000 b/tests/generic/2000 > > new file mode 100755 > > index 000000000000..b4410628c241 > > --- /dev/null > > +++ b/tests/generic/2000 > > @@ -0,0 +1,113 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved. > > +# > > +# FS QA Test No. 2000 > > +# > > +# Test file_getattr/file_setattr syscalls > > +# > > +. ./common/preamble > > +_begin_fstest auto > > + > > +# Import common functions. > > +# . ./common/filter > > + > > +_wants_kernel_commit xxxxxxxxxxx \ > > + "fs: introduce file_getattr and file_setattr syscalls" > > + > > +# Modify as appropriate. > > +_require_scratch > > +_require_test_program "af_unix" > > +_require_test_program "file_attr" > > +_require_symlinks > > +_require_mknod > > + > > +_scratch_mkfs >>$seqres.full 2>&1 > > +_scratch_mount > > + > > +file_attr () { > > + $here/src/file_attr $* > > +} > > + > > +create_af_unix () { > > + $here/src/af_unix $* || echo af_unix failed > > +} > > + > > +projectdir=$SCRATCH_MNT/prj > > + > > +# Create normal files and special files > > +mkdir $projectdir > > +mkfifo $projectdir/fifo > > +mknod $projectdir/chardev c 1 1 > > +mknod $projectdir/blockdev b 1 1 > > +create_af_unix $projectdir/socket > > +touch $projectdir/foo > > +ln -s $projectdir/foo $projectdir/symlink > > +touch $projectdir/bar > > +ln -s $projectdir/bar $projectdir/broken-symlink > > +rm -f $projectdir/bar > > + > > +echo "Error codes" > > +# wrong AT_ flags > > +file_attr --get --invalid-at $projectdir ./foo > > +file_attr --set --invalid-at $projectdir ./foo > > +# wrong fsxattr size (too big, too small) > > +file_attr --get --too-big-arg $projectdir ./foo > > +file_attr --get --too-small-arg $projectdir ./foo > > +file_attr --set --too-big-arg $projectdir ./foo > > +file_attr --set --too-small-arg $projectdir ./foo > > +# out of fsx_xflags mask > > +file_attr --set --new-fsx-flag $projectdir ./foo > > + > > +echo "Initial attributes state" > > +file_attr --get $projectdir > > +file_attr --get $projectdir ./fifo > > +file_attr --get $projectdir ./chardev > > +file_attr --get $projectdir ./blockdev > > +file_attr --get $projectdir ./socket > > +file_attr --get $projectdir ./foo > > +file_attr --get $projectdir ./symlink > > + > > +echo "Set FS_XFLAG_NODUMP (d)" > > +file_attr --set --set-nodump $projectdir > > +file_attr --set --set-nodump $projectdir ./fifo > > +file_attr --set --set-nodump $projectdir ./chardev > > +file_attr --set --set-nodump $projectdir ./blockdev > > +file_attr --set --set-nodump $projectdir ./socket > > +file_attr --set --set-nodump $projectdir ./foo > > +file_attr --set --set-nodump $projectdir ./symlink > > + > > +echo "Read attributes" > > +file_attr --get $projectdir > > +file_attr --get $projectdir ./fifo > > +file_attr --get $projectdir ./chardev > > +file_attr --get $projectdir ./blockdev > > +file_attr --get $projectdir ./socket > > +file_attr --get $projectdir ./foo > > +file_attr --get $projectdir ./symlink > > + > > +echo "Set attribute on broken link with AT_SYMLINK_NOFOLLOW" > > +file_attr --set --set-nodump $projectdir ./broken-symlink > > +file_attr --get $projectdir ./broken-symlink > > + > > +file_attr --set --no-follow --set-nodump $projectdir ./broken-symlink > > +file_attr --get --no-follow $projectdir ./broken-symlink > > + > > +cd $SCRATCH_MNT > > +touch ./foo2 > > +echo "Initial state of foo2" > > +file_attr --get --at-cwd ./foo2 > > +echo "Set attribute relative to AT_FDCWD" > > +file_attr --set --at-cwd --set-nodump ./foo2 > > +file_attr --get --at-cwd ./foo2 > > + > > +echo "Set attribute on AT_FDCWD" > > +mkdir ./bar > > +file_attr --get --at-cwd ./bar > > +cd ./bar > > +file_attr --set --at-cwd --set-nodump "" > > +file_attr --get --at-cwd . > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/generic/2000.out b/tests/generic/2000.out > > new file mode 100644 > > index 000000000000..51b4d84e2bae > > --- /dev/null > > +++ b/tests/generic/2000.out > > @@ -0,0 +1,37 @@ > > +QA output created by 2000 > > +Error codes > > +Can not get fsxattr on ./foo: Invalid argument > > +Can not get fsxattr on ./foo: Invalid argument > > +Can not get fsxattr on ./foo: Argument list too long > > +Can not get fsxattr on ./foo: Invalid argument > > +Can not get fsxattr on ./foo: Argument list too long > > +Can not get fsxattr on ./foo: Invalid argument > > +Can not set fsxattr on ./foo: Invalid argument > > +Initial attributes state > > +----------------- /mnt/scratch/prj > > Assuming SCRATCH_DIR=/mnt/scratch on your system, please _filter_scratch > the output. > > (The rest looks reasonable to me.) > > --D > ops, will fix, thanks -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls 2025-08-08 19:31 ` [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls Andrey Albershteyn 2025-08-11 15:17 ` Darrick J. Wong @ 2025-08-11 17:55 ` Zorro Lang 2025-08-11 18:18 ` Andrey Albershteyn 1 sibling, 1 reply; 33+ messages in thread From: Zorro Lang @ 2025-08-11 17:55 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On Fri, Aug 08, 2025 at 09:31:57PM +0200, Andrey Albershteyn wrote: > Add a test to test basic functionality of file_getattr() and > file_setattr() syscalls. Most of the work is done in file_attr > utility. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > tests/generic/2000 | 113 +++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/2000.out | 37 ++++++++++++++++ > 2 files changed, 150 insertions(+) > > diff --git a/tests/generic/2000 b/tests/generic/2000 > new file mode 100755 > index 000000000000..b4410628c241 > --- /dev/null > +++ b/tests/generic/2000 > @@ -0,0 +1,113 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved. > +# > +# FS QA Test No. 2000 > +# > +# Test file_getattr/file_setattr syscalls > +# > +. ./common/preamble > +_begin_fstest auto > + > +# Import common functions. > +# . ./common/filter > + > +_wants_kernel_commit xxxxxxxxxxx \ > + "fs: introduce file_getattr and file_setattr syscalls" As this's a new feature test, I'm wondering if we should use a _require_ function to check if current kernel and FSTYP supports file_set/getattr syscalls, and _notrun if it's not supported, rather than fail the test. Thanks, Zorro > + > +# Modify as appropriate. > +_require_scratch > +_require_test_program "af_unix" > +_require_test_program "file_attr" > +_require_symlinks > +_require_mknod > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_scratch_mount > + > +file_attr () { > + $here/src/file_attr $* > +} > + > +create_af_unix () { > + $here/src/af_unix $* || echo af_unix failed > +} > + > +projectdir=$SCRATCH_MNT/prj > + > +# Create normal files and special files > +mkdir $projectdir > +mkfifo $projectdir/fifo > +mknod $projectdir/chardev c 1 1 > +mknod $projectdir/blockdev b 1 1 > +create_af_unix $projectdir/socket > +touch $projectdir/foo > +ln -s $projectdir/foo $projectdir/symlink > +touch $projectdir/bar > +ln -s $projectdir/bar $projectdir/broken-symlink > +rm -f $projectdir/bar > + > +echo "Error codes" > +# wrong AT_ flags > +file_attr --get --invalid-at $projectdir ./foo > +file_attr --set --invalid-at $projectdir ./foo > +# wrong fsxattr size (too big, too small) > +file_attr --get --too-big-arg $projectdir ./foo > +file_attr --get --too-small-arg $projectdir ./foo > +file_attr --set --too-big-arg $projectdir ./foo > +file_attr --set --too-small-arg $projectdir ./foo > +# out of fsx_xflags mask > +file_attr --set --new-fsx-flag $projectdir ./foo > + > +echo "Initial attributes state" > +file_attr --get $projectdir > +file_attr --get $projectdir ./fifo > +file_attr --get $projectdir ./chardev > +file_attr --get $projectdir ./blockdev > +file_attr --get $projectdir ./socket > +file_attr --get $projectdir ./foo > +file_attr --get $projectdir ./symlink > + > +echo "Set FS_XFLAG_NODUMP (d)" > +file_attr --set --set-nodump $projectdir > +file_attr --set --set-nodump $projectdir ./fifo > +file_attr --set --set-nodump $projectdir ./chardev > +file_attr --set --set-nodump $projectdir ./blockdev > +file_attr --set --set-nodump $projectdir ./socket > +file_attr --set --set-nodump $projectdir ./foo > +file_attr --set --set-nodump $projectdir ./symlink > + > +echo "Read attributes" > +file_attr --get $projectdir > +file_attr --get $projectdir ./fifo > +file_attr --get $projectdir ./chardev > +file_attr --get $projectdir ./blockdev > +file_attr --get $projectdir ./socket > +file_attr --get $projectdir ./foo > +file_attr --get $projectdir ./symlink > + > +echo "Set attribute on broken link with AT_SYMLINK_NOFOLLOW" > +file_attr --set --set-nodump $projectdir ./broken-symlink > +file_attr --get $projectdir ./broken-symlink > + > +file_attr --set --no-follow --set-nodump $projectdir ./broken-symlink > +file_attr --get --no-follow $projectdir ./broken-symlink > + > +cd $SCRATCH_MNT > +touch ./foo2 > +echo "Initial state of foo2" > +file_attr --get --at-cwd ./foo2 > +echo "Set attribute relative to AT_FDCWD" > +file_attr --set --at-cwd --set-nodump ./foo2 > +file_attr --get --at-cwd ./foo2 > + > +echo "Set attribute on AT_FDCWD" > +mkdir ./bar > +file_attr --get --at-cwd ./bar > +cd ./bar > +file_attr --set --at-cwd --set-nodump "" > +file_attr --get --at-cwd . > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/2000.out b/tests/generic/2000.out > new file mode 100644 > index 000000000000..51b4d84e2bae > --- /dev/null > +++ b/tests/generic/2000.out > @@ -0,0 +1,37 @@ > +QA output created by 2000 > +Error codes > +Can not get fsxattr on ./foo: Invalid argument > +Can not get fsxattr on ./foo: Invalid argument > +Can not get fsxattr on ./foo: Argument list too long > +Can not get fsxattr on ./foo: Invalid argument > +Can not get fsxattr on ./foo: Argument list too long > +Can not get fsxattr on ./foo: Invalid argument > +Can not set fsxattr on ./foo: Invalid argument > +Initial attributes state > +----------------- /mnt/scratch/prj > +----------------- ./fifo > +----------------- ./chardev > +----------------- ./blockdev > +----------------- ./socket > +----------------- ./foo > +----------------- ./symlink > +Set FS_XFLAG_NODUMP (d) > +Read attributes > +------d---------- /mnt/scratch/prj > +------d---------- ./fifo > +------d---------- ./chardev > +------d---------- ./blockdev > +------d---------- ./socket > +------d---------- ./foo > +------d---------- ./symlink > +Set attribute on broken link with AT_SYMLINK_NOFOLLOW > +Can not get fsxattr on ./broken-symlink: No such file or directory > +Can not get fsxattr on ./broken-symlink: No such file or directory > +------d---------- ./broken-symlink > +Initial state of foo2 > +----------------- ./foo2 > +Set attribute relative to AT_FDCWD > +------d---------- ./foo2 > +Set attribute on AT_FDCWD > +----------------- ./bar > +------d---------- . > > -- > 2.49.0 > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls 2025-08-11 17:55 ` Zorro Lang @ 2025-08-11 18:18 ` Andrey Albershteyn 2025-08-11 18:43 ` Zorro Lang 0 siblings, 1 reply; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 18:18 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On 2025-08-12 01:55:41, Zorro Lang wrote: > On Fri, Aug 08, 2025 at 09:31:57PM +0200, Andrey Albershteyn wrote: > > Add a test to test basic functionality of file_getattr() and > > file_setattr() syscalls. Most of the work is done in file_attr > > utility. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > tests/generic/2000 | 113 +++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/2000.out | 37 ++++++++++++++++ > > 2 files changed, 150 insertions(+) > > > > diff --git a/tests/generic/2000 b/tests/generic/2000 > > new file mode 100755 > > index 000000000000..b4410628c241 > > --- /dev/null > > +++ b/tests/generic/2000 > > @@ -0,0 +1,113 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved. > > +# > > +# FS QA Test No. 2000 > > +# > > +# Test file_getattr/file_setattr syscalls > > +# > > +. ./common/preamble > > +_begin_fstest auto > > + > > +# Import common functions. > > +# . ./common/filter > > + > > +_wants_kernel_commit xxxxxxxxxxx \ > > + "fs: introduce file_getattr and file_setattr syscalls" > > As this's a new feature test, I'm wondering if we should use a _require_ > function to check if current kernel and FSTYP supports file_set/getattr > syscalls, and _notrun if it's not supported, rather than fail the test. hmm, I don't see where _require_function is defined Anyway, the _notrun makes more sense, I will look into what to check for to skip this one if it's not supported -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls 2025-08-11 18:18 ` Andrey Albershteyn @ 2025-08-11 18:43 ` Zorro Lang 0 siblings, 0 replies; 33+ messages in thread From: Zorro Lang @ 2025-08-11 18:43 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On Mon, Aug 11, 2025 at 08:18:24PM +0200, Andrey Albershteyn wrote: > On 2025-08-12 01:55:41, Zorro Lang wrote: > > On Fri, Aug 08, 2025 at 09:31:57PM +0200, Andrey Albershteyn wrote: > > > Add a test to test basic functionality of file_getattr() and > > > file_setattr() syscalls. Most of the work is done in file_attr > > > utility. > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > > --- > > > tests/generic/2000 | 113 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > tests/generic/2000.out | 37 ++++++++++++++++ > > > 2 files changed, 150 insertions(+) > > > > > > diff --git a/tests/generic/2000 b/tests/generic/2000 > > > new file mode 100755 > > > index 000000000000..b4410628c241 > > > --- /dev/null > > > +++ b/tests/generic/2000 > > > @@ -0,0 +1,113 @@ > > > +#! /bin/bash > > > +# SPDX-License-Identifier: GPL-2.0 > > > +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved. > > > +# > > > +# FS QA Test No. 2000 > > > +# > > > +# Test file_getattr/file_setattr syscalls > > > +# > > > +. ./common/preamble > > > +_begin_fstest auto > > > + > > > +# Import common functions. > > > +# . ./common/filter > > > + > > > +_wants_kernel_commit xxxxxxxxxxx \ > > > + "fs: introduce file_getattr and file_setattr syscalls" > > > > As this's a new feature test, I'm wondering if we should use a _require_ > > function to check if current kernel and FSTYP supports file_set/getattr > > syscalls, and _notrun if it's not supported, rather than fail the test. > > hmm, I don't see where _require_function is defined There's not that _require_ function, you need to write a new one to check if current kernel and FSTYP supports file_set/getattr syscalls:) e.g. name as _require_file_setattr. You can use your src/file_attr to check that, or update src/feature.c for that. refer to _require_aio or _require_scratch_shutdown. > > Anyway, the _notrun makes more sense, I will look into what to check > for to skip this one if it's not supported > > -- > - Andrey > ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/3] xfs: test quota's project ID on special files 2025-08-08 19:31 ` [PATCH 0/3] Test file_getattr and file_setattr syscalls Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 1/3] file_attr: introduce program to set/get fsxattr Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls Andrey Albershteyn @ 2025-08-08 19:31 ` Andrey Albershteyn 2025-08-11 15:21 ` Darrick J. Wong 2025-08-11 17:46 ` Zorro Lang 2 siblings, 2 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-08 19:31 UTC (permalink / raw) To: fstests Cc: zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn, Andrey Albershteyn From: Andrey Albershteyn <aalbersh@redhat.com> With addition of file_getattr() and file_setattr(), xfs_quota now can set project ID on filesystem inodes behind special files. Previously, quota reporting didn't count inodes of special files created before project initialization. Only new inodes had project ID set. Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> --- tests/xfs/2000 | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/2000.out | 17 ++++++++++++ 2 files changed, 94 insertions(+) diff --git a/tests/xfs/2000 b/tests/xfs/2000 new file mode 100755 index 000000000000..26a0093c1da1 --- /dev/null +++ b/tests/xfs/2000 @@ -0,0 +1,77 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Red Hat. All Rights Reserved. +# +# FS QA Test No. 2000 +# +# Test that XFS can set quota project ID on special files +# +. ./common/preamble +_begin_fstest auto quota + +# Import common functions. +. ./common/quota +. ./common/filter + +_wants_kernel_commit xxxxxxxxxxx \ + "xfs: allow setting file attributes on special files" +_wants_git_commit xfsprogs xxxxxxxxxxx \ + "xfs_quota: utilize file_setattr to set prjid on special files" + +# Modify as appropriate. +_require_scratch +_require_xfs_quota +_require_test_program "af_unix" +_require_symlinks +_require_mknod + +_scratch_mkfs >>$seqres.full 2>&1 +_qmount_option "pquota" +_scratch_mount + +create_af_unix () { + $here/src/af_unix $* || echo af_unix failed +} + +filter_quota() { + _filter_quota | sed "s~$tmp.projects~PROJECTS_FILE~" +} + +projectdir=$SCRATCH_MNT/prj +id=42 + +mkdir $projectdir +mkfifo $projectdir/fifo +mknod $projectdir/chardev c 1 1 +mknod $projectdir/blockdev b 1 1 +create_af_unix $projectdir/socket +touch $projectdir/foo +ln -s $projectdir/foo $projectdir/symlink +touch $projectdir/bar +ln -s $projectdir/bar $projectdir/broken-symlink +rm -f $projectdir/bar + +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "report -inN -p" $SCRATCH_DEV +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota + +# Let's check that we can recreate the project (flags were cleared out) +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "report -inN -p" $SCRATCH_DEV +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota + +# success, all done +status=0 +exit diff --git a/tests/xfs/2000.out b/tests/xfs/2000.out new file mode 100644 index 000000000000..dd3918f1376d --- /dev/null +++ b/tests/xfs/2000.out @@ -0,0 +1,17 @@ +QA output created by 2000 +Setting up project 42 (path SCRATCH_MNT/prj)... +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). +Checking project 42 (path SCRATCH_MNT/prj)... +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). +#0 3 0 0 00 [--------] +#42 8 20 20 00 [--------] + +Clearing project 42 (path SCRATCH_MNT/prj)... +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). +Setting up project 42 (path SCRATCH_MNT/prj)... +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). +#0 3 0 0 00 [--------] +#42 8 20 20 00 [--------] + +Clearing project 42 (path SCRATCH_MNT/prj)... +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). -- 2.49.0 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] xfs: test quota's project ID on special files 2025-08-08 19:31 ` [PATCH 3/3] xfs: test quota's project ID on special files Andrey Albershteyn @ 2025-08-11 15:21 ` Darrick J. Wong 2025-08-11 18:21 ` Andrey Albershteyn 2025-08-11 17:46 ` Zorro Lang 1 sibling, 1 reply; 33+ messages in thread From: Darrick J. Wong @ 2025-08-11 15:21 UTC (permalink / raw) To: Andrey Albershteyn Cc: fstests, zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn On Fri, Aug 08, 2025 at 09:31:58PM +0200, Andrey Albershteyn wrote: > From: Andrey Albershteyn <aalbersh@redhat.com> > > With addition of file_getattr() and file_setattr(), xfs_quota now can > set project ID on filesystem inodes behind special files. Previously, > quota reporting didn't count inodes of special files created before > project initialization. Only new inodes had project ID set. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > tests/xfs/2000 | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/2000.out | 17 ++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/tests/xfs/2000 b/tests/xfs/2000 > new file mode 100755 > index 000000000000..26a0093c1da1 > --- /dev/null > +++ b/tests/xfs/2000 > @@ -0,0 +1,77 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2024 Red Hat. All Rights Reserved. > +# > +# FS QA Test No. 2000 > +# > +# Test that XFS can set quota project ID on special files > +# > +. ./common/preamble > +_begin_fstest auto quota > + > +# Import common functions. > +. ./common/quota > +. ./common/filter > + > +_wants_kernel_commit xxxxxxxxxxx \ > + "xfs: allow setting file attributes on special files" > +_wants_git_commit xfsprogs xxxxxxxxxxx \ > + "xfs_quota: utilize file_setattr to set prjid on special files" These syscalls aren't going to be backported to old kernels, so I think these two tests are going to need a _require_file_getattr to skip them. --D > + > +# Modify as appropriate. > +_require_scratch > +_require_xfs_quota > +_require_test_program "af_unix" > +_require_symlinks > +_require_mknod > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_qmount_option "pquota" > +_scratch_mount > + > +create_af_unix () { > + $here/src/af_unix $* || echo af_unix failed > +} > + > +filter_quota() { > + _filter_quota | sed "s~$tmp.projects~PROJECTS_FILE~" > +} > + > +projectdir=$SCRATCH_MNT/prj > +id=42 > + > +mkdir $projectdir > +mkfifo $projectdir/fifo > +mknod $projectdir/chardev c 1 1 > +mknod $projectdir/blockdev b 1 1 > +create_af_unix $projectdir/socket > +touch $projectdir/foo > +ln -s $projectdir/foo $projectdir/symlink > +touch $projectdir/bar > +ln -s $projectdir/bar $projectdir/broken-symlink > +rm -f $projectdir/bar > + > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "report -inN -p" $SCRATCH_DEV > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota > + > +# Let's check that we can recreate the project (flags were cleared out) > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "report -inN -p" $SCRATCH_DEV > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota > + > +# success, all done > +status=0 > +exit > diff --git a/tests/xfs/2000.out b/tests/xfs/2000.out > new file mode 100644 > index 000000000000..dd3918f1376d > --- /dev/null > +++ b/tests/xfs/2000.out > @@ -0,0 +1,17 @@ > +QA output created by 2000 > +Setting up project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +Checking project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +#0 3 0 0 00 [--------] > +#42 8 20 20 00 [--------] > + > +Clearing project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +Setting up project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +#0 3 0 0 00 [--------] > +#42 8 20 20 00 [--------] > + > +Clearing project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] xfs: test quota's project ID on special files 2025-08-11 15:21 ` Darrick J. Wong @ 2025-08-11 18:21 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 18:21 UTC (permalink / raw) To: Darrick J. Wong Cc: fstests, zlang, linux-fsdevel, linux-xfs, Andrey Albershteyn On 2025-08-11 08:21:09, Darrick J. Wong wrote: > On Fri, Aug 08, 2025 at 09:31:58PM +0200, Andrey Albershteyn wrote: > > From: Andrey Albershteyn <aalbersh@redhat.com> > > > > With addition of file_getattr() and file_setattr(), xfs_quota now can > > set project ID on filesystem inodes behind special files. Previously, > > quota reporting didn't count inodes of special files created before > > project initialization. Only new inodes had project ID set. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > tests/xfs/2000 | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/xfs/2000.out | 17 ++++++++++++ > > 2 files changed, 94 insertions(+) > > > > diff --git a/tests/xfs/2000 b/tests/xfs/2000 > > new file mode 100755 > > index 000000000000..26a0093c1da1 > > --- /dev/null > > +++ b/tests/xfs/2000 > > @@ -0,0 +1,77 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2024 Red Hat. All Rights Reserved. > > +# > > +# FS QA Test No. 2000 > > +# > > +# Test that XFS can set quota project ID on special files > > +# > > +. ./common/preamble > > +_begin_fstest auto quota > > + > > +# Import common functions. > > +. ./common/quota > > +. ./common/filter > > + > > +_wants_kernel_commit xxxxxxxxxxx \ > > + "xfs: allow setting file attributes on special files" > > +_wants_git_commit xfsprogs xxxxxxxxxxx \ > > + "xfs_quota: utilize file_setattr to set prjid on special files" > > These syscalls aren't going to be backported to old kernels, so I think > these two tests are going to need a _require_file_getattr to skip them. > > --D > will replace it here and in generic/ test -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] xfs: test quota's project ID on special files 2025-08-08 19:31 ` [PATCH 3/3] xfs: test quota's project ID on special files Andrey Albershteyn 2025-08-11 15:21 ` Darrick J. Wong @ 2025-08-11 17:46 ` Zorro Lang 2025-08-11 18:20 ` Andrey Albershteyn 1 sibling, 1 reply; 33+ messages in thread From: Zorro Lang @ 2025-08-11 17:46 UTC (permalink / raw) To: Andrey Albershteyn; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On Fri, Aug 08, 2025 at 09:31:58PM +0200, Andrey Albershteyn wrote: > From: Andrey Albershteyn <aalbersh@redhat.com> > > With addition of file_getattr() and file_setattr(), xfs_quota now can > set project ID on filesystem inodes behind special files. Previously, > quota reporting didn't count inodes of special files created before > project initialization. Only new inodes had project ID set. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > --- > tests/xfs/2000 | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/2000.out | 17 ++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/tests/xfs/2000 b/tests/xfs/2000 > new file mode 100755 > index 000000000000..26a0093c1da1 > --- /dev/null > +++ b/tests/xfs/2000 > @@ -0,0 +1,77 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2024 Red Hat. All Rights Reserved. > +# > +# FS QA Test No. 2000 > +# > +# Test that XFS can set quota project ID on special files > +# > +. ./common/preamble > +_begin_fstest auto quota > + > +# Import common functions. > +. ./common/quota > +. ./common/filter > + > +_wants_kernel_commit xxxxxxxxxxx \ > + "xfs: allow setting file attributes on special files" > +_wants_git_commit xfsprogs xxxxxxxxxxx \ > + "xfs_quota: utilize file_setattr to set prjid on special files" > + > +# Modify as appropriate. > +_require_scratch > +_require_xfs_quota > +_require_test_program "af_unix" > +_require_symlinks > +_require_mknod > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_qmount_option "pquota" > +_scratch_mount > + > +create_af_unix () { > + $here/src/af_unix $* || echo af_unix failed > +} > + > +filter_quota() { > + _filter_quota | sed "s~$tmp.projects~PROJECTS_FILE~" > +} > + > +projectdir=$SCRATCH_MNT/prj > +id=42 > + > +mkdir $projectdir > +mkfifo $projectdir/fifo > +mknod $projectdir/chardev c 1 1 > +mknod $projectdir/blockdev b 1 1 > +create_af_unix $projectdir/socket > +touch $projectdir/foo > +ln -s $projectdir/foo $projectdir/symlink > +touch $projectdir/bar > +ln -s $projectdir/bar $projectdir/broken-symlink > +rm -f $projectdir/bar > + > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "report -inN -p" $SCRATCH_DEV > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota > + > +# Let's check that we can recreate the project (flags were cleared out) > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "report -inN -p" $SCRATCH_DEV > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota > + > +# success, all done > +status=0 > +exit > diff --git a/tests/xfs/2000.out b/tests/xfs/2000.out > new file mode 100644 > index 000000000000..dd3918f1376d > --- /dev/null > +++ b/tests/xfs/2000.out > @@ -0,0 +1,17 @@ > +QA output created by 2000 > +Setting up project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +Checking project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +#0 3 0 0 00 [--------] Better to filter out the root quota report, it might fail on some test environments. Thanks, Zorro > +#42 8 20 20 00 [--------] > + > +Clearing project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +Setting up project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > +#0 3 0 0 00 [--------] > +#42 8 20 20 00 [--------] > + > +Clearing project 42 (path SCRATCH_MNT/prj)... > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > > -- > 2.49.0 > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] xfs: test quota's project ID on special files 2025-08-11 17:46 ` Zorro Lang @ 2025-08-11 18:20 ` Andrey Albershteyn 0 siblings, 0 replies; 33+ messages in thread From: Andrey Albershteyn @ 2025-08-11 18:20 UTC (permalink / raw) To: Zorro Lang; +Cc: fstests, linux-fsdevel, linux-xfs, Andrey Albershteyn On 2025-08-12 01:46:13, Zorro Lang wrote: > On Fri, Aug 08, 2025 at 09:31:58PM +0200, Andrey Albershteyn wrote: > > From: Andrey Albershteyn <aalbersh@redhat.com> > > > > With addition of file_getattr() and file_setattr(), xfs_quota now can > > set project ID on filesystem inodes behind special files. Previously, > > quota reporting didn't count inodes of special files created before > > project initialization. Only new inodes had project ID set. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > --- > > tests/xfs/2000 | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/xfs/2000.out | 17 ++++++++++++ > > 2 files changed, 94 insertions(+) > > > > diff --git a/tests/xfs/2000 b/tests/xfs/2000 > > new file mode 100755 > > index 000000000000..26a0093c1da1 > > --- /dev/null > > +++ b/tests/xfs/2000 > > @@ -0,0 +1,77 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2024 Red Hat. All Rights Reserved. > > +# > > +# FS QA Test No. 2000 > > +# > > +# Test that XFS can set quota project ID on special files > > +# > > +. ./common/preamble > > +_begin_fstest auto quota > > + > > +# Import common functions. > > +. ./common/quota > > +. ./common/filter > > + > > +_wants_kernel_commit xxxxxxxxxxx \ > > + "xfs: allow setting file attributes on special files" > > +_wants_git_commit xfsprogs xxxxxxxxxxx \ > > + "xfs_quota: utilize file_setattr to set prjid on special files" > > + > > +# Modify as appropriate. > > +_require_scratch > > +_require_xfs_quota > > +_require_test_program "af_unix" > > +_require_symlinks > > +_require_mknod > > + > > +_scratch_mkfs >>$seqres.full 2>&1 > > +_qmount_option "pquota" > > +_scratch_mount > > + > > +create_af_unix () { > > + $here/src/af_unix $* || echo af_unix failed > > +} > > + > > +filter_quota() { > > + _filter_quota | sed "s~$tmp.projects~PROJECTS_FILE~" > > +} > > + > > +projectdir=$SCRATCH_MNT/prj > > +id=42 > > + > > +mkdir $projectdir > > +mkfifo $projectdir/fifo > > +mknod $projectdir/chardev c 1 1 > > +mknod $projectdir/blockdev b 1 1 > > +create_af_unix $projectdir/socket > > +touch $projectdir/foo > > +ln -s $projectdir/foo $projectdir/symlink > > +touch $projectdir/bar > > +ln -s $projectdir/bar $projectdir/broken-symlink > > +rm -f $projectdir/bar > > + > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "project -cp $projectdir $id" $SCRATCH_DEV | filter_quota > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "report -inN -p" $SCRATCH_DEV > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota > > + > > +# Let's check that we can recreate the project (flags were cleared out) > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "project -sp $projectdir $id" $SCRATCH_DEV | filter_quota > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "limit -p isoft=20 ihard=20 $id " $SCRATCH_DEV | filter_quota > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "report -inN -p" $SCRATCH_DEV > > +$XFS_QUOTA_PROG -D $tmp.projects -P $tmp.projid -x \ > > + -c "project -Cp $projectdir $id" $SCRATCH_DEV | filter_quota > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/xfs/2000.out b/tests/xfs/2000.out > > new file mode 100644 > > index 000000000000..dd3918f1376d > > --- /dev/null > > +++ b/tests/xfs/2000.out > > @@ -0,0 +1,17 @@ > > +QA output created by 2000 > > +Setting up project 42 (path SCRATCH_MNT/prj)... > > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > > +Checking project 42 (path SCRATCH_MNT/prj)... > > +Processed 1 (PROJECTS_FILE and cmdline) paths for project 42 with recursion depth infinite (-1). > > +#0 3 0 0 00 [--------] > > Better to filter out the root quota report, it might fail on some test environments. > sure -- - Andrey ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2025-08-12 17:28 UTC | newest] Thread overview: 33+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-08 19:28 Tests for file_getattr()/file_setattr() and xfsprogs update Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 0/4] xfsprogs: utilize file_getattr() and file_setattr() Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 1/4] libfrog: add wrappers for file_getattr/file_setattr syscalls Andrey Albershteyn 2025-08-11 15:02 ` Darrick J. Wong 2025-08-11 17:44 ` Andrey Albershteyn 2025-08-12 14:53 ` Darrick J. Wong 2025-08-08 19:30 ` [PATCH 2/4] xfs_quota: utilize file_setattr to set prjid on special files Andrey Albershteyn 2025-08-11 15:07 ` Darrick J. Wong 2025-08-11 17:51 ` Andrey Albershteyn 2025-08-08 19:30 ` [PATCH 3/4] xfs_io: make ls/chattr work with " Andrey Albershteyn 2025-08-11 15:12 ` Darrick J. Wong 2025-08-11 17:57 ` Andrey Albershteyn 2025-08-12 17:28 ` Darrick J. Wong 2025-08-08 19:30 ` [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump Andrey Albershteyn 2025-08-11 15:14 ` Darrick J. Wong 2025-08-11 17:59 ` Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 0/3] Test file_getattr and file_setattr syscalls Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 1/3] file_attr: introduce program to set/get fsxattr Andrey Albershteyn 2025-08-11 15:23 ` Darrick J. Wong 2025-08-11 18:06 ` Andrey Albershteyn 2025-08-11 17:51 ` Zorro Lang 2025-08-11 18:12 ` Andrey Albershteyn 2025-08-08 19:31 ` [PATCH 2/3] generic: introduce test to test file_getattr/file_setattr syscalls Andrey Albershteyn 2025-08-11 15:17 ` Darrick J. Wong 2025-08-11 18:13 ` Andrey Albershteyn 2025-08-11 17:55 ` Zorro Lang 2025-08-11 18:18 ` Andrey Albershteyn 2025-08-11 18:43 ` Zorro Lang 2025-08-08 19:31 ` [PATCH 3/3] xfs: test quota's project ID on special files Andrey Albershteyn 2025-08-11 15:21 ` Darrick J. Wong 2025-08-11 18:21 ` Andrey Albershteyn 2025-08-11 17:46 ` Zorro Lang 2025-08-11 18:20 ` Andrey Albershteyn
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).