* [PATCHBOMB v30.10] xfsprogs-6.10: filesystem properties
@ 2024-08-06 18:14 Darrick J. Wong
2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong
` (3 more replies)
0 siblings, 4 replies; 30+ messages in thread
From: Darrick J. Wong @ 2024-08-06 18:14 UTC (permalink / raw)
To: Christoph Hellwig, Dave Chinner; +Cc: xfs, fstests
Hi all,
After recent discussions about how to allow sysadmins to opt in or out
of automatick fsck for XFS[1][2], I now have a patchset for xfsprogs
6.10 that implements the filesystem properties that we talked about.
As a refresher, the design I settled on is to add ATTR_ROOT (aka
"trusted") xattrs to the root directory. ATTR_ROOT xattrs can only be
accessed by processes with CAP_SYS_ADMIN, so unprivileged userspace
can't mess with the sysadmin's configured preferences.
I decided that all fs properties should have "xfs:" in the name to make
them look distinct, and defined "trusted.xfs:autofsck" as the property
that controls the behavior of automatic fsck. There's a new wrapper
program "xfs_property" that one can use to administer the properties.
xfs_scrub{,bed} uses the property; and mkfs.xfs can set it for you at
format time.
# mkfs.xfs -m autofsck /dev/sda
# xfs_property /dev/sda get autofsck
repair
# mount /dev/sda /mnt
# xfs_property /mnt set autofsck=check
# xfs_scrub -o autofsck /mnt
Info: /mnt: Checking per autofsck directive.
--D
[1] https://lore.kernel.org/linux-xfs/20240724213852.GA612460@frogsfrogsfrogs/
[2] https://lore.kernel.org/linux-xfs/20240730031030.GA6333@frogsfrogsfrogs/
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHSET v30.10 1/3] xfsprogs: filesystem properties 2024-08-06 18:14 [PATCHBOMB v30.10] xfsprogs-6.10: filesystem properties Darrick J. Wong @ 2024-08-06 18:18 ` Darrick J. Wong 2024-08-06 18:19 ` [PATCH 1/7] libfrog: support editing filesystem property sets Darrick J. Wong ` (6 more replies) 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong ` (2 subsequent siblings) 3 siblings, 7 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:18 UTC (permalink / raw) To: djwong, cem Cc: Dave Chinner, Christoph Hellwig, Dave Chinner, Christoph Hellwig, hch, dchinner, fstests, linux-xfs Hi all, It would be very useful if system administrators could set properties for a given xfs filesystem to control its behavior. This we can do easily and extensibly by setting ATTR_ROOT (aka "trusted") extended attributes on the root directory. To prevent this from becoming a weird free for all, let's add some library and tooling support so that sysadmins simply run the xfs_property program to administer these properties. If you're going to start using this code, I strongly recommend pulling from my git trees, which are linked below. This has been running on the djcloud for months with no problems. Enjoy! Comments and questions are, as always, welcome. xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=filesystem-properties-6.10 --- Commits in this patchset: * libfrog: support editing filesystem property sets * xfs_io: edit filesystem properties * xfs_db: improve getting and setting extended attributes * libxfs: hoist listxattr from xfs_repair * libxfs: pass a transaction context through listxattr * xfs_db: add a command to list xattrs * xfs_property: add a new tool to administer fs properties --- db/attrset.c | 463 ++++++++++++++++++++++++++++++++++++++++++++++- io/Makefile | 4 io/fsproperties.c | 365 +++++++++++++++++++++++++++++++++++++ io/init.c | 1 io/io.h | 1 io/xfs_property | 77 ++++++++ libfrog/Makefile | 7 + libfrog/fsproperties.c | 39 ++++ libfrog/fsproperties.h | 53 +++++ libfrog/fsprops.c | 202 +++++++++++++++++++++ libfrog/fsprops.h | 34 +++ libxfs/Makefile | 2 libxfs/listxattr.c | 42 ++-- libxfs/listxattr.h | 17 ++ man/man8/xfs_db.8 | 68 +++++++ man/man8/xfs_io.8 | 16 ++ man/man8/xfs_property.8 | 61 ++++++ repair/Makefile | 2 repair/listxattr.h | 15 -- repair/pptr.c | 9 + 20 files changed, 1430 insertions(+), 48 deletions(-) create mode 100644 io/fsproperties.c create mode 100755 io/xfs_property create mode 100644 libfrog/fsproperties.c create mode 100644 libfrog/fsproperties.h create mode 100644 libfrog/fsprops.c create mode 100644 libfrog/fsprops.h rename repair/listxattr.c => libxfs/listxattr.c (84%) create mode 100644 libxfs/listxattr.h create mode 100644 man/man8/xfs_property.8 delete mode 100644 repair/listxattr.h ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/7] libfrog: support editing filesystem property sets 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong @ 2024-08-06 18:19 ` Darrick J. Wong 2024-08-06 18:19 ` [PATCH 2/7] xfs_io: edit filesystem properties Darrick J. Wong ` (5 subsequent siblings) 6 siblings, 0 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:19 UTC (permalink / raw) To: djwong, cem Cc: Christoph Hellwig, Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add some library functions so that spaceman and scrub can share the same code to edit and retrieve filesystem properties. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Dave Chinner <dchinner@redhat.com> --- libfrog/Makefile | 7 ++ libfrog/fsproperties.c | 39 +++++++++ libfrog/fsproperties.h | 53 +++++++++++++ libfrog/fsprops.c | 202 ++++++++++++++++++++++++++++++++++++++++++++++++ libfrog/fsprops.h | 34 ++++++++ 5 files changed, 335 insertions(+) create mode 100644 libfrog/fsproperties.c create mode 100644 libfrog/fsproperties.h create mode 100644 libfrog/fsprops.c create mode 100644 libfrog/fsprops.h diff --git a/libfrog/Makefile b/libfrog/Makefile index 0b5b23893..acddc894e 100644 --- a/libfrog/Makefile +++ b/libfrog/Makefile @@ -20,6 +20,7 @@ convert.c \ crc32.c \ file_exchange.c \ fsgeom.c \ +fsproperties.c \ getparents.c \ histogram.c \ list_sort.c \ @@ -47,6 +48,7 @@ dahashselftest.h \ div64.h \ file_exchange.h \ fsgeom.h \ +fsproperties.h \ getparents.h \ histogram.h \ logging.h \ @@ -60,6 +62,11 @@ workqueue.h LSRCFILES += gen_crc32table.c +ifeq ($(HAVE_LIBATTR),yes) +CFILES+=fsprops.c +HFILES+=fsprops.h +endif + LDIRT = gen_crc32table crc32table.h default: ltdepend $(LTLIBRARY) diff --git a/libfrog/fsproperties.c b/libfrog/fsproperties.c new file mode 100644 index 000000000..c317d15c1 --- /dev/null +++ b/libfrog/fsproperties.c @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2024 Oracle. All Rights Reserved. + * Author: Darrick J. Wong <djwong@kernel.org> + */ +#include <string.h> +#include "xfs.h" +#include "libfrog/fsgeom.h" +#include "libfrog/fsproperties.h" +#include "list.h" + +/* Return the offset of a string in a string table, or -1 if not found. */ +static inline int +__fsprops_lookup( + const char *values[], + unsigned int nr_values, + const char *value) +{ + unsigned int i; + + for (i = 0; i < nr_values; i++) { + if (values[i] && !strcmp(value, values[i])) + return i; + } + + return -1; +} + +#define fsprops_lookup(values, value) \ + __fsprops_lookup((values), ARRAY_SIZE(values), (value)) + +/* Return true if a fs property name=value tuple is allowed. */ +bool +fsprop_validate( + const char *name, + const char *value) +{ + return true; +} diff --git a/libfrog/fsproperties.h b/libfrog/fsproperties.h new file mode 100644 index 000000000..b1ac4cdd7 --- /dev/null +++ b/libfrog/fsproperties.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (c) 2024 Oracle. All Rights Reserved. + * Author: Darrick J. Wong <djwong@kernel.org> + */ +#ifndef __LIBFROG_FSPROPERTIES_H__ +#define __LIBFROG_FSPROPERTIES_H__ + +/* Name space for filesystem properties. */ +#define FSPROP_NAMESPACE "trusted." + +/* + * All filesystem property xattr names must have this string after the + * namespace. For example, the VFS xattr calls should use the name + * "trusted.xfs:fubar". The xfs xattr ioctls would set ATTR_ROOT and use the + * name "xfs:fubar". + */ +#define FSPROP_NAME_PREFIX "xfs:" + +/* Maximum size the value of a filesystem property. */ +#define FSPROP_MAX_VALUELEN (65536) + +static inline int +fsprop_name_to_attr_name( + const char *prop_name, + char **attr_name) +{ + return asprintf(attr_name, FSPROP_NAME_PREFIX "%s", prop_name); +} + +static inline const char * +attr_name_to_fsprop_name( + const char *attr_name) +{ + const size_t bytes = sizeof(FSPROP_NAME_PREFIX) - 1; + unsigned int i; + + for (i = 0; i < bytes; i++) { + if (attr_name[i] == 0) + return NULL; + } + + if (memcmp(attr_name, FSPROP_NAME_PREFIX, bytes) != 0) + return NULL; + + return attr_name + bytes; +} + +bool fsprop_validate(const char *name, const char *value); + +/* Specific Filesystem Properties */ + +#endif /* __LIBFROG_FSPROPERTIES_H__ */ diff --git a/libfrog/fsprops.c b/libfrog/fsprops.c new file mode 100644 index 000000000..88046b7a0 --- /dev/null +++ b/libfrog/fsprops.c @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2024 Oracle. All Rights Reserved. + * Author: Darrick J. Wong <djwong@kernel.org> + */ +#include "xfs.h" +#include "handle.h" +#include "libfrog/fsgeom.h" +#include "libfrog/paths.h" +#include "libfrog/bulkstat.h" +#include "libfrog/fsprops.h" +#include "libfrog/fsproperties.h" + +#include <attr/attributes.h> + +/* + * Given an xfd and a mount table path, get us the handle for the root dir so + * we can set filesystem properties. + */ +int +fsprops_open_handle( + struct xfs_fd *xfd, + struct fs_path *fs_path, + struct fsprops_handle *fph) +{ + struct xfs_bulkstat bulkstat; + struct stat sb; + int ret; + + /* fs properties only supported on V5 filesystems */ + if (!(xfd->fsgeom.flags & XFS_FSOP_GEOM_FLAGS_V5SB)) { + errno = EOPNOTSUPP; + return -1; + } + + ret = -xfrog_bulkstat_single(xfd, XFS_BULK_IREQ_SPECIAL_ROOT, + XFS_BULK_IREQ_SPECIAL, &bulkstat); + if (ret) + return ret; + + ret = fstat(xfd->fd, &sb); + if (ret) + return ret; + + if (sb.st_ino != bulkstat.bs_ino) { + errno = ESRMNT; + return -1; + } + + return fd_to_handle(xfd->fd, &fph->hanp, &fph->hlen); +} + +/* Free a fsproperties handle. */ +void +fsprops_free_handle( + struct fsprops_handle *fph) +{ + if (fph->hanp) + free_handle(fph->hanp, fph->hlen); + fph->hanp = NULL; + fph->hlen = 0; +} + +/* Call the given callback on every fs property accessible through the handle. */ +int +fsprops_walk_names( + struct fsprops_handle *fph, + fsprops_name_walk_fn walk_fn, + void *priv) +{ + struct attrlist_cursor cur = { }; + char attrbuf[XFS_XATTR_LIST_MAX]; + struct attrlist *attrlist = (struct attrlist *)attrbuf; + int ret; + + memset(attrbuf, 0, XFS_XATTR_LIST_MAX); + + while ((ret = attr_list_by_handle(fph->hanp, fph->hlen, attrbuf, + XFS_XATTR_LIST_MAX, XFS_IOC_ATTR_ROOT, + &cur)) == 0) { + unsigned int i; + + for (i = 0; i < attrlist->al_count; i++) { + struct attrlist_ent *ent = ATTR_ENTRY(attrlist, i); + const char *p = + attr_name_to_fsprop_name(ent->a_name); + + if (p) { + ret = walk_fn(fph, p, ent->a_valuelen, priv); + if (ret) + break; + } + } + + if (!attrlist->al_more) + break; + } + + return ret; +} + +/* Retrieve the value of a specific fileystem property. */ +int +fsprops_get( + struct fsprops_handle *fph, + const char *name, + void *valuebuf, + size_t *valuelen) +{ + struct xfs_attr_multiop ops = { + .am_opcode = ATTR_OP_GET, + .am_attrvalue = valuebuf, + .am_length = *valuelen, + .am_flags = XFS_IOC_ATTR_ROOT, + }; + int ret; + + ret = fsprop_name_to_attr_name(name, (char **)&ops.am_attrname); + if (ret < 0) + return ret; + + ret = attr_multi_by_handle(fph->hanp, fph->hlen, &ops, 1, 0); + if (ret < 0) + goto out_name; + + if (ops.am_error) { + errno = -ops.am_error; + ret = -1; + goto out_name; + } + + *valuelen = ops.am_length; +out_name: + free(ops.am_attrname); + return ret; +} + +/* Set the value of a specific fileystem property. */ +int +fsprops_set( + struct fsprops_handle *fph, + const char *name, + void *valuebuf, + size_t valuelen) +{ + struct xfs_attr_multiop ops = { + .am_opcode = ATTR_OP_SET, + .am_attrvalue = valuebuf, + .am_length = valuelen, + .am_flags = XFS_IOC_ATTR_ROOT, + }; + int ret; + + ret = fsprop_name_to_attr_name(name, (char **)&ops.am_attrname); + if (ret < 0) + return ret; + + ret = attr_multi_by_handle(fph->hanp, fph->hlen, &ops, 1, 0); + if (ret < 0) + goto out_name; + + if (ops.am_error) { + errno = -ops.am_error; + ret = -1; + goto out_name; + } + +out_name: + free(ops.am_attrname); + return ret; +} + +/* Unset the value of a specific fileystem property. */ +int +fsprops_remove( + struct fsprops_handle *fph, + const char *name) +{ + struct xfs_attr_multiop ops = { + .am_opcode = ATTR_OP_REMOVE, + .am_flags = XFS_IOC_ATTR_ROOT, + }; + int ret; + + ret = fsprop_name_to_attr_name(name, (char **)&ops.am_attrname); + if (ret < 0) + return ret; + + ret = attr_multi_by_handle(fph->hanp, fph->hlen, &ops, 1, 0); + if (ret < 0) + goto out_name; + + if (ops.am_error) { + errno = -ops.am_error; + ret = -1; + goto out_name; + } + +out_name: + free(ops.am_attrname); + return ret; +} diff --git a/libfrog/fsprops.h b/libfrog/fsprops.h new file mode 100644 index 000000000..9276f2425 --- /dev/null +++ b/libfrog/fsprops.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (c) 2024 Oracle. All Rights Reserved. + * Author: Darrick J. Wong <djwong@kernel.org> + */ +#ifndef __LIBFROG_FSPROPS_H__ +#define __LIBFROG_FSPROPS_H__ + +/* Edit and view filesystem property sets. */ + +struct fsprops_handle { + void *hanp; + size_t hlen; +}; + +struct xfs_fd; +struct fs_path; + +int fsprops_open_handle(struct xfs_fd *xfd, struct fs_path *fspath, + struct fsprops_handle *fph); +void fsprops_free_handle(struct fsprops_handle *fph); + +typedef int (*fsprops_name_walk_fn)(struct fsprops_handle *fph, + const char *name, size_t valuelen, void *priv); + +int fsprops_walk_names(struct fsprops_handle *fph, fsprops_name_walk_fn walk_fn, + void *priv); +int fsprops_get(struct fsprops_handle *fph, const char *name, void *attrbuf, + size_t *attrlen); +int fsprops_set(struct fsprops_handle *fph, const char *name, void *attrbuf, + size_t attrlen); +int fsprops_remove(struct fsprops_handle *fph, const char *name); + +#endif /* __LIBFROG_FSPROPS_H__ */ ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 2/7] xfs_io: edit filesystem properties 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong 2024-08-06 18:19 ` [PATCH 1/7] libfrog: support editing filesystem property sets Darrick J. Wong @ 2024-08-06 18:19 ` Darrick J. Wong 2024-08-07 16:08 ` Christoph Hellwig 2024-08-06 18:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong ` (4 subsequent siblings) 6 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:19 UTC (permalink / raw) To: djwong, cem; +Cc: Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add some new subcommands to xfs_io so that users can administer filesystem properties. Signed-off-by: Darrick J. Wong <djwong@kernel.org>Acked-by: Dave Chinner <dchinner@redhat.com> --- io/Makefile | 1 io/fsproperties.c | 365 +++++++++++++++++++++++++++++++++++++++++++++++++++++ io/init.c | 1 io/io.h | 1 man/man8/xfs_io.8 | 16 ++ 5 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 io/fsproperties.c diff --git a/io/Makefile b/io/Makefile index 3192b813c..0bdd05b57 100644 --- a/io/Makefile +++ b/io/Makefile @@ -20,6 +20,7 @@ CFILES = \ fiemap.c \ file.c \ freeze.c \ + fsproperties.c \ fsuuid.c \ fsync.c \ getrusage.c \ diff --git a/io/fsproperties.c b/io/fsproperties.c new file mode 100644 index 000000000..79837132e --- /dev/null +++ b/io/fsproperties.c @@ -0,0 +1,365 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2024 Oracle. All Rights Reserved. + * Author: Darrick J. Wong <djwong@kernel.org> + */ +#include "platform_defs.h" +#include "command.h" +#include "init.h" +#include "libfrog/paths.h" +#include "input.h" +#include "libfrog/fsgeom.h" +#include "handle.h" +#include "io.h" +#include "libfrog/fsprops.h" +#include "libfrog/fsproperties.h" + +static void +listfsprops_help(void) +{ + printf(_( +"Print the names of the filesystem properties stored in this filesystem.\n" +"\n")); +} + +static int +fileio_to_fsprops_handle( + struct fileio *file, + struct fsprops_handle *fph) +{ + struct xfs_fd xfd = XFS_FD_INIT(file->fd); + struct fs_path *fs; + void *hanp = NULL; + size_t hlen; + int ret; + + /* + * Look up the mount point info for the open file, which confirms that + * we were passed a mount point. + */ + fs = fs_table_lookup(file->name, FS_MOUNT_POINT); + if (!fs) { + fprintf(stderr, _("%s: Not a XFS mount point.\n"), + file->name); + goto bad; + } + + /* + * Register the mountpoint in the fsfd cache so we can use handle + * functions later. + */ + ret = path_to_fshandle(fs->fs_dir, &hanp, &hlen); + if (ret) { + perror(fs->fs_dir); + goto bad; + } + + ret = -xfd_prepare_geometry(&xfd); + if (ret) { + perror(file->name); + goto free_handle; + } + + ret = fsprops_open_handle(&xfd, &file->fs_path, fph); + if (ret) { + if (errno == ESRMNT) + fprintf(stderr, _("%s: Not a XFS mount point.\n"), + file->name); + else + perror(file->name); + goto free_handle; + } + + free_handle(hanp, hlen); + return 0; +free_handle: + free_handle(hanp, hlen); +bad: + exitcode = 1; + return 1; +} + +static int +print_fsprop( + struct fsprops_handle *fph, + const char *name, + size_t unused, + void *priv) +{ + bool *print_values = priv; + char valuebuf[FSPROP_MAX_VALUELEN]; + size_t valuelen = FSPROP_MAX_VALUELEN; + int ret; + + if (!(*print_values)) { + printf("%s\n", name); + return 0; + } + + ret = fsprops_get(fph, name, valuebuf, &valuelen); + if (ret) + return ret; + + printf("%s=%.*s\n", name, (int)valuelen, valuebuf); + return 0; +} + +static int +listfsprops_f( + int argc, + char **argv) +{ + struct fsprops_handle fph = { }; + bool print_values = false; + int c; + int ret; + + while ((c = getopt(argc, argv, "v")) != EOF) { + switch (c) { + case 'v': + print_values = true; + break; + default: + exitcode = 1; + listfsprops_help(); + return 0; + } + } + + ret = fileio_to_fsprops_handle(file, &fph); + if (ret) + return 1; + + ret = fsprops_walk_names(&fph, print_fsprop, &print_values); + if (ret) { + perror(file->name); + exitcode = 1; + } + + fsprops_free_handle(&fph); + return 0; +} + +static struct cmdinfo listfsprops_cmd = { + .name = "listfsprops", + .cfunc = listfsprops_f, + .argmin = 0, + .argmax = -1, + .flags = CMD_NOMAP_OK, + .args = "", + .help = listfsprops_help, +}; + +static void +getfsprops_help(void) +{ + printf(_( +"Print the values of filesystem properties stored in this filesystem.\n" +"\n" +"Pass property names as the arguments.\n" +"\n")); +} + +static int +getfsprops_f( + int argc, + char **argv) +{ + struct fsprops_handle fph = { }; + int c; + int ret; + + while ((c = getopt(argc, argv, "")) != EOF) { + switch (c) { + default: + exitcode = 1; + getfsprops_help(); + return 0; + } + } + + ret = fileio_to_fsprops_handle(file, &fph); + if (ret) + return ret; + + for (c = optind; c < argc; c++) { + char valuebuf[FSPROP_MAX_VALUELEN]; + size_t valuelen = FSPROP_MAX_VALUELEN; + + ret = fsprops_get(&fph, argv[c], valuebuf, &valuelen); + if (ret) { + perror(argv[c]); + exitcode = 1; + break; + } + + printf("%s=%.*s\n", argv[c], (int)valuelen, valuebuf); + } + + fsprops_free_handle(&fph); + return 0; +} + +static struct cmdinfo getfsprops_cmd = { + .name = "getfsprops", + .cfunc = getfsprops_f, + .argmin = 0, + .argmax = -1, + .flags = CMD_NOMAP_OK, + .args = "", + .help = getfsprops_help, +}; + +static void +setfsprops_help(void) +{ + printf(_( +"Set values of filesystem properties stored in this filesystem.\n" +"\n" +" -f Do not try to validate property value.\n" +"\n" +"Provide name=value tuples as the arguments.\n" +"\n")); +} + +static int +setfsprops_f( + int argc, + char **argv) +{ + struct fsprops_handle fph = { }; + bool force = false; + int c; + int ret; + + while ((c = getopt(argc, argv, "f")) != EOF) { + switch (c) { + case 'f': + force = true; + break; + default: + exitcode = 1; + getfsprops_help(); + return 0; + } + } + + ret = fileio_to_fsprops_handle(file, &fph); + if (ret) + return ret; + + for (c = optind; c < argc; c ++) { + char *equals = strchr(argv[c], '='); + + if (!equals) { + fprintf(stderr, _("%s: property value required.\n"), + argv[c]); + exitcode = 1; + break; + } + + *equals = 0; + + if (!force && !fsprop_validate(argv[c], equals + 1)) { + fprintf(stderr, _("%s: invalid value \"%s\".\n"), + argv[c], equals + 1); + *equals = '='; + exitcode = 1; + break; + } + + ret = fsprops_set(&fph, argv[c], equals + 1, + strlen(equals + 1)); + if (ret) { + perror(argv[c]); + *equals = '='; + exitcode = 1; + break; + } + + printf("%s=%s\n", argv[c], equals + 1); + *equals = '='; + } + + fsprops_free_handle(&fph); + return 0; +} + +static struct cmdinfo setfsprops_cmd = { + .name = "setfsprops", + .cfunc = setfsprops_f, + .argmin = 0, + .argmax = -1, + .flags = CMD_NOMAP_OK, + .args = "", + .help = setfsprops_help, +}; + +static void +removefsprops_help(void) +{ + printf(_( +"Unset a filesystem property.\n" +"\n" +"Pass property names as the arguments.\n" +"\n")); +} + +static int +removefsprops_f( + int argc, + char **argv) +{ + struct fsprops_handle fph = { }; + int c; + int ret; + + while ((c = getopt(argc, argv, "")) != EOF) { + switch (c) { + default: + exitcode = 1; + getfsprops_help(); + return 0; + } + } + + ret = fileio_to_fsprops_handle(file, &fph); + if (ret) + return ret; + + for (c = optind; c < argc; c++) { + ret = fsprops_remove(&fph, argv[c]); + if (ret) { + perror(argv[c]); + exitcode = 1; + break; + } + } + + fsprops_free_handle(&fph); + return 0; +} + +static struct cmdinfo removefsprops_cmd = { + .name = "removefsprops", + .cfunc = removefsprops_f, + .argmin = 0, + .argmax = -1, + .flags = CMD_NOMAP_OK, + .args = "", + .help = removefsprops_help, +}; + +void +fsprops_init(void) +{ + listfsprops_cmd.oneline = _("list file system properties"); + getfsprops_cmd.oneline = _("print file system properties"); + setfsprops_cmd.oneline = _("set file system properties"); + removefsprops_cmd.oneline = _("unset file system properties"); + + add_command(&listfsprops_cmd); + add_command(&getfsprops_cmd); + add_command(&setfsprops_cmd); + add_command(&removefsprops_cmd); +} diff --git a/io/init.c b/io/init.c index 37e0f093c..5727f7351 100644 --- a/io/init.c +++ b/io/init.c @@ -89,6 +89,7 @@ init_commands(void) utimes_init(); crc32cselftest_init(); exchangerange_init(); + fsprops_init(); } /* diff --git a/io/io.h b/io/io.h index fdb715ff0..8c5e59100 100644 --- a/io/io.h +++ b/io/io.h @@ -150,3 +150,4 @@ extern void repair_init(void); extern void crc32cselftest_init(void); extern void bulkstat_init(void); void exchangerange_init(void); +void fsprops_init(void); diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 index 657bdaec4..303c64478 100644 --- a/man/man8/xfs_io.8 +++ b/man/man8/xfs_io.8 @@ -1578,7 +1578,21 @@ Print the sysfs or debugfs path for the mounted filesystem. The .B -d option selects debugfs instead of sysfs. - +.TP +.BI "getfsprops " name " [ " names "... ]" +Retrieve the values of the given filesystem properties. +.TP +.BI "listfsprops [ " \-v " ]" +List all filesystem properties that have been stored in the filesystem. +If the +.B \-v +flag is specified, prints the values of the properties. +.TP +.BI "setfsprops " name = value " [ " name = value "... ]" +Set the given filesystem properties to the specified values. +.TP +.BI "removefsprops " name " [ " names "... ]" +Remove the given filesystem properties. .SH OTHER COMMANDS .TP ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 2/7] xfs_io: edit filesystem properties 2024-08-06 18:19 ` [PATCH 2/7] xfs_io: edit filesystem properties Darrick J. Wong @ 2024-08-07 16:08 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:08 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, Dave Chinner, hch, fstests, linux-xfs On Tue, Aug 06, 2024 at 11:19:50AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Add some new subcommands to xfs_io so that users can administer > filesystem properties. s/some //? > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>Acked-by: Dave Chinner <dchinner@redhat.com> Missing newline here. > + if (!(*print_values)) { The inner set of braces here should not be needed. Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/7] xfs_db: improve getting and setting extended attributes 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong 2024-08-06 18:19 ` [PATCH 1/7] libfrog: support editing filesystem property sets Darrick J. Wong 2024-08-06 18:19 ` [PATCH 2/7] xfs_io: edit filesystem properties Darrick J. Wong @ 2024-08-06 18:20 ` Darrick J. Wong 2024-08-07 16:10 ` Christoph Hellwig 2024-08-06 18:20 ` [PATCH 4/7] libxfs: hoist listxattr from xfs_repair Darrick J. Wong ` (3 subsequent siblings) 6 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:20 UTC (permalink / raw) To: djwong, cem Cc: Christoph Hellwig, Christoph Hellwig, Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add an attr_get command to retrieve the value of an xattr from a file; and extend the attr_set command to allow passing of string values. Signed-off-by: Darrick J. Wong <djwong@kernel.org>Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Dave Chinner <dchinner@redhat.com> --- db/attrset.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++++++- man/man8/xfs_db.8 | 40 ++++++++ 2 files changed, 293 insertions(+), 9 deletions(-) diff --git a/db/attrset.c b/db/attrset.c index 81d530055..9e53e63c9 100644 --- a/db/attrset.c +++ b/db/attrset.c @@ -17,20 +17,43 @@ #include "inode.h" #include "malloc.h" #include <sys/xattr.h> +#include "libfrog/fsproperties.h" +static int attr_get_f(int argc, char **argv); static int attr_set_f(int argc, char **argv); static int attr_remove_f(int argc, char **argv); +static void attrget_help(void); static void attrset_help(void); +static const cmdinfo_t attr_get_cmd = + { "attr_get", "aget", attr_get_f, 1, -1, 0, + N_("[-r|-s|-u|-p|-Z] name"), + N_("get the named attribute on the current inode"), attrget_help }; static const cmdinfo_t attr_set_cmd = { "attr_set", "aset", attr_set_f, 1, -1, 0, - N_("[-r|-s|-u|-p] [-n] [-R|-C] [-v n] name"), + N_("[-r|-s|-u|-p|-Z] [-n] [-R|-C] [-v n] name"), N_("set the named attribute on the current inode"), attrset_help }; static const cmdinfo_t attr_remove_cmd = { "attr_remove", "aremove", attr_remove_f, 1, -1, 0, - N_("[-r|-s|-u|-p] [-n] name"), + N_("[-r|-s|-u|-p|-Z] [-n] name"), N_("remove the named attribute from the current inode"), attrset_help }; +static void +attrget_help(void) +{ + dbprintf(_( +"\n" +" The attr_get command provide interfaces for retrieving the values of extended\n" +" attributes of a file. This command requires attribute names to be specified.\n" +" There are 4 namespace flags:\n" +" -r -- 'root'\n" +" -u -- 'user' (default)\n" +" -s -- 'secure'\n" +" -p -- 'parent'\n" +" -Z -- fs property\n" +"\n")); +} + static void attrset_help(void) { @@ -45,10 +68,15 @@ attrset_help(void) " -u -- 'user' (default)\n" " -s -- 'secure'\n" " -p -- 'parent'\n" +" -Z -- fs property\n" "\n" " For attr_set, these options further define the type of set operation:\n" " -C -- 'create' - create attribute, fail if it already exists\n" " -R -- 'replace' - replace attribute, fail if it does not exist\n" +"\n" +" If the attribute value is a string, it can be specified after the\n" +" attribute name.\n" +"\n" " The backward compatibility mode 'noattr2' can be emulated (-n) also.\n" "\n")); } @@ -59,6 +87,7 @@ attrset_init(void) if (!expert_mode) return; + add_command(&attr_get_cmd); add_command(&attr_set_cmd); add_command(&attr_remove_cmd); } @@ -106,6 +135,58 @@ get_buf_from_file( LIBXFS_ATTR_ROOT | \ LIBXFS_ATTR_PARENT) +static bool +adjust_fsprop_attr_name( + struct xfs_da_args *args, + bool *free_name) +{ + const char *o = (const char *)args->name; + char *p; + int ret; + + if ((args->attr_filter & LIBXFS_ATTR_NS) != LIBXFS_ATTR_ROOT) { + dbprintf(_("fs properties must be ATTR_ROOT\n")); + return false; + } + + ret = fsprop_name_to_attr_name(o, &p); + if (ret < 0) { + dbprintf(_("could not allocate fs property name string\n")); + return false; + } + args->namelen = ret; + args->name = (const uint8_t *)p; + + if (*free_name) + free((void *)o); + *free_name = true; + + if (args->namelen > MAXNAMELEN) { + dbprintf(_("%s: name too long\n"), args->name); + return false; + } + + if (args->valuelen > ATTR_MAX_VALUELEN) { + dbprintf(_("%s: value too long\n"), args->name); + return false; + } + + return true; +} + +static void +print_fsprop( + struct xfs_da_args *args) +{ + const char *p = + attr_name_to_fsprop_name((const char *)args->name); + + if (p) + printf("%s=%.*s\n", p, args->valuelen, (char *)args->value); + else + fprintf(stderr, _("%s: not a fs property?\n"), args->name); +} + static int attr_set_f( int argc, @@ -119,7 +200,9 @@ attr_set_f( char *sp; char *name_from_file = NULL; char *value_from_file = NULL; + bool free_name = false; enum xfs_attr_update op = XFS_ATTRUPDATE_UPSERT; + bool fsprop = false; int c; int error; @@ -132,9 +215,12 @@ attr_set_f( return 0; } - while ((c = getopt(argc, argv, "ruspCRnN:v:V:")) != EOF) { + while ((c = getopt(argc, argv, "ruspCRnN:v:V:Z")) != EOF) { switch (c) { /* namespaces */ + case 'Z': + fsprop = true; + fallthrough; case 'r': args.attr_filter &= ~LIBXFS_ATTR_NS; args.attr_filter |= LIBXFS_ATTR_ROOT; @@ -213,9 +299,10 @@ attr_set_f( if (!args.name) return 0; + free_name = true; args.namelen = namelen; } else { - if (optind != argc - 1) { + if (optind != argc - 1 && optind != argc - 2) { dbprintf(_("too few options for attr_set (no name given)\n")); return 0; } @@ -250,6 +337,25 @@ attr_set_f( goto out; } memset(args.value, 'v', args.valuelen); + } else if (optind == argc - 2) { + args.valuelen = strlen(argv[optind + 1]); + args.value = strdup(argv[optind + 1]); + if (!args.value) { + dbprintf(_("cannot allocate buffer (%d)\n"), + args.valuelen); + goto out; + } + } + + if (fsprop) { + if (!fsprop_validate((const char *)args.name, args.value)) { + dbprintf(_("%s: invalid value \"%s\"\n"), + args.name, args.value); + goto out; + } + + if (!adjust_fsprop_attr_name(&args, &free_name)) + goto out; } if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &args.dp)) { @@ -269,6 +375,9 @@ attr_set_f( goto out; } + if (fsprop) + print_fsprop(&args); + /* refresh with updated inode contents */ set_cur_inode(iocur_top->ino); @@ -277,7 +386,7 @@ attr_set_f( libxfs_irele(args.dp); if (args.value) free(args.value); - if (name_from_file) + if (free_name) free((void *)args.name); return 0; } @@ -293,6 +402,8 @@ attr_remove_f( .op_flags = XFS_DA_OP_OKNOENT, }; char *name_from_file = NULL; + bool free_name = false; + bool fsprop = false; int c; int error; @@ -305,9 +416,12 @@ attr_remove_f( return 0; } - while ((c = getopt(argc, argv, "ruspnN:")) != EOF) { + while ((c = getopt(argc, argv, "ruspnN:Z")) != EOF) { switch (c) { /* namespaces */ + case 'Z': + fsprop = true; + fallthrough; case 'r': args.attr_filter &= ~LIBXFS_ATTR_NS; args.attr_filter |= LIBXFS_ATTR_ROOT; @@ -354,6 +468,7 @@ attr_remove_f( if (!args.name) return 0; + free_name = true; args.namelen = namelen; } else { if (optind != argc - 1) { @@ -374,6 +489,9 @@ attr_remove_f( } } + if (fsprop && !adjust_fsprop_attr_name(&args, &free_name)) + goto out; + if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &args.dp)) { dbprintf(_("failed to iget inode %llu\n"), (unsigned long long)iocur_top->ino); @@ -398,7 +516,137 @@ attr_remove_f( out: if (args.dp) libxfs_irele(args.dp); - if (name_from_file) + if (free_name) + free((void *)args.name); + return 0; +} + +static int +attr_get_f( + int argc, + char **argv) +{ + struct xfs_da_args args = { + .geo = mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .op_flags = XFS_DA_OP_OKNOENT, + }; + char *name_from_file = NULL; + bool free_name = false; + bool fsprop = false; + int c; + int error; + + if (cur_typ == NULL) { + dbprintf(_("no current type\n")); + return 0; + } + if (cur_typ->typnm != TYP_INODE) { + dbprintf(_("current type is not inode\n")); + return 0; + } + + while ((c = getopt(argc, argv, "ruspN:Z")) != EOF) { + switch (c) { + /* namespaces */ + case 'Z': + fsprop = true; + fallthrough; + case 'r': + args.attr_filter &= ~LIBXFS_ATTR_NS; + args.attr_filter |= LIBXFS_ATTR_ROOT; + break; + case 'u': + args.attr_filter &= ~LIBXFS_ATTR_NS; + break; + case 's': + args.attr_filter &= ~LIBXFS_ATTR_NS; + args.attr_filter |= LIBXFS_ATTR_SECURE; + break; + case 'p': + args.attr_filter &= ~LIBXFS_ATTR_NS; + args.attr_filter |= XFS_ATTR_PARENT; + break; + + case 'N': + name_from_file = optarg; + break; + default: + dbprintf(_("bad option for attr_get command\n")); + return 0; + } + } + + if (name_from_file) { + int namelen; + + if (optind != argc) { + dbprintf(_("too many options for attr_get (no name needed)\n")); + return 0; + } + + args.name = get_buf_from_file(name_from_file, MAXNAMELEN, + &namelen); + if (!args.name) + return 0; + + free_name = true; + args.namelen = namelen; + } else { + if (optind != argc - 1) { + dbprintf(_("too few options for attr_get (no name given)\n")); + return 0; + } + + args.name = (const unsigned char *)argv[optind]; + if (!args.name) { + dbprintf(_("invalid name\n")); + return 0; + } + + args.namelen = strlen(argv[optind]); + if (args.namelen >= MAXNAMELEN) { + dbprintf(_("name too long\n")); + goto out; + } + } + + if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &args.dp)) { + dbprintf(_("failed to iget inode %llu\n"), + (unsigned long long)iocur_top->ino); + goto out; + } + + if (fsprop && !adjust_fsprop_attr_name(&args, &free_name)) + goto out; + + args.owner = iocur_top->ino; + libxfs_attr_sethash(&args); + + /* + * Look up attr value with a maximally long length and a null buffer + * to return the value and the correct length. + */ + args.valuelen = XATTR_SIZE_MAX; + error = -libxfs_attr_get(&args); + if (error) { + dbprintf(_("failed to get attr %s on inode %llu: %s\n"), + args.name, (unsigned long long)iocur_top->ino, + strerror(error)); + goto out; + } + + if (fsprop) + print_fsprop(&args); + else + printf("%.*s\n", args.valuelen, (char *)args.value); + +out: + if (args.dp) + libxfs_irele(args.dp); + if (args.value) + free(args.value); + if (free_name) free((void *)args.name); return 0; } diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8 index 9f6fea574..f0865b2df 100644 --- a/man/man8/xfs_db.8 +++ b/man/man8/xfs_db.8 @@ -184,7 +184,35 @@ Displays the length, free block count, per-AG reservation size, and per-AG reservation usage for a given AG. If no argument is given, display information for all AGs. .TP -.BI "attr_remove [\-p|\-r|\-u|\-s] [\-n] [\-N " namefile "|" name "] " +.BI "attr_get [\-p|\-r|\-u|\-s|\-Z] [\-N " namefile "|" name "] " +Print the value of the specified extended attribute from the current file. +.RS 1.0i +.TP 0.4i +.B \-p +Sets the attribute in the parent namespace. +Only one namespace option can be specified. +.TP +.B \-r +Sets the attribute in the root namespace. +Only one namespace option can be specified. +.TP +.B \-u +Sets the attribute in the user namespace. +Only one namespace option can be specified. +.TP +.B \-s +Sets the attribute in the secure namespace. +Only one namespace option can be specified. +.TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP +.B \-N +Read the name from this file. +.RE +.TP +.BI "attr_remove [\-p|\-r|\-u|\-s|\-Z] [\-n] [\-N " namefile "|" name "] " Remove the specified extended attribute from the current file. .RS 1.0i .TP 0.4i @@ -204,6 +232,10 @@ Only one namespace option can be specified. Sets the attribute in the secure namespace. Only one namespace option can be specified. .TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP .B \-N Read the name from this file. .TP @@ -211,7 +243,7 @@ Read the name from this file. Do not enable 'noattr2' mode on V4 filesystems. .RE .TP -.BI "attr_set [\-p\-r|\-u|\-s] [\-n] [\-R|\-C] [\-v " valuelen "|\-V " valuefile "] [\-N " namefile "|" name "] " +.BI "attr_set [\-p\-r|\-u|\-s|\-Z] [\-n] [\-R|\-C] [\-v " valuelen "|\-V " valuefile "] [\-N " namefile "|" name "] [" value "]" Sets an extended attribute on the current file with the given name. .RS 1.0i .TP 0.4i @@ -231,6 +263,10 @@ Only one namespace option can be specified. Sets the attribute in the secure namespace. Only one namespace option can be specified. .TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP .B \-N Read the name from this file. .TP ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] xfs_db: improve getting and setting extended attributes 2024-08-06 18:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong @ 2024-08-07 16:10 ` Christoph Hellwig 2024-08-07 16:43 ` Darrick J. Wong 0 siblings, 1 reply; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:10 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, Christoph Hellwig, Dave Chinner, fstests, linux-xfs On Tue, Aug 06, 2024 at 11:20:06AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Add an attr_get command to retrieve the value of an xattr from a file; > and extend the attr_set command to allow passing of string values. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>Reviewed-by: Christoph Hellwig <hch@lst.de> Missing newline here again (not going to say this again if it shows up on more patches..) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] xfs_db: improve getting and setting extended attributes 2024-08-07 16:10 ` Christoph Hellwig @ 2024-08-07 16:43 ` Darrick J. Wong 0 siblings, 0 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-07 16:43 UTC (permalink / raw) To: Christoph Hellwig; +Cc: cem, Dave Chinner, fstests, linux-xfs On Wed, Aug 07, 2024 at 06:10:04PM +0200, Christoph Hellwig wrote: > On Tue, Aug 06, 2024 at 11:20:06AM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Add an attr_get command to retrieve the value of an xattr from a file; > > and extend the attr_set command to allow passing of string values. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>Reviewed-by: Christoph Hellwig <hch@lst.de> > > Missing newline here again (not going to say this again if it shows > up on more patches..) Yeah, I don't know why that happened. Something went wrong in my text processing scripts. --D ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 4/7] libxfs: hoist listxattr from xfs_repair 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong ` (2 preceding siblings ...) 2024-08-06 18:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong @ 2024-08-06 18:20 ` Darrick J. Wong 2024-08-06 18:20 ` [PATCH 5/7] libxfs: pass a transaction context through listxattr Darrick J. Wong ` (2 subsequent siblings) 6 siblings, 0 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:20 UTC (permalink / raw) To: djwong, cem Cc: Christoph Hellwig, Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Hoist the listxattr code from xfs_repair so that we can use it in xfs_db. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Dave Chinner <dchinner@redhat.com> --- libxfs/Makefile | 2 ++ libxfs/listxattr.c | 2 +- libxfs/listxattr.h | 6 +++--- repair/Makefile | 2 -- repair/pptr.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename repair/listxattr.c => libxfs/listxattr.c (99%) rename repair/listxattr.h => libxfs/listxattr.h (81%) diff --git a/libxfs/Makefile b/libxfs/Makefile index 4e8f9a135..2f2791cae 100644 --- a/libxfs/Makefile +++ b/libxfs/Makefile @@ -23,6 +23,7 @@ HFILES = \ defer_item.h \ libxfs_io.h \ libxfs_api_defs.h \ + listxattr.h \ init.h \ libxfs_priv.h \ linux-err.h \ @@ -69,6 +70,7 @@ CFILES = buf_mem.c \ defer_item.c \ init.c \ kmem.c \ + listxattr.c \ logitem.c \ rdwr.c \ topology.c \ diff --git a/repair/listxattr.c b/libxfs/listxattr.c similarity index 99% rename from repair/listxattr.c rename to libxfs/listxattr.c index 2af77b7b2..bedaca678 100644 --- a/repair/listxattr.c +++ b/libxfs/listxattr.c @@ -6,7 +6,7 @@ #include "libxfs.h" #include "libxlog.h" #include "libfrog/bitmap.h" -#include "repair/listxattr.h" +#include "listxattr.h" /* Call a function for every entry in a shortform xattr structure. */ STATIC int diff --git a/repair/listxattr.h b/libxfs/listxattr.h similarity index 81% rename from repair/listxattr.h rename to libxfs/listxattr.h index 2d26fce0f..cddd96af7 100644 --- a/repair/listxattr.h +++ b/libxfs/listxattr.h @@ -3,8 +3,8 @@ * Copyright (c) 2022-2024 Oracle. All Rights Reserved. * Author: Darrick J. Wong <djwong@kernel.org> */ -#ifndef __REPAIR_LISTXATTR_H__ -#define __REPAIR_LISTXATTR_H__ +#ifndef __LIBXFS_LISTXATTR_H__ +#define __LIBXFS_LISTXATTR_H__ typedef int (*xattr_walk_fn)(struct xfs_inode *ip, unsigned int attr_flags, const unsigned char *name, unsigned int namelen, @@ -12,4 +12,4 @@ typedef int (*xattr_walk_fn)(struct xfs_inode *ip, unsigned int attr_flags, int xattr_walk(struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv); -#endif /* __REPAIR_LISTXATTR_H__ */ +#endif /* __LIBXFS_LISTXATTR_H__ */ diff --git a/repair/Makefile b/repair/Makefile index e7445d53e..a36a95e35 100644 --- a/repair/Makefile +++ b/repair/Makefile @@ -24,7 +24,6 @@ HFILES = \ err_protos.h \ globals.h \ incore.h \ - listxattr.h \ pptr.h \ prefetch.h \ progress.h \ @@ -59,7 +58,6 @@ CFILES = \ incore_ext.c \ incore_ino.c \ init.c \ - listxattr.c \ phase1.c \ phase2.c \ phase3.c \ diff --git a/repair/pptr.c b/repair/pptr.c index 8ec6a51d2..cc66e6372 100644 --- a/repair/pptr.c +++ b/repair/pptr.c @@ -11,7 +11,7 @@ #include "repair/globals.h" #include "repair/err_protos.h" #include "repair/slab.h" -#include "repair/listxattr.h" +#include "libxfs/listxattr.h" #include "repair/threads.h" #include "repair/incore.h" #include "repair/pptr.h" ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 5/7] libxfs: pass a transaction context through listxattr 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong ` (3 preceding siblings ...) 2024-08-06 18:20 ` [PATCH 4/7] libxfs: hoist listxattr from xfs_repair Darrick J. Wong @ 2024-08-06 18:20 ` Darrick J. Wong 2024-08-06 18:20 ` [PATCH 6/7] xfs_db: add a command to list xattrs Darrick J. Wong 2024-08-06 18:21 ` [PATCH 7/7] xfs_property: add a new tool to administer fs properties Darrick J. Wong 6 siblings, 0 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:20 UTC (permalink / raw) To: djwong, cem Cc: Christoph Hellwig, Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Pass a transaction context so that a new caller can walk the attr names and query the values all in one go without deadlocking on nested buffer access. While we're at it, make the existing xfs_repair callers try to use empty transactions so that we don't deadlock on cycles in the xattr structure. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Dave Chinner <dchinner@redhat.com> --- libxfs/listxattr.c | 40 +++++++++++++++++++++++----------------- libxfs/listxattr.h | 6 ++++-- repair/pptr.c | 7 ++++++- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/libxfs/listxattr.c b/libxfs/listxattr.c index bedaca678..34205682f 100644 --- a/libxfs/listxattr.c +++ b/libxfs/listxattr.c @@ -11,6 +11,7 @@ /* Call a function for every entry in a shortform xattr structure. */ STATIC int xattr_walk_sf( + struct xfs_trans *tp, struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv) @@ -22,7 +23,7 @@ xattr_walk_sf( sfe = libxfs_attr_sf_firstentry(hdr); for (i = 0; i < hdr->count; i++) { - error = attr_fn(ip, sfe->flags, sfe->nameval, sfe->namelen, + error = attr_fn(tp, ip, sfe->flags, sfe->nameval, sfe->namelen, &sfe->nameval[sfe->namelen], sfe->valuelen, priv); if (error) @@ -37,6 +38,7 @@ xattr_walk_sf( /* Call a function for every entry in this xattr leaf block. */ STATIC int xattr_walk_leaf_entries( + struct xfs_trans *tp, struct xfs_inode *ip, xattr_walk_fn attr_fn, struct xfs_buf *bp, @@ -75,7 +77,7 @@ xattr_walk_leaf_entries( valuelen = be32_to_cpu(name_rmt->valuelen); } - error = attr_fn(ip, entry->flags, name, namelen, value, + error = attr_fn(tp, ip, entry->flags, name, namelen, value, valuelen, priv); if (error) return error; @@ -91,6 +93,7 @@ xattr_walk_leaf_entries( */ STATIC int xattr_walk_leaf( + struct xfs_trans *tp, struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv) @@ -98,18 +101,19 @@ xattr_walk_leaf( struct xfs_buf *leaf_bp; int error; - error = -libxfs_attr3_leaf_read(NULL, ip, ip->i_ino, 0, &leaf_bp); + error = -libxfs_attr3_leaf_read(tp, ip, ip->i_ino, 0, &leaf_bp); if (error) return error; - error = xattr_walk_leaf_entries(ip, attr_fn, leaf_bp, priv); - libxfs_trans_brelse(NULL, leaf_bp); + error = xattr_walk_leaf_entries(tp, ip, attr_fn, leaf_bp, priv); + libxfs_trans_brelse(tp, leaf_bp); return error; } /* Find the leftmost leaf in the xattr dabtree. */ STATIC int xattr_walk_find_leftmost_leaf( + struct xfs_trans *tp, struct xfs_inode *ip, struct bitmap *seen_blocks, struct xfs_buf **leaf_bpp) @@ -127,7 +131,7 @@ xattr_walk_find_leftmost_leaf( for (;;) { uint16_t magic; - error = -libxfs_da3_node_read(NULL, ip, blkno, &bp, + error = -libxfs_da3_node_read(tp, ip, blkno, &bp, XFS_ATTR_FORK); if (error) return error; @@ -164,7 +168,7 @@ xattr_walk_find_leftmost_leaf( /* Find the next level towards the leaves of the dabtree. */ btree = nodehdr.btree; blkno = be32_to_cpu(btree->before); - libxfs_trans_brelse(NULL, bp); + libxfs_trans_brelse(tp, bp); /* Make sure we haven't seen this new block already. */ if (bitmap_test(seen_blocks, blkno, 1)) @@ -184,13 +188,14 @@ xattr_walk_find_leftmost_leaf( return 0; out_buf: - libxfs_trans_brelse(NULL, bp); + libxfs_trans_brelse(tp, bp); return error; } /* Call a function for every entry in a node-format xattr structure. */ STATIC int xattr_walk_node( + struct xfs_trans *tp, struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv) @@ -204,12 +209,12 @@ xattr_walk_node( bitmap_alloc(&seen_blocks); - error = xattr_walk_find_leftmost_leaf(ip, seen_blocks, &leaf_bp); + error = xattr_walk_find_leftmost_leaf(tp, ip, seen_blocks, &leaf_bp); if (error) goto out_bitmap; for (;;) { - error = xattr_walk_leaf_entries(ip, attr_fn, leaf_bp, + error = xattr_walk_leaf_entries(tp, ip, attr_fn, leaf_bp, priv); if (error) goto out_leaf; @@ -220,13 +225,13 @@ xattr_walk_node( if (leafhdr.forw == 0) goto out_leaf; - libxfs_trans_brelse(NULL, leaf_bp); + libxfs_trans_brelse(tp, leaf_bp); /* Make sure we haven't seen this new leaf already. */ if (bitmap_test(seen_blocks, leafhdr.forw, 1)) goto out_bitmap; - error = -libxfs_attr3_leaf_read(NULL, ip, ip->i_ino, + error = -libxfs_attr3_leaf_read(tp, ip, ip->i_ino, leafhdr.forw, &leaf_bp); if (error) goto out_bitmap; @@ -238,7 +243,7 @@ xattr_walk_node( } out_leaf: - libxfs_trans_brelse(NULL, leaf_bp); + libxfs_trans_brelse(tp, leaf_bp); out_bitmap: bitmap_free(&seen_blocks); return error; @@ -247,6 +252,7 @@ xattr_walk_node( /* Call a function for every extended attribute in a file. */ int xattr_walk( + struct xfs_trans *tp, struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv) @@ -257,15 +263,15 @@ xattr_walk( return 0; if (ip->i_af.if_format == XFS_DINODE_FMT_LOCAL) - return xattr_walk_sf(ip, attr_fn, priv); + return xattr_walk_sf(tp, ip, attr_fn, priv); /* attr functions require that the attr fork is loaded */ - error = -libxfs_iread_extents(NULL, ip, XFS_ATTR_FORK); + error = -libxfs_iread_extents(tp, ip, XFS_ATTR_FORK); if (error) return error; if (libxfs_attr_is_leaf(ip)) - return xattr_walk_leaf(ip, attr_fn, priv); + return xattr_walk_leaf(tp, ip, attr_fn, priv); - return xattr_walk_node(ip, attr_fn, priv); + return xattr_walk_node(tp, ip, attr_fn, priv); } diff --git a/libxfs/listxattr.h b/libxfs/listxattr.h index cddd96af7..933e0f529 100644 --- a/libxfs/listxattr.h +++ b/libxfs/listxattr.h @@ -6,10 +6,12 @@ #ifndef __LIBXFS_LISTXATTR_H__ #define __LIBXFS_LISTXATTR_H__ -typedef int (*xattr_walk_fn)(struct xfs_inode *ip, unsigned int attr_flags, +typedef int (*xattr_walk_fn)(struct xfs_trans *tp, struct xfs_inode *ip, + unsigned int attr_flags, const unsigned char *name, unsigned int namelen, const void *value, unsigned int valuelen, void *priv); -int xattr_walk(struct xfs_inode *ip, xattr_walk_fn attr_fn, void *priv); +int xattr_walk(struct xfs_trans *tp, struct xfs_inode *ip, + xattr_walk_fn attr_fn, void *priv); #endif /* __LIBXFS_LISTXATTR_H__ */ diff --git a/repair/pptr.c b/repair/pptr.c index cc66e6372..ee29e47a8 100644 --- a/repair/pptr.c +++ b/repair/pptr.c @@ -593,6 +593,7 @@ store_file_pptr_name( /* Decide if this is a directory parent pointer and stash it if so. */ static int examine_xattr( + struct xfs_trans *tp, struct xfs_inode *ip, unsigned int attr_flags, const unsigned char *name, @@ -1205,6 +1206,7 @@ check_file_parent_ptrs( struct xfs_inode *ip, struct file_scan *fscan) { + struct xfs_trans *tp = NULL; int error; error = -init_slab(&fscan->file_pptr_recs, sizeof(struct file_pptr)); @@ -1215,7 +1217,10 @@ check_file_parent_ptrs( fscan->have_garbage = false; fscan->nr_file_pptrs = 0; - error = xattr_walk(ip, examine_xattr, fscan); + libxfs_trans_alloc_empty(ip->i_mount, &tp); + error = xattr_walk(tp, ip, examine_xattr, fscan); + if (tp) + libxfs_trans_cancel(tp); if (error && !no_modify) do_error(_("ino %llu parent pointer scan failed: %s\n"), (unsigned long long)ip->i_ino, ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 6/7] xfs_db: add a command to list xattrs 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong ` (4 preceding siblings ...) 2024-08-06 18:20 ` [PATCH 5/7] libxfs: pass a transaction context through listxattr Darrick J. Wong @ 2024-08-06 18:20 ` Darrick J. Wong 2024-08-06 18:21 ` [PATCH 7/7] xfs_property: add a new tool to administer fs properties Darrick J. Wong 6 siblings, 0 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:20 UTC (permalink / raw) To: djwong, cem Cc: Christoph Hellwig, Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add a command to list extended attributes from xfs_db. We'll need this later to manage the fs properties when unmounted. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Dave Chinner <dchinner@redhat.com> --- db/attrset.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++ man/man8/xfs_db.8 | 28 +++++++ 2 files changed, 229 insertions(+) diff --git a/db/attrset.c b/db/attrset.c index 9e53e63c9..e3ffb75aa 100644 --- a/db/attrset.c +++ b/db/attrset.c @@ -18,13 +18,21 @@ #include "malloc.h" #include <sys/xattr.h> #include "libfrog/fsproperties.h" +#include "libxfs/listxattr.h" +static int attr_list_f(int argc, char **argv); static int attr_get_f(int argc, char **argv); static int attr_set_f(int argc, char **argv); static int attr_remove_f(int argc, char **argv); + +static void attrlist_help(void); static void attrget_help(void); static void attrset_help(void); +static const cmdinfo_t attr_list_cmd = + { "attr_list", "alist", attr_list_f, 0, -1, 0, + N_("[-r|-s|-u|-p|-Z] [-v]"), + N_("list attributes on the current inode"), attrlist_help }; static const cmdinfo_t attr_get_cmd = { "attr_get", "aget", attr_get_f, 1, -1, 0, N_("[-r|-s|-u|-p|-Z] name"), @@ -38,6 +46,24 @@ static const cmdinfo_t attr_remove_cmd = N_("[-r|-s|-u|-p|-Z] [-n] name"), N_("remove the named attribute from the current inode"), attrset_help }; +static void +attrlist_help(void) +{ + dbprintf(_( +"\n" +" The attr_list command provide interfaces for listing all extended attributes\n" +" attached to an inode.\n" +" There are 4 namespace flags:\n" +" -r -- 'root'\n" +" -u -- 'user' (default)\n" +" -s -- 'secure'\n" +" -p -- 'parent'\n" +" -Z -- fs property\n" +"\n" +" -v -- print the value of the attributes\n" +"\n")); +} + static void attrget_help(void) { @@ -87,6 +113,7 @@ attrset_init(void) if (!expert_mode) return; + add_command(&attr_list_cmd); add_command(&attr_get_cmd); add_command(&attr_set_cmd); add_command(&attr_remove_cmd); @@ -650,3 +677,177 @@ attr_get_f( free((void *)args.name); return 0; } + +struct attrlist_ctx { + unsigned int attr_filter; + bool print_values; + bool fsprop; +}; + +static int +attrlist_print( + struct xfs_trans *tp, + struct xfs_inode *ip, + unsigned int attr_flags, + const unsigned char *name, + unsigned int namelen, + const void *value, + unsigned int valuelen, + void *priv) +{ + struct attrlist_ctx *ctx = priv; + struct xfs_da_args args = { + .geo = mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .op_flags = XFS_DA_OP_OKNOENT, + .dp = ip, + .owner = ip->i_ino, + .trans = tp, + .attr_filter = attr_flags & XFS_ATTR_NSP_ONDISK_MASK, + .name = name, + .namelen = namelen, + }; + char namebuf[MAXNAMELEN + 1]; + const char *print_name = namebuf; + int error; + + if ((attr_flags & XFS_ATTR_NSP_ONDISK_MASK) != ctx->attr_filter) + return 0; + + /* Make sure the name is null terminated. */ + memcpy(namebuf, name, namelen); + namebuf[MAXNAMELEN] = 0; + + if (ctx->fsprop) { + const char *p = attr_name_to_fsprop_name(namebuf); + + if (!p) + return 0; + + namelen -= (p - namebuf); + print_name = p; + } + + if (!ctx->print_values) { + printf("%.*s\n", namelen, print_name); + return 0; + } + + if (value) { + printf("%.*s=%.*s\n", namelen, print_name, valuelen, + (char *)value); + return 0; + } + + libxfs_attr_sethash(&args); + + /* + * Look up attr value with a maximally long length and a null buffer + * to return the value and the correct length. + */ + args.valuelen = XATTR_SIZE_MAX; + error = -libxfs_attr_get(&args); + if (error) { + dbprintf(_("failed to get attr %s on inode %llu: %s\n"), + args.name, (unsigned long long)iocur_top->ino, + strerror(error)); + return error; + } + + printf("%.*s=%.*s\n", namelen, print_name, args.valuelen, + (char *)args.value); + kfree(args.value); + + return 0; +} + +static int +attr_list_f( + int argc, + char **argv) +{ + struct attrlist_ctx ctx = { }; + struct xfs_trans *tp; + struct xfs_inode *ip; + int c; + int error; + + if (cur_typ == NULL) { + dbprintf(_("no current type\n")); + return 0; + } + if (cur_typ->typnm != TYP_INODE) { + dbprintf(_("current type is not inode\n")); + return 0; + } + + while ((c = getopt(argc, argv, "ruspvZ")) != EOF) { + switch (c) { + /* namespaces */ + case 'Z': + ctx.fsprop = true; + fallthrough; + case 'r': + ctx.attr_filter &= ~LIBXFS_ATTR_NS; + ctx.attr_filter |= LIBXFS_ATTR_ROOT; + break; + case 'u': + ctx.attr_filter &= ~LIBXFS_ATTR_NS; + break; + case 's': + ctx.attr_filter &= ~LIBXFS_ATTR_NS; + ctx.attr_filter |= LIBXFS_ATTR_SECURE; + break; + case 'p': + ctx.attr_filter &= ~LIBXFS_ATTR_NS; + ctx.attr_filter |= XFS_ATTR_PARENT; + break; + + case 'v': + ctx.print_values = true; + break; + default: + dbprintf(_("bad option for attr_list command\n")); + return 0; + } + } + + if (ctx.fsprop && + (ctx.attr_filter & LIBXFS_ATTR_NS) != LIBXFS_ATTR_ROOT) { + dbprintf(_("fs properties must be ATTR_ROOT\n")); + return false; + } + + if (optind != argc) { + dbprintf(_("too many options for attr_list (no name needed)\n")); + return 0; + } + + error = -libxfs_trans_alloc_empty(mp, &tp); + if (error) { + dbprintf(_("failed to allocate empty transaction\n")); + return 0; + } + + error = -libxfs_iget(mp, NULL, iocur_top->ino, 0, &ip); + if (error) { + dbprintf(_("failed to iget inode %llu: %s\n"), + (unsigned long long)iocur_top->ino, + strerror(error)); + goto out_trans; + } + + error = xattr_walk(tp, ip, attrlist_print, &ctx); + if (error) { + dbprintf(_("walking inode %llu xattrs: %s\n"), + (unsigned long long)iocur_top->ino, + strerror(error)); + goto out_inode; + } + +out_inode: + libxfs_irele(ip); +out_trans: + libxfs_trans_cancel(tp); + return 0; +} diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8 index f0865b2df..291ec1c58 100644 --- a/man/man8/xfs_db.8 +++ b/man/man8/xfs_db.8 @@ -212,6 +212,34 @@ Only one namespace option can be specified. Read the name from this file. .RE .TP +.BI "attr_list [\-p|\-r|\-u|\-s|\-Z] [\-v] " +Lists the extended attributes of the current file. +.RS 1.0i +.TP 0.4i +.B \-p +Sets the attribute in the parent namespace. +Only one namespace option can be specified. +.TP +.B \-r +Sets the attribute in the root namespace. +Only one namespace option can be specified. +.TP +.B \-u +Sets the attribute in the user namespace. +Only one namespace option can be specified. +.TP +.B \-s +Sets the attribute in the secure namespace. +Only one namespace option can be specified. +.TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP +.B \-v +Print the extended attribute values too. +.RE +.TP .BI "attr_remove [\-p|\-r|\-u|\-s|\-Z] [\-n] [\-N " namefile "|" name "] " Remove the specified extended attribute from the current file. .RS 1.0i ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 7/7] xfs_property: add a new tool to administer fs properties 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong ` (5 preceding siblings ...) 2024-08-06 18:20 ` [PATCH 6/7] xfs_db: add a command to list xattrs Darrick J. Wong @ 2024-08-06 18:21 ` Darrick J. Wong 2024-08-07 16:10 ` Christoph Hellwig 6 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:21 UTC (permalink / raw) To: djwong, cem; +Cc: Dave Chinner, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Create a tool to list, get, set, and remove filesystem properties. Signed-off-by: Darrick J. Wong <djwong@kernel.org>Acked-by: Dave Chinner <dchinner@redhat.com> --- io/Makefile | 3 +- io/xfs_property | 77 +++++++++++++++++++++++++++++++++++++++++++++++ man/man8/xfs_property.8 | 61 +++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100755 io/xfs_property create mode 100644 man/man8/xfs_property.8 diff --git a/io/Makefile b/io/Makefile index 0bdd05b57..c33d57f5e 100644 --- a/io/Makefile +++ b/io/Makefile @@ -6,7 +6,7 @@ TOPDIR = .. include $(TOPDIR)/include/builddefs LTCOMMAND = xfs_io -LSRCFILES = xfs_bmap.sh xfs_freeze.sh xfs_mkfile.sh +LSRCFILES = xfs_bmap.sh xfs_freeze.sh xfs_mkfile.sh xfs_property HFILES = init.h io.h CFILES = \ attr.c \ @@ -92,6 +92,7 @@ install: default $(LTINSTALL) -m 755 xfs_bmap.sh $(PKG_SBIN_DIR)/xfs_bmap $(LTINSTALL) -m 755 xfs_freeze.sh $(PKG_SBIN_DIR)/xfs_freeze $(LTINSTALL) -m 755 xfs_mkfile.sh $(PKG_SBIN_DIR)/xfs_mkfile + $(LTINSTALL) -m 755 xfs_property $(PKG_SBIN_DIR)/xfs_property install-dev: -include .dep diff --git a/io/xfs_property b/io/xfs_property new file mode 100755 index 000000000..6f630312a --- /dev/null +++ b/io/xfs_property @@ -0,0 +1,77 @@ +#!/bin/bash -f +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2024 Oracle. All Rights Reserved. +# Author: Darrick J. Wong <djwong@kernel.org> +# + +OPTS="" +USAGE="Usage: xfs_property [-V] [mountpoint|device|file] [list [-v]|get name...|set name=value...|remove name...]" + +# Try to find a loop device associated with a file. We only want to return +# one loopdev (multiple loop devices can attach to a single file) so we grab +# the last line and return it if it's actually a block device. +try_find_loop_dev_for_file() { + local x="$(losetup -O NAME -j "$1" 2> /dev/null | tail -n 1)" + test -b "${x}" && echo "${x}" +} + +while getopts "V" c +do + case $c in + V) xfs_io -p xfs_info -V + status=$? + exit ${status} + ;; + *) echo "${USAGE}" 1>&2 + exit 2 + ;; + esac +done +set -- extra "$@" +shift $OPTIND + +if [ $# -lt 2 ]; then + echo "${USAGE}" 1>&2 + exit 2 +fi + +target="$1" +shift +subcommand="$1" +shift + +db_args=() +io_args=() + +case "$subcommand" in +"list") + vparam= + if [ $# -eq 1 ] && [ "$1" = "-v" ]; then + vparam=" -v" + fi + db_args+=('-c' "attr_list -Z${vparam}") + io_args+=('-c' "listfsprops${vparam}") + ;; +"get"|"remove"|"set") + for arg in "$@"; do + db_args+=('-c' "attr_${subcommand} -Z ${arg/=/ }") + io_args+=('-c' "${subcommand}fsprops ${arg}") + done + ;; +*) + echo "${USAGE}" 1>&2 + exit 2 +esac + +# See if we can map the arg to a loop device +loopdev="$(try_find_loop_dev_for_file "${target}")" +test -n "${loopdev}" && target="${loopdev}" + +# If we find a mountpoint for the device, do a live query; otherwise try +# reading the fs with xfs_db. +if mountpt="$(findmnt -t xfs -f -n -o TARGET "${target}" 2> /dev/null)"; then + exec xfs_io -p xfs_property "${io_args[@]}" "${mountpt}" +else + exec xfs_db -p xfs_property -x -c 'path /' "${db_args[@]}" "${target}" +fi diff --git a/man/man8/xfs_property.8 b/man/man8/xfs_property.8 new file mode 100644 index 000000000..19c1c0e37 --- /dev/null +++ b/man/man8/xfs_property.8 @@ -0,0 +1,61 @@ +.TH xfs_property 8 +.SH NAME +xfs_property \- examine and edit properties about an XFS filesystem +.SH SYNOPSIS +.B xfs_property +.I target +.B get +.IR name ... +.br +.B xfs_property +.I target +.B list [ \-v ] +.br +.B xfs_property +.I target +.B set +.IR name=value ... +.br +.B xfs_property +.I target +.B remove +.IR name ... +.br +.B xfs_property \-V +.SH DESCRIPTION +.B xfs_property +retrieves, lists, sets, or removes properties of an XFS filesystem. +Filesystem properties are root-controlled attributes set on the root directory +of the filesystem to enable the system administrator to coordinate with +userspace programs. + +.I target +is one of: the root directory of a mounted filesystem; a block device containing +an XFS filesystem; or a regular file containing an XFS filesystem. + +.SH OPTIONS +.TP +.B \-V +Print the version number and exits. + +.SH COMMANDS +.TP +.B get +.IR name ... +Prints the values of the given filesystem properties. +.TP +.B list +Lists the names of all filesystem properties. +If the +.B -v +flag is specified, prints the values as well. +.TP +.B set +.IR name = value ... +Sets the given filesystem properties to the specified values and prints what +was set. +.TP +.B +remove +.IR name ... +Unsets the given filesystem properties. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 7/7] xfs_property: add a new tool to administer fs properties 2024-08-06 18:21 ` [PATCH 7/7] xfs_property: add a new tool to administer fs properties Darrick J. Wong @ 2024-08-07 16:10 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:10 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, Dave Chinner, hch, fstests, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck 2024-08-06 18:14 [PATCHBOMB v30.10] xfsprogs-6.10: filesystem properties Darrick J. Wong 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong @ 2024-08-06 18:18 ` Darrick J. Wong 2024-08-06 18:21 ` [PATCH 1/4] libfrog: define a autofsck filesystem property Darrick J. Wong ` (3 more replies) 2024-08-06 18:19 ` [PATCHSET v30.10 3/3] debian: enable xfs_scrub_all by default Darrick J. Wong 2024-08-06 18:19 ` [PATCHSET v30.10] fstests: xfs filesystem properties Darrick J. Wong 3 siblings, 4 replies; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:18 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs Hi all, Now that we have the ability to set per-filesystem properties, teach the background xfs_scrub service to pick up advice from the filesystem that it wants to examine, and pick a mode from that. We're only going to enable this by default for newer filesystems. If you're going to start using this code, I strongly recommend pulling from my git trees, which are linked below. This has been running on the djcloud for months with no problems. Enjoy! Comments and questions are, as always, welcome. xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=autofsck-6.10 --- Commits in this patchset: * libfrog: define a autofsck filesystem property * xfs_scrub: allow sysadmin to control background scrubs * xfs_scrub: use the autofsck fsproperty to select mode * mkfs: set autofsck filesystem property --- libfrog/fsproperties.c | 38 ++++++++++++ libfrog/fsproperties.h | 13 ++++ man/man8/mkfs.xfs.8.in | 6 ++ man/man8/xfs_property.8 | 8 ++ man/man8/xfs_scrub.8 | 46 ++++++++++++++ mkfs/lts_4.19.conf | 1 mkfs/lts_5.10.conf | 1 mkfs/lts_5.15.conf | 1 mkfs/lts_5.4.conf | 1 mkfs/lts_6.1.conf | 1 mkfs/lts_6.6.conf | 1 mkfs/xfs_mkfs.c | 122 +++++++++++++++++++++++++++++++++++++ scrub/Makefile | 3 - scrub/phase1.c | 91 ++++++++++++++++++++++++++++ scrub/xfs_scrub.c | 14 ++++ scrub/xfs_scrub.h | 7 ++ scrub/xfs_scrub@.service.in | 2 - scrub/xfs_scrub_media@.service.in | 2 - 18 files changed, 353 insertions(+), 5 deletions(-) ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/4] libfrog: define a autofsck filesystem property 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong @ 2024-08-06 18:21 ` Darrick J. Wong 2024-08-07 16:11 ` Christoph Hellwig 2024-08-06 18:21 ` [PATCH 2/4] xfs_scrub: allow sysadmin to control background scrubs Darrick J. Wong ` (2 subsequent siblings) 3 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:21 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Now that we have the ability to set properties on filesystems, create an "autofsck" property so that sysadmins can control background xfs_scrub behaviors. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- libfrog/fsproperties.c | 38 ++++++++++++++++++++++++++++++++++++++ libfrog/fsproperties.h | 13 +++++++++++++ 2 files changed, 51 insertions(+) diff --git a/libfrog/fsproperties.c b/libfrog/fsproperties.c index c317d15c1..72485f627 100644 --- a/libfrog/fsproperties.c +++ b/libfrog/fsproperties.c @@ -29,11 +29,49 @@ __fsprops_lookup( #define fsprops_lookup(values, value) \ __fsprops_lookup((values), ARRAY_SIZE(values), (value)) +/* Automatic background fsck fs property */ + +static const char *fsprop_autofsck_values[] = { + [FSPROP_AUTOFSCK_UNSET] = NULL, + [FSPROP_AUTOFSCK_NONE] = "none", + [FSPROP_AUTOFSCK_CHECK] = "check", + [FSPROP_AUTOFSCK_OPTIMIZE] = "optimize", + [FSPROP_AUTOFSCK_REPAIR] = "repair", +}; + +/* Convert the autofsck property enum to a string. */ +const char * +fsprop_autofsck_write( + enum fsprop_autofsck x) +{ + if (x <= FSPROP_AUTOFSCK_UNSET || + x >= ARRAY_SIZE(fsprop_autofsck_values)) + return NULL; + return fsprop_autofsck_values[x]; +} + +/* + * Turn a autofsck value string into an enumerated value, or _UNSET if it's + * not recognized. + */ +enum fsprop_autofsck +fsprop_autofsck_read( + const char *value) +{ + int ret = fsprops_lookup(fsprop_autofsck_values, value); + if (ret < 0) + return FSPROP_AUTOFSCK_UNSET; + return ret; +} + /* Return true if a fs property name=value tuple is allowed. */ bool fsprop_validate( const char *name, const char *value) { + if (!strcmp(name, FSPROP_AUTOFSCK_NAME)) + return fsprops_lookup(fsprop_autofsck_values, value) >= 0; + return true; } diff --git a/libfrog/fsproperties.h b/libfrog/fsproperties.h index b1ac4cdd7..11d6530bc 100644 --- a/libfrog/fsproperties.h +++ b/libfrog/fsproperties.h @@ -50,4 +50,17 @@ bool fsprop_validate(const char *name, const char *value); /* Specific Filesystem Properties */ +#define FSPROP_AUTOFSCK_NAME "autofsck" + +enum fsprop_autofsck { + FSPROP_AUTOFSCK_UNSET = 0, /* do not set property */ + FSPROP_AUTOFSCK_NONE, /* no background scrubs */ + FSPROP_AUTOFSCK_CHECK, /* allow only background checking */ + FSPROP_AUTOFSCK_OPTIMIZE, /* allow background optimization */ + FSPROP_AUTOFSCK_REPAIR, /* allow background repair & optimization */ +}; + +const char *fsprop_autofsck_write(enum fsprop_autofsck x); +enum fsprop_autofsck fsprop_autofsck_read(const char *value); + #endif /* __LIBFROG_FSPROPERTIES_H__ */ ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/4] libfrog: define a autofsck filesystem property 2024-08-06 18:21 ` [PATCH 1/4] libfrog: define a autofsck filesystem property Darrick J. Wong @ 2024-08-07 16:11 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:11 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, hch, dchinner, fstests, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/4] xfs_scrub: allow sysadmin to control background scrubs 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong 2024-08-06 18:21 ` [PATCH 1/4] libfrog: define a autofsck filesystem property Darrick J. Wong @ 2024-08-06 18:21 ` Darrick J. Wong 2024-08-07 16:12 ` Christoph Hellwig 2024-08-06 18:21 ` [PATCH 3/4] xfs_scrub: use the autofsck fsproperty to select mode Darrick J. Wong 2024-08-06 18:22 ` [PATCH 4/4] mkfs: set autofsck filesystem property Darrick J. Wong 3 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:21 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add an extended option -o autofsck to xfs_scrub so that it selects the operation mode from the "autofsck" filesystem property. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- man/man8/xfs_property.8 | 8 ++++ man/man8/xfs_scrub.8 | 46 ++++++++++++++++++++++++ scrub/phase1.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++ scrub/xfs_scrub.c | 14 +++++++ scrub/xfs_scrub.h | 7 ++++ 5 files changed, 166 insertions(+) diff --git a/man/man8/xfs_property.8 b/man/man8/xfs_property.8 index 19c1c0e37..9fb1fd7cc 100644 --- a/man/man8/xfs_property.8 +++ b/man/man8/xfs_property.8 @@ -59,3 +59,11 @@ was set. remove .IR name ... Unsets the given filesystem properties. + +.SH FILESYSTEM PROPERTIES +Known filesystem properties for XFS are: + +.I autofsck +See +.BR xfs_scrub (8) +for more information. diff --git a/man/man8/xfs_scrub.8 b/man/man8/xfs_scrub.8 index 1fd122f2a..1ed4b176b 100644 --- a/man/man8/xfs_scrub.8 +++ b/man/man8/xfs_scrub.8 @@ -107,6 +107,14 @@ The supported are: .RS 1.0i .TP +.B autofsck +Decide the operating mode from the value of the +.I autofsck +filesystem property. +See the +.B filesystem properties +section for more details. +.TP .BI fstrim_pct= percentage To constrain the amount of time spent on fstrim activities during phase 8, this program tries to balance estimated runtime against completeness of the @@ -192,6 +200,44 @@ Scheduling a quotacheck for the next mount. .PP If corrupt metadata is successfully repaired, this program will log that a repair has succeeded instead of a corruption report. +.SH FILESYSTEM PROPERTIES +System administrators can convey their preferences for scrubbing of a +particular filesystem by setting the filesystem property +.B autofsck +via the +.BR xfs_property (8) +command on the filesystem. +These preferences will be honored if the +.B -o autofsck +option is specified. + +Recognized values for the +.B autofsck +property are: +.RS +.TP +.I none +Do not scan the filesystem at all. +.TP +.I check +Scan and report corruption and opportunities for optimization, but do not +change anything. +.TP +.I optimize +Scan the filesystem and optimize where possible. +Report corruptions, but do not fix them. +.TP +.I repair +Scan the filesystem, fix corruptions, and optimize where possible. +.RE + +If the property is not set, the default is +.I check +if the filesystem has either reverse mapping btrees or parent pointers enabled, +or +.I none +otherwise. + .SH EXIT CODE The exit code returned by .B xfs_scrub diff --git a/scrub/phase1.c b/scrub/phase1.c index 091b59e57..d03a9099a 100644 --- a/scrub/phase1.c +++ b/scrub/phase1.c @@ -28,6 +28,8 @@ #include "repair.h" #include "libfrog/fsgeom.h" #include "xfs_errortag.h" +#include "libfrog/fsprops.h" +#include "libfrog/fsproperties.h" /* Phase 1: Find filesystem geometry (and clean up after) */ @@ -130,6 +132,87 @@ enable_force_repair( return error; } +/* + * Decide the operating mode from the autofsck fs property. No fs property or + * system errors means we check the fs if rmapbt or pptrs are enabled, or none + * if it doesn't. + */ +static void +mode_from_autofsck( + struct scrub_ctx *ctx) +{ + struct fsprops_handle fph = { }; + char valuebuf[FSPROP_MAX_VALUELEN + 1] = { 0 }; + size_t valuelen = FSPROP_MAX_VALUELEN; + enum fsprop_autofsck shval; + int ret; + + ret = fsprops_open_handle(&ctx->mnt, &ctx->fsinfo, &fph); + if (ret) + goto no_property; + + ret = fsprops_get(&fph, FSPROP_AUTOFSCK_NAME, valuebuf, &valuelen); + if (ret) + goto no_property; + + shval = fsprop_autofsck_read(valuebuf); + switch (shval) { + case FSPROP_AUTOFSCK_NONE: + ctx->mode = SCRUB_MODE_NONE; + break; + case FSPROP_AUTOFSCK_OPTIMIZE: + ctx->mode = SCRUB_MODE_PREEN; + break; + case FSPROP_AUTOFSCK_REPAIR: + ctx->mode = SCRUB_MODE_REPAIR; + break; + case FSPROP_AUTOFSCK_UNSET: + str_info(ctx, ctx->mntpoint, + _("Unknown autofsck directive \"%s\"."), + valuebuf); + goto no_property; + case FSPROP_AUTOFSCK_CHECK: + ctx->mode = SCRUB_MODE_DRY_RUN; + break; + } + + fsprops_free_handle(&fph); + +summarize: + switch (ctx->mode) { + case SCRUB_MODE_NONE: + str_info(ctx, ctx->mntpoint, + _("Disabling scrub per autofsck directive.")); + break; + case SCRUB_MODE_DRY_RUN: + str_info(ctx, ctx->mntpoint, + _("Checking per autofsck directive.")); + break; + case SCRUB_MODE_PREEN: + str_info(ctx, ctx->mntpoint, + _("Optimizing per autofsck directive.")); + break; + case SCRUB_MODE_REPAIR: + str_info(ctx, ctx->mntpoint, + _("Checking and repairing per autofsck directive.")); + break; + } + + return; +no_property: + /* + * If we don't find an autofsck property, check the metadata if any + * backrefs are available for cross-referencing. Otherwise do no + * checking. + */ + if (ctx->mnt.fsgeom.flags & (XFS_FSOP_GEOM_FLAGS_PARENT | + XFS_FSOP_GEOM_FLAGS_RMAPBT)) + ctx->mode = SCRUB_MODE_DRY_RUN; + else + ctx->mode = SCRUB_MODE_NONE; + goto summarize; +} + /* * Bind to the mountpoint, read the XFS geometry, bind to the block devices. * Anything we've already built will be cleaned up by scrub_cleanup. @@ -206,6 +289,14 @@ _("Not an XFS filesystem.")); return error; } + /* + * If we've been instructed to decide the operating mode from the + * autofsck fs property, do that now before we start downgrading based + * on actual fs/kernel capabilities. + */ + if (ctx->mode == SCRUB_MODE_NONE) + mode_from_autofsck(ctx); + /* Do we have kernel-assisted metadata scrubbing? */ if (!can_scrub_fs_metadata(ctx) || !can_scrub_inode(ctx) || !can_scrub_bmap(ctx) || !can_scrub_dir(ctx) || diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c index f5b58de12..3e7d9138f 100644 --- a/scrub/xfs_scrub.c +++ b/scrub/xfs_scrub.c @@ -526,6 +526,10 @@ _("Scrub aborted after phase %d."), if (ret) break; + /* Did background scrub get canceled on us? */ + if (ctx->mode == SCRUB_MODE_NONE) + break; + /* Too many errors? */ if (scrub_excessive_errors(ctx)) { ret = ECANCELED; @@ -630,12 +634,14 @@ report_outcome( enum o_opt_nums { IWARN = 0, FSTRIM_PCT, + AUTOFSCK, O_MAX_OPTS, }; static char *o_opts[] = { [IWARN] = "iwarn", [FSTRIM_PCT] = "fstrim_pct", + [AUTOFSCK] = "autofsck", [O_MAX_OPTS] = NULL, }; @@ -688,6 +694,14 @@ parse_o_opts( ctx->fstrim_block_pct = dval / 100.0; break; + case AUTOFSCK: + if (val) { + fprintf(stderr, + _("-o autofsck does not take an argument\n")); + usage(); + } + ctx->mode = SCRUB_MODE_NONE; + break; default: usage(); break; diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h index 4d9a02892..5d336cb55 100644 --- a/scrub/xfs_scrub.h +++ b/scrub/xfs_scrub.h @@ -26,6 +26,13 @@ extern bool use_force_rebuild; extern bool info_is_warning; enum scrub_mode { + /* + * Prior to phase 1, this means that xfs_scrub should read the + * "autofsck" fs property from the mount and set the value + * appropriate. If it's still set after phase 1, this means we should + * exit without doing anything. + */ + SCRUB_MODE_NONE, SCRUB_MODE_DRY_RUN, SCRUB_MODE_PREEN, SCRUB_MODE_REPAIR, ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 2/4] xfs_scrub: allow sysadmin to control background scrubs 2024-08-06 18:21 ` [PATCH 2/4] xfs_scrub: allow sysadmin to control background scrubs Darrick J. Wong @ 2024-08-07 16:12 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:12 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, hch, dchinner, fstests, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/4] xfs_scrub: use the autofsck fsproperty to select mode 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong 2024-08-06 18:21 ` [PATCH 1/4] libfrog: define a autofsck filesystem property Darrick J. Wong 2024-08-06 18:21 ` [PATCH 2/4] xfs_scrub: allow sysadmin to control background scrubs Darrick J. Wong @ 2024-08-06 18:21 ` Darrick J. Wong 2024-08-07 16:12 ` Christoph Hellwig 2024-08-06 18:22 ` [PATCH 4/4] mkfs: set autofsck filesystem property Darrick J. Wong 3 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:21 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Now that we can set properties on xfs filesystems, make the xfs_scrub background service query the autofsck property to figure out which operating mode it should use. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- scrub/Makefile | 3 +-- scrub/xfs_scrub@.service.in | 2 +- scrub/xfs_scrub_media@.service.in | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/scrub/Makefile b/scrub/Makefile index 885b43e99..53e8cb02a 100644 --- a/scrub/Makefile +++ b/scrub/Makefile @@ -17,7 +17,7 @@ INSTALL_SCRUB = install-scrub XFS_SCRUB_ALL_PROG = xfs_scrub_all XFS_SCRUB_FAIL_PROG = xfs_scrub_fail XFS_SCRUB_ARGS = -p -XFS_SCRUB_SERVICE_ARGS = -b +XFS_SCRUB_SERVICE_ARGS = -b -o autofsck ifeq ($(HAVE_SYSTEMD),yes) INSTALL_SCRUB += install-systemd SYSTEMD_SERVICES=\ @@ -144,7 +144,6 @@ install: $(INSTALL_SCRUB) @echo " [SED] $@" $(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \ -e "s|@scrub_service_args@|$(XFS_SCRUB_SERVICE_ARGS)|g" \ - -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" \ -e "s|@pkg_libexec_dir@|$(PKG_LIBEXEC_DIR)|g" \ -e "s|@pkg_state_dir@|$(PKG_STATE_DIR)|g" \ -e "s|@media_scan_interval@|$(XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_INTERVAL)|g" \ diff --git a/scrub/xfs_scrub@.service.in b/scrub/xfs_scrub@.service.in index 5fa5f3282..fb38319e9 100644 --- a/scrub/xfs_scrub@.service.in +++ b/scrub/xfs_scrub@.service.in @@ -22,7 +22,7 @@ RequiresMountsFor=%f [Service] Type=oneshot Environment=SERVICE_MODE=1 -ExecStart=@sbindir@/xfs_scrub @scrub_service_args@ @scrub_args@ -M /tmp/scrub/ %f +ExecStart=@sbindir@/xfs_scrub @scrub_service_args@ -M /tmp/scrub/ %f SyslogIdentifier=%N # Run scrub with minimal CPU and IO priority so that nothing else will starve. diff --git a/scrub/xfs_scrub_media@.service.in b/scrub/xfs_scrub_media@.service.in index e670748ce..98cd1ac44 100644 --- a/scrub/xfs_scrub_media@.service.in +++ b/scrub/xfs_scrub_media@.service.in @@ -22,7 +22,7 @@ RequiresMountsFor=%f [Service] Type=oneshot Environment=SERVICE_MODE=1 -ExecStart=@sbindir@/xfs_scrub @scrub_service_args@ @scrub_args@ -M /tmp/scrub/ -x %f +ExecStart=@sbindir@/xfs_scrub @scrub_service_args@ -M /tmp/scrub/ -x %f SyslogIdentifier=%N # Run scrub with minimal CPU and IO priority so that nothing else will starve. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 3/4] xfs_scrub: use the autofsck fsproperty to select mode 2024-08-06 18:21 ` [PATCH 3/4] xfs_scrub: use the autofsck fsproperty to select mode Darrick J. Wong @ 2024-08-07 16:12 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:12 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, hch, dchinner, fstests, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 4/4] mkfs: set autofsck filesystem property 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong ` (2 preceding siblings ...) 2024-08-06 18:21 ` [PATCH 3/4] xfs_scrub: use the autofsck fsproperty to select mode Darrick J. Wong @ 2024-08-06 18:22 ` Darrick J. Wong 2024-08-07 16:13 ` Christoph Hellwig 3 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:22 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add a new mkfs option -m autofsck so that sysadmins can control the background scrubbing behavior of filesystems from the start. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- man/man8/mkfs.xfs.8.in | 6 ++ mkfs/lts_4.19.conf | 1 mkfs/lts_5.10.conf | 1 mkfs/lts_5.15.conf | 1 mkfs/lts_5.4.conf | 1 mkfs/lts_6.1.conf | 1 mkfs/lts_6.6.conf | 1 mkfs/xfs_mkfs.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 133 insertions(+), 1 deletion(-) diff --git a/man/man8/mkfs.xfs.8.in b/man/man8/mkfs.xfs.8.in index d5a0783ac..a854b0e87 100644 --- a/man/man8/mkfs.xfs.8.in +++ b/man/man8/mkfs.xfs.8.in @@ -323,6 +323,12 @@ option set. When the option .B \-m crc=0 is used, the reference count btree feature is not supported and reflink is disabled. +.TP +.BI autofsck= value +Set the autofsck filesystem property to this value. +See the +.BI xfs_scrub (8) +manual page for more information on this property. .RE .PP .PD 0 diff --git a/mkfs/lts_4.19.conf b/mkfs/lts_4.19.conf index 9fa1f9378..4f190bacf 100644 --- a/mkfs/lts_4.19.conf +++ b/mkfs/lts_4.19.conf @@ -8,6 +8,7 @@ finobt=1 inobtcount=0 reflink=0 rmapbt=0 +autofsck=0 [inode] sparse=1 diff --git a/mkfs/lts_5.10.conf b/mkfs/lts_5.10.conf index d64bcdf8c..a55fc68e4 100644 --- a/mkfs/lts_5.10.conf +++ b/mkfs/lts_5.10.conf @@ -8,6 +8,7 @@ finobt=1 inobtcount=0 reflink=1 rmapbt=0 +autofsck=0 [inode] sparse=1 diff --git a/mkfs/lts_5.15.conf b/mkfs/lts_5.15.conf index 775fd9ab9..daea0b406 100644 --- a/mkfs/lts_5.15.conf +++ b/mkfs/lts_5.15.conf @@ -8,6 +8,7 @@ finobt=1 inobtcount=1 reflink=1 rmapbt=0 +autofsck=0 [inode] sparse=1 diff --git a/mkfs/lts_5.4.conf b/mkfs/lts_5.4.conf index 6f43a6c6d..0f807fc35 100644 --- a/mkfs/lts_5.4.conf +++ b/mkfs/lts_5.4.conf @@ -8,6 +8,7 @@ finobt=1 inobtcount=0 reflink=1 rmapbt=0 +autofsck=0 [inode] sparse=1 diff --git a/mkfs/lts_6.1.conf b/mkfs/lts_6.1.conf index a78a4f9e3..0ff5bbad5 100644 --- a/mkfs/lts_6.1.conf +++ b/mkfs/lts_6.1.conf @@ -8,6 +8,7 @@ finobt=1 inobtcount=1 reflink=1 rmapbt=0 +autofsck=0 [inode] sparse=1 diff --git a/mkfs/lts_6.6.conf b/mkfs/lts_6.6.conf index 91a25bd81..2ef5957e0 100644 --- a/mkfs/lts_6.6.conf +++ b/mkfs/lts_6.6.conf @@ -8,6 +8,7 @@ finobt=1 inobtcount=1 reflink=1 rmapbt=1 +autofsck=0 [inode] sparse=1 diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 394a35771..bbd0dbb6c 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -12,6 +12,7 @@ #include "libfrog/convert.h" #include "libfrog/crc32cselftest.h" #include "libfrog/dahashselftest.h" +#include "libfrog/fsproperties.h" #include "proto.h" #include <ini.h> @@ -148,6 +149,7 @@ enum { M_REFLINK, M_INOBTCNT, M_BIGTIME, + M_AUTOFSCK, M_MAX_OPTS, }; @@ -809,6 +811,7 @@ static struct opt_params mopts = { [M_REFLINK] = "reflink", [M_INOBTCNT] = "inobtcount", [M_BIGTIME] = "bigtime", + [M_AUTOFSCK] = "autofsck", [M_MAX_OPTS] = NULL, }, .subopt_params = { @@ -852,6 +855,12 @@ static struct opt_params mopts = { .maxval = 1, .defaultval = 1, }, + { .index = M_AUTOFSCK, + .conflicts = { { NULL, LAST_CONFLICT } }, + .minval = 0, + .maxval = 1, + .defaultval = 1, + }, }, }; @@ -917,6 +926,8 @@ struct cli_params { char *cfgfile; char *protofile; + enum fsprop_autofsck autofsck; + /* parameters that depend on sector/block size being validated. */ char *dsize; char *agsize; @@ -1037,7 +1048,7 @@ usage( void ) /* blocksize */ [-b size=num]\n\ /* config file */ [-c options=xxx]\n\ /* metadata */ [-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1,reflink=0|1,\n\ - inobtcount=0|1,bigtime=0|1]\n\ + inobtcount=0|1,bigtime=0|1,autofsck=xxx]\n\ /* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num,\n\ (sunit=value,swidth=value|su=num,sw=num|noalign),\n\ sectsize=num,concurrency=num]\n\ @@ -1858,6 +1869,20 @@ meta_opts_parser( case M_BIGTIME: cli->sb_feat.bigtime = getnum(value, opts, subopt); break; + case M_AUTOFSCK: + if (!value || value[0] == 0 || isdigit(value[0])) { + long long ival = getnum(value, opts, subopt); + + if (ival) + cli->autofsck = FSPROP_AUTOFSCK_REPAIR; + else + cli->autofsck = FSPROP_AUTOFSCK_NONE; + } else { + cli->autofsck = fsprop_autofsck_read(value); + if (cli->autofsck == FSPROP_AUTOFSCK_UNSET) + illegal(value, "m autofsck"); + } + break; default: return -EINVAL; } @@ -2323,6 +2348,32 @@ _("Directory ftype field always enabled on CRC enabled filesystems\n")); usage(); } + /* + * Self-healing through online fsck relies heavily on back + * reference metadata, so we really want to try to enable rmap + * and parent pointers. + */ + if (cli->autofsck >= FSPROP_AUTOFSCK_CHECK) { + if (!cli->sb_feat.rmapbt) { + if (cli_opt_set(&mopts, M_RMAPBT)) { + fprintf(stdout, +_("-m autofsck=%s is less effective without reverse mapping\n"), + fsprop_autofsck_write(cli->autofsck)); + } else { + cli->sb_feat.rmapbt = true; + } + } + if (!cli->sb_feat.parent_pointers) { + if (cli_opt_set(&nopts, N_PARENT)) { + fprintf(stdout, +_("-m autofsck=%s is less effective without parent pointers\n"), + fsprop_autofsck_write(cli->autofsck)); + } else { + cli->sb_feat.parent_pointers = true; + } + } + } + } else { /* !crcs_enabled */ /* * The V4 filesystem format is deprecated in the upstream Linux @@ -2406,6 +2457,14 @@ _("parent pointers not supported without CRC support\n")); usage(); } cli->sb_feat.parent_pointers = false; + + if (cli->autofsck != FSPROP_AUTOFSCK_UNSET && + cli_opt_set(&mopts, M_AUTOFSCK)) { + fprintf(stderr, +_("autofsck not supported without CRC support\n")); + usage(); + } + cli->autofsck = FSPROP_AUTOFSCK_UNSET; } if (!cli->sb_feat.finobt) { @@ -4332,6 +4391,63 @@ cfgfile_parse( cli->cfgfile); } +static void +set_autofsck( + struct xfs_mount *mp, + struct cli_params *cli) +{ + struct xfs_da_args args = { + .geo = mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .op_flags = XFS_DA_OP_OKNOENT, + .attr_filter = LIBXFS_ATTR_ROOT, + .owner = mp->m_sb.sb_rootino, + }; + const char *word; + char *p; + int error; + + error = fsprop_name_to_attr_name(FSPROP_AUTOFSCK_NAME, &p); + if (error < 0) { + fprintf(stderr, + _("%s: error %d while allocating fs property name\n"), + progname, error); + exit(1); + } + args.namelen = error; + args.name = (const uint8_t *)p; + + word = fsprop_autofsck_write(cli->autofsck); + if (!word) { + fprintf(stderr, + _("%s: not sure what to do with autofsck value %u\n"), + progname, cli->autofsck); + exit(1); + } + args.value = (void *)word; + args.valuelen = strlen(word); + + error = -libxfs_iget(mp, NULL, mp->m_sb.sb_rootino, 0, &args.dp); + if (error) { + fprintf(stderr, + _("%s: error %d while opening root directory\n"), + progname, error); + exit(1); + } + + libxfs_attr_sethash(&args); + + error = -libxfs_attr_set(&args, XFS_ATTRUPDATE_UPSERT, false); + if (error) { + fprintf(stderr, + _("%s: error %d while setting autofsck property\n"), + progname, error); + exit(1); + } + + libxfs_irele(args.dp); +} + int main( int argc, @@ -4361,6 +4477,7 @@ main( .is_supported = 1, .data_concurrency = -1, /* auto detect non-mechanical storage */ .log_concurrency = -1, /* auto detect non-mechanical ddev */ + .autofsck = FSPROP_AUTOFSCK_UNSET, }; struct mkfs_params cfg = {}; @@ -4669,6 +4786,9 @@ main( if (mp->m_sb.sb_agcount > 1) rewrite_secondary_superblocks(mp); + if (cli.autofsck != FSPROP_AUTOFSCK_UNSET) + set_autofsck(mp, &cli); + /* * Dump all inodes and buffers before marking us all done. * Need to drop references to inodes we still hold, first. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 4/4] mkfs: set autofsck filesystem property 2024-08-06 18:22 ` [PATCH 4/4] mkfs: set autofsck filesystem property Darrick J. Wong @ 2024-08-07 16:13 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:13 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, hch, dchinner, fstests, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHSET v30.10 3/3] debian: enable xfs_scrub_all by default 2024-08-06 18:14 [PATCHBOMB v30.10] xfsprogs-6.10: filesystem properties Darrick J. Wong 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong @ 2024-08-06 18:19 ` Darrick J. Wong 2024-08-06 18:22 ` [PATCH 1/1] debian: enable xfs_scrub_all systemd timer services " Darrick J. Wong 2024-08-06 18:19 ` [PATCHSET v30.10] fstests: xfs filesystem properties Darrick J. Wong 3 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:19 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs Hi all, Update our packaging to enable the background xfs_scrub timer by default. This won't do much unless the sysadmin sets the autofsck fs property or formats a filesystem with backref metadata. If you're going to start using this code, I strongly recommend pulling from my git trees, which are linked below. This has been running on the djcloud for months with no problems. Enjoy! Comments and questions are, as always, welcome. xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=debian-autofsck-6.10 --- Commits in this patchset: * debian: enable xfs_scrub_all systemd timer services by default --- debian/rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/1] debian: enable xfs_scrub_all systemd timer services by default 2024-08-06 18:19 ` [PATCHSET v30.10 3/3] debian: enable xfs_scrub_all by default Darrick J. Wong @ 2024-08-06 18:22 ` Darrick J. Wong 2024-08-07 16:13 ` Christoph Hellwig 0 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:22 UTC (permalink / raw) To: djwong, cem; +Cc: hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Now that we're finished building online fsck, enable the periodic background scrub service by default. This involves the postinst script starting the resource management slice and the timer. No other sub-services need to be enabled or unmasked explicitly. They also shouldn't be started or restarted because that might interrupt background operation unnecessarily. Although the xfs_scrub_all timer is activated by default, the individual xfs_scrub@ services that it spawns will only do real work on filesystems that are new enough to have back reference metadata available. This avoids surprises for people who are upgrading Debian; only new installs with new mkfs will get any automatic fsck behavior. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- debian/rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/rules b/debian/rules index 69a79fc67..c3fbcd262 100755 --- a/debian/rules +++ b/debian/rules @@ -114,7 +114,7 @@ binary-arch: checkroot built dh_compress dh_fixperms dh_makeshlibs - dh_installsystemd -p xfsprogs --no-enable --no-start --no-restart-after-upgrade --no-stop-on-upgrade + dh_installsystemd -p xfsprogs --no-restart-after-upgrade --no-stop-on-upgrade system-xfs_scrub.slice xfs_scrub_all.timer dh_installdeb dh_shlibdeps dh_gencontrol ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/1] debian: enable xfs_scrub_all systemd timer services by default 2024-08-06 18:22 ` [PATCH 1/1] debian: enable xfs_scrub_all systemd timer services " Darrick J. Wong @ 2024-08-07 16:13 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:13 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, hch, dchinner, fstests, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHSET v30.10] fstests: xfs filesystem properties 2024-08-06 18:14 [PATCHBOMB v30.10] xfsprogs-6.10: filesystem properties Darrick J. Wong ` (2 preceding siblings ...) 2024-08-06 18:19 ` [PATCHSET v30.10 3/3] debian: enable xfs_scrub_all by default Darrick J. Wong @ 2024-08-06 18:19 ` Darrick J. Wong 2024-08-06 18:22 ` [PATCH 1/1] xfs: functional testing for " Darrick J. Wong 3 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:19 UTC (permalink / raw) To: djwong, zlang; +Cc: fstests, hch, dchinner, fstests, linux-xfs Hi all, It would be very useful if system administrators could set properties for a given xfs filesystem to control its behavior. This we can do easily and extensibly by setting ATTR_ROOT (aka "trusted") extended attributes on the root directory. To prevent this from becoming a weird free for all, let's add some library and tooling support so that sysadmins simply run the xfs_property program to administer these properties. If you're going to start using this code, I strongly recommend pulling from my git trees, which are linked below. This has been running on the djcloud for months with no problems. Enjoy! Comments and questions are, as always, welcome. --D xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=filesystem-properties fstests git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=filesystem-properties --- Commits in this patchset: * xfs: functional testing for filesystem properties --- common/config | 1 common/xfs | 14 ++++- doc/group-names.txt | 1 tests/generic/062 | 4 + tests/xfs/1886 | 137 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/1886.out | 53 ++++++++++++++++++++ tests/xfs/1887 | 122 +++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/1887.out | 46 +++++++++++++++++ tests/xfs/1888 | 66 +++++++++++++++++++++++++ tests/xfs/1888.out | 9 +++ tests/xfs/1889 | 63 +++++++++++++++++++++++ tests/xfs/1889.out | 8 +++ 12 files changed, 522 insertions(+), 2 deletions(-) create mode 100755 tests/xfs/1886 create mode 100644 tests/xfs/1886.out create mode 100755 tests/xfs/1887 create mode 100644 tests/xfs/1887.out create mode 100755 tests/xfs/1888 create mode 100644 tests/xfs/1888.out create mode 100755 tests/xfs/1889 create mode 100644 tests/xfs/1889.out ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/1] xfs: functional testing for filesystem properties 2024-08-06 18:19 ` [PATCHSET v30.10] fstests: xfs filesystem properties Darrick J. Wong @ 2024-08-06 18:22 ` Darrick J. Wong 2024-08-07 16:13 ` Christoph Hellwig 0 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-08-06 18:22 UTC (permalink / raw) To: djwong, zlang; +Cc: fstests, hch, dchinner, fstests, linux-xfs From: Darrick J. Wong <djwong@kernel.org> Make sure that fs property storage and retrieval actually work. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- common/config | 1 common/xfs | 14 ++++- doc/group-names.txt | 1 tests/generic/062 | 4 + tests/xfs/1886 | 137 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/1886.out | 53 ++++++++++++++++++++ tests/xfs/1887 | 122 +++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/1887.out | 46 +++++++++++++++++ tests/xfs/1888 | 66 +++++++++++++++++++++++++ tests/xfs/1888.out | 9 +++ tests/xfs/1889 | 63 +++++++++++++++++++++++ tests/xfs/1889.out | 8 +++ 12 files changed, 522 insertions(+), 2 deletions(-) create mode 100755 tests/xfs/1886 create mode 100644 tests/xfs/1886.out create mode 100755 tests/xfs/1887 create mode 100644 tests/xfs/1887.out create mode 100755 tests/xfs/1888 create mode 100644 tests/xfs/1888.out create mode 100755 tests/xfs/1889 create mode 100644 tests/xfs/1889.out diff --git a/common/config b/common/config index 22740c0af8..07bd2cf315 100644 --- a/common/config +++ b/common/config @@ -234,6 +234,7 @@ export GZIP_PROG="$(type -P gzip)" export BTRFS_IMAGE_PROG="$(type -P btrfs-image)" export BTRFS_MAP_LOGICAL_PROG=$(type -P btrfs-map-logical) export PARTED_PROG="$(type -P parted)" +export XFS_PROPERTY_PROG="$(type -P xfs_property)" # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. # newer systems have udevadm command but older systems like RHEL5 don't. diff --git a/common/xfs b/common/xfs index bd40a02ed2..5da2208164 100644 --- a/common/xfs +++ b/common/xfs @@ -1318,8 +1318,8 @@ _require_xfs_spaceman_command() testfile=$TEST_DIR/$$.xfs_spaceman touch $testfile case $command in - "health") - testio=`$XFS_SPACEMAN_PROG -c "health $param" $TEST_DIR 2>&1` + "health"|"listfsprops") + testio=`$XFS_SPACEMAN_PROG -c "$command $param" $TEST_DIR 2>&1` param_checked=1 ;; *) @@ -1812,3 +1812,13 @@ _require_xfs_parent() || _notrun "kernel does not support parent pointers" _scratch_unmount } + +# Wipe all filesystem properties from an xfs filesystem. The sole argument +# must be the root directory of a filesystem. +_wipe_xfs_properties() +{ + getfattr --match="^trusted.xfs:" --absolute-names --dump --encoding=hex "$1" | \ + grep '=' | sed -e 's/=.*$//g' | while read name; do + setfattr --remove="$name" "$1" + done +} diff --git a/doc/group-names.txt b/doc/group-names.txt index 6cf717969d..ed886caac0 100644 --- a/doc/group-names.txt +++ b/doc/group-names.txt @@ -56,6 +56,7 @@ fiexchange XFS_IOC_EXCHANGE_RANGE ioctl freeze filesystem freeze tests fsck general fsck tests fsmap FS_IOC_GETFSMAP ioctl +fsproperties Filesystem properties fsr XFS free space reorganizer fuzzers filesystem fuzz tests growfs increasing the size of a filesystem diff --git a/tests/generic/062 b/tests/generic/062 index 8f4dfcbf55..f0904992d1 100755 --- a/tests/generic/062 +++ b/tests/generic/062 @@ -75,6 +75,10 @@ fi _require_attrs $ATTR_MODES +# Wipe all xfs filesystem properties (which are rootdir xattrs) before we dump +# them all later. +test $FSTYP = "xfs" && _wipe_xfs_properties $SCRATCH_MNT + for nsp in $ATTR_MODES; do for inode in reg dir lnk dev/b dev/c dev/p; do diff --git a/tests/xfs/1886 b/tests/xfs/1886 new file mode 100755 index 0000000000..68abfedd15 --- /dev/null +++ b/tests/xfs/1886 @@ -0,0 +1,137 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Oracle. All Rights Reserved. +# +# FS QA Test 1886 +# +# Functional testing for low level filesystem property manipulation by +# xfs_{spaceman,db}. +# +. ./common/preamble +_begin_fstest auto fsproperties + +. ./common/filter +. ./common/attr + +_require_test +_require_user fsgqa +_require_attrs +_require_xfs_io_command listfsprops +_require_xfs_db_command attr_list + +_cleanup() + +{ + cd / + rm -r -f $tmp.* + rm -f $TEST_DIR/$seq.somefile + rm -r -f $TEST_DIR/$seq.somedir + test -n "$propname" && $ATTR_PROG -R -r $propname $TEST_DEV &>/dev/null +} + +filter_inum() +{ + sed -e 's/inode [0-9]*/inode XXX/g' +} + +propname="fakeproperty" # must not be an actual property +propval="1721943740" +longpropname="$(perl -e 'print "x" x 300;')" +longpropval="$(perl -e 'print "x" x 80000;')" + +echo "*** IO TEST ***" + +echo empty get property +$XFS_IO_PROG -c "getfsprops $propname" $TEST_DIR + +echo pointless remove property +$XFS_IO_PROG -c "removefsprops $propname" $TEST_DIR + +echo list property +$XFS_IO_PROG -c "listfsprops" $TEST_DIR | grep $propname + +echo set property +$XFS_IO_PROG -c "setfsprops $propname=$propval" $TEST_DIR + +echo list property +$XFS_IO_PROG -c "listfsprops" $TEST_DIR | grep $propname + +echo dump xattrs +$ATTR_PROG -R -l $TEST_DIR | grep $propname | _filter_test_dir + +echo get property +$XFS_IO_PROG -c "getfsprops $propname" $TEST_DIR + +echo list property +$XFS_IO_PROG -c "listfsprops" $TEST_DIR | grep $propname + +echo child file rejected +touch $TEST_DIR/$seq.somefile +$XFS_IO_PROG -c "listfsprops $propname" $TEST_DIR/$seq.somefile 2>&1 | \ + _filter_test_dir + +echo child dir rejected +mkdir -p $TEST_DIR/$seq.somedir +$XFS_IO_PROG -c "listfsprops $propname" $TEST_DIR/$seq.somedir 2>&1 | \ + _filter_test_dir + +echo remove property +$XFS_IO_PROG -c "removefsprops $propname" $TEST_DIR + +echo pointless remove property +$XFS_IO_PROG -c "removefsprops $propname" $TEST_DIR + +echo set too long name +$XFS_IO_PROG -c "setfsprops $longpropname=$propval" $TEST_DIR + +echo set too long value +$XFS_IO_PROG -c "setfsprops $propname=$longpropval" $TEST_DIR + +echo not enough permissions +su - "$qa_user" -c "$XFS_IO_PROG -c \"setfsprops $propname=$propval\" $TEST_DIR" 2>&1 | _filter_test_dir + +echo "*** DB TEST ***" + +propval=$((propval + 1)) +_test_unmount + +echo empty get property +_test_xfs_db -x -c 'path /' -c "attr_get -Z $propname" 2>&1 | filter_inum + +echo pointless remove property +_test_xfs_db -x -c 'path /' -c "attr_remove -Z $propname" 2>&1 | filter_inum + +echo list property +_test_xfs_db -x -c 'path /' -c "attr_list -Z" | grep $propname + +echo set property +_test_xfs_db -x -c 'path /' -c "attr_set -Z $propname $propval" + +echo list property +_test_xfs_db -x -c 'path /' -c "attr_list -Z" | grep $propname + +echo dump xattrs +_test_mount +$ATTR_PROG -R -l $TEST_DIR | grep $propname | _filter_test_dir +_test_unmount + +echo get property +_test_xfs_db -x -c 'path /' -c "attr_get -Z $propname" + +echo list property +_test_xfs_db -x -c 'path /' -c "attr_list -Z" | grep $propname + +echo remove property +_test_xfs_db -x -c 'path /' -c "attr_remove -Z $propname" + +echo pointless remove property +_test_xfs_db -x -c 'path /' -c "attr_remove -Z $propname" 2>&1 | filter_inum + +echo set too long name +_test_xfs_db -x -c 'path /' -c "attr_set -Z $longpropname $propval" + +echo set too long value +_test_xfs_db -x -c 'path /' -c "attr_set -Z $propname $longpropval" + +status=0 +exit diff --git a/tests/xfs/1886.out b/tests/xfs/1886.out new file mode 100644 index 0000000000..e02d810406 --- /dev/null +++ b/tests/xfs/1886.out @@ -0,0 +1,53 @@ +QA output created by 1886 +*** IO TEST *** +empty get property +fakeproperty: No data available +pointless remove property +fakeproperty: No data available +list property +set property +fakeproperty=1721943740 +list property +fakeproperty +dump xattrs +Attribute "xfs:fakeproperty" has a 10 byte value for TEST_DIR +get property +fakeproperty=1721943740 +list property +fakeproperty +child file rejected +TEST_DIR/1886.somefile: Not a XFS mount point. +child dir rejected +TEST_DIR/1886.somedir: Not a XFS mount point. +remove property +pointless remove property +fakeproperty: No data available +set too long name +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: Invalid argument +set too long value +fakeproperty: Invalid argument +not enough permissions +TEST_DIR: Operation not permitted +*** DB TEST *** +empty get property +failed to get attr xfs:fakeproperty on inode XXX: No data available +pointless remove property +failed to remove attr xfs:fakeproperty from inode XXX: No data available +list property +set property +fakeproperty=1721943741 +list property +fakeproperty +dump xattrs +Attribute "xfs:fakeproperty" has a 10 byte value for TEST_DIR +get property +fakeproperty=1721943741 +list property +fakeproperty +remove property +pointless remove property +failed to remove attr xfs:fakeproperty from inode XXX: No data available +set too long name +name too long +set too long value +xfs:fakeproperty: value too long diff --git a/tests/xfs/1887 b/tests/xfs/1887 new file mode 100755 index 0000000000..cd70c42021 --- /dev/null +++ b/tests/xfs/1887 @@ -0,0 +1,122 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Oracle. All Rights Reserved. +# +# FS QA Test 1887 +# +# Functional testing for xfs_property the wrapper script. +# +. ./common/preamble +_begin_fstest auto fsproperties + +. ./common/filter +. ./common/attr + +_require_test +_require_attrs +_require_command "$XFS_PROPERTY_PROG" xfs_property +_require_xfs_io_command listfsprops # actually detect support + +_cleanup() + +{ + cd / + rm -r -f $tmp.* + rm -f $TEST_DIR/$seq.somefile + rm -r -f $TEST_DIR/$seq.somedir + test -n "$propname" && $ATTR_PROG -R -r $propname $TEST_DEV &>/dev/null +} + +filter_inum() +{ + sed -e 's/inode [0-9]*/inode XXX/g' +} + +propname="fakeproperty" # must not be an actual property +propval="1721943742" +longpropname="$(perl -e 'print "x" x 300;')" +longpropval="$(perl -e 'print "x" x 80000;')" + +echo "*** OFFLINE XFS_PROPERTY TEST ***" + +_test_unmount + +echo empty get property +$XFS_PROPERTY_PROG $TEST_DEV get "$propname" 2>&1 | filter_inum + +echo pointless remove property +$XFS_PROPERTY_PROG $TEST_DEV remove "$propname" 2>&1 | filter_inum + +echo list property +$XFS_PROPERTY_PROG $TEST_DEV list | grep $propname + +echo set property +$XFS_PROPERTY_PROG $TEST_DEV set "$propname=$propval" + +echo list property +$XFS_PROPERTY_PROG $TEST_DEV list | grep $propname + +echo dump xattrs +$ATTR_PROG -R -l $TEST_DEV | grep $propname | _filter_test_dir + +echo get property +$XFS_PROPERTY_PROG $TEST_DEV get "$propname" + +echo list property +$XFS_PROPERTY_PROG $TEST_DEV list | grep $propname + +echo remove property +$XFS_PROPERTY_PROG $TEST_DEV remove "$propname" + +echo pointless remove property +$XFS_PROPERTY_PROG $TEST_DEV remove "$propname" 2>&1 | filter_inum + +echo set too long name +$XFS_PROPERTY_PROG $TEST_DEV set "$longpropname=$propval" + +echo set too long value +$XFS_PROPERTY_PROG $TEST_DEV set "$propname=$longpropval" + +echo "*** ONLINE XFS_PROPERTY TEST ***" + +propval=$((propval+1)) +_test_mount + +echo empty get property +$XFS_PROPERTY_PROG $TEST_DIR get "$propname" + +echo pointless remove property +$XFS_PROPERTY_PROG $TEST_DIR remove "$propname" + +echo list property +$XFS_PROPERTY_PROG $TEST_DIR list | grep $propname + +echo set property +$XFS_PROPERTY_PROG $TEST_DIR set "$propname=$propval" + +echo list property +$XFS_PROPERTY_PROG $TEST_DIR list | grep $propname + +echo dump xattrs +$ATTR_PROG -R -l $TEST_DIR | grep $propname | _filter_test_dir + +echo get property +$XFS_PROPERTY_PROG $TEST_DIR get "$propname" + +echo list property +$XFS_PROPERTY_PROG $TEST_DIR list | grep $propname + +echo remove property +$XFS_PROPERTY_PROG $TEST_DIR remove "$propname" + +echo pointless remove property +$XFS_PROPERTY_PROG $TEST_DIR remove "$propname" + +echo set too long name +$XFS_PROPERTY_PROG $TEST_DIR set "$longpropname=$propval" + +echo set too long value +$XFS_PROPERTY_PROG $TEST_DIR set "$propname=$longpropval" + +status=0 +exit diff --git a/tests/xfs/1887.out b/tests/xfs/1887.out new file mode 100644 index 0000000000..2c27206acf --- /dev/null +++ b/tests/xfs/1887.out @@ -0,0 +1,46 @@ +QA output created by 1887 +*** OFFLINE XFS_PROPERTY TEST *** +empty get property +failed to get attr xfs:fakeproperty on inode XXX: No data available +pointless remove property +failed to remove attr xfs:fakeproperty from inode XXX: No data available +list property +set property +fakeproperty=1721943742 +list property +fakeproperty +dump xattrs +get property +fakeproperty=1721943742 +list property +fakeproperty +remove property +pointless remove property +failed to remove attr xfs:fakeproperty from inode XXX: No data available +set too long name +name too long +set too long value +xfs:fakeproperty: value too long +*** ONLINE XFS_PROPERTY TEST *** +empty get property +fakeproperty: No data available +pointless remove property +fakeproperty: No data available +list property +set property +fakeproperty=1721943743 +list property +fakeproperty +dump xattrs +Attribute "xfs:fakeproperty" has a 10 byte value for TEST_DIR +get property +fakeproperty=1721943743 +list property +fakeproperty +remove property +pointless remove property +fakeproperty: No data available +set too long name +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: Invalid argument +set too long value +fakeproperty: Invalid argument diff --git a/tests/xfs/1888 b/tests/xfs/1888 new file mode 100755 index 0000000000..f16a1c0675 --- /dev/null +++ b/tests/xfs/1888 @@ -0,0 +1,66 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Oracle. All Rights Reserved. +# +# FS QA Test 1888 +# +# Functional testing for mkfs applying autofsck fs property. +# +. ./common/preamble +_begin_fstest auto fsproperties + +. ./common/filter +. ./common/attr + +_require_test +_require_xfs_io_command listfsprops # needed for fs props +_require_xfs_db_command attr_get + +_cleanup() + +{ + cd / + rm -r -f $tmp.* + rm -f $dummyfile + rmdir $dummymnt &>/dev/null +} + +dummyfile=$TEST_DIR/$seq.somefile +dummymnt=$TEST_DIR/$seq.mount + +truncate -s 10g $dummyfile +mkdir -p $dummymnt + +filter_inum() +{ + sed -e 's/inode [0-9]*/inode XXX/g' +} + +testme() { + local mkfs_args=('-f') + local value="$1" + test -n "$value" && value="=$value" + + if [ $# -gt 0 ]; then + mkfs_args+=('-m' "autofsck$value") + fi + + echo "testing ${mkfs_args[*]}" >> $seqres.full + + $MKFS_XFS_PROG "${mkfs_args[@]}" $dummyfile >> $seqres.full || \ + _notrun "mkfs.xfs ${mkfs_args[*]} failed?" + + $XFS_DB_PROG -x -c 'path /' -c "attr_get -Z autofsck" $dummyfile 2>&1 | filter_inum +} + +testme '' +testme +testme none +testme check +testme optimize +testme repair +testme 0 +testme 1 + +status=0 +exit diff --git a/tests/xfs/1888.out b/tests/xfs/1888.out new file mode 100644 index 0000000000..73857cabd4 --- /dev/null +++ b/tests/xfs/1888.out @@ -0,0 +1,9 @@ +QA output created by 1888 +autofsck=repair +failed to get attr xfs:autofsck on inode XXX: No data available +autofsck=none +autofsck=check +autofsck=optimize +autofsck=repair +autofsck=none +autofsck=repair diff --git a/tests/xfs/1889 b/tests/xfs/1889 new file mode 100755 index 0000000000..792a67652a --- /dev/null +++ b/tests/xfs/1889 @@ -0,0 +1,63 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Oracle. All Rights Reserved. +# +# FS QA Test 1889 +# +# Functional testing for mkfs applying autofsck fs property and xfs_scrub +# changing its behavior accordingly. Or at least claiming to. +# +. ./common/preamble +_begin_fstest auto fsproperties + +. ./common/filter +. ./common/fuzzy + +_require_test +_require_xfs_io_command listfsprops # needed for fs props +_require_xfs_db_command attr_get +_require_scrub + +_cleanup() + +{ + cd / + rm -r -f $tmp.* + _umount $dummymnt &>/dev/null + rmdir $dummymnt &>/dev/null + rm -f $dummyfile +} + +dummyfile=$TEST_DIR/$seq.somefile +dummymnt=$TEST_DIR/$seq.mount + +truncate -s 10g $dummyfile +mkdir -p $dummymnt + +testme() { + local mkfs_args=('-f' '-m' "$1") + + echo "testing ${mkfs_args[*]}" >> $seqres.full + + $MKFS_XFS_PROG "${mkfs_args[@]}" $dummyfile >> $seqres.full || \ + echo "mkfs.xfs ${mkfs_args[*]} failed?" + + _mount -o loop $dummyfile $dummymnt + XFS_SCRUB_PHASE=7 $XFS_SCRUB_PROG -d -o autofsck $dummymnt 2>&1 | \ + grep autofsck | _filter_test_dir | \ + sed -e 's/\(directive.\).*$/\1/g' + _umount $dummymnt +} + +# We don't test the absence of an autofsck directive because xfs_scrub behaves +# differently depending on whether or not mkfs adds rmapbt/pptrs to the fs. +testme 'autofsck' +testme 'autofsck=none' +testme 'autofsck=check' +testme 'autofsck=optimize' +testme 'autofsck=repair' +testme 'autofsck=0' +testme 'autofsck=1' + +status=0 +exit diff --git a/tests/xfs/1889.out b/tests/xfs/1889.out new file mode 100644 index 0000000000..fd8123b53a --- /dev/null +++ b/tests/xfs/1889.out @@ -0,0 +1,8 @@ +QA output created by 1889 +Info: TEST_DIR/1889.mount: Checking and repairing per autofsck directive. +Info: TEST_DIR/1889.mount: Disabling scrub per autofsck directive. +Info: TEST_DIR/1889.mount: Checking per autofsck directive. +Info: TEST_DIR/1889.mount: Optimizing per autofsck directive. +Info: TEST_DIR/1889.mount: Checking and repairing per autofsck directive. +Info: TEST_DIR/1889.mount: Disabling scrub per autofsck directive. +Info: TEST_DIR/1889.mount: Checking and repairing per autofsck directive. ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/1] xfs: functional testing for filesystem properties 2024-08-06 18:22 ` [PATCH 1/1] xfs: functional testing for " Darrick J. Wong @ 2024-08-07 16:13 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-08-07 16:13 UTC (permalink / raw) To: Darrick J. Wong; +Cc: zlang, fstests, hch, dchinner, linux-xfs Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHSET v30.9 1/3] xfsprogs: filesystem properties @ 2024-07-30 3:18 Darrick J. Wong 2024-07-30 3:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong 0 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-07-30 3:18 UTC (permalink / raw) To: djwong, cem; +Cc: linux-xfs Hi all, It would be very useful if system administrators could set properties for a given xfs filesystem to control its behavior. This we can do easily and extensibly by setting ATTR_ROOT (aka "trusted") extended attributes on the root directory. To prevent this from becoming a weird free for all, let's add some library and tooling support so that sysadmins simply run the xfs_property program to administer these properties. If you're going to start using this code, I strongly recommend pulling from my git trees, which are linked below. This has been running on the djcloud for months with no problems. Enjoy! Comments and questions are, as always, welcome. --D xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=filesystem-properties fstests git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=filesystem-properties --- Commits in this patchset: * libfrog: support editing filesystem property sets * xfs_spaceman: edit filesystem properties * xfs_db: improve getting and setting extended attributes * libxfs: hoist listxattr from xfs_repair * libxfs: pass a transaction context through listxattr * xfs_db: add a command to list xattrs * xfs_property: add a new tool to administer fs properties --- db/attrset.c | 463 ++++++++++++++++++++++++++++++++++++++++++++++- libfrog/Makefile | 7 + libfrog/fsproperties.c | 39 ++++ libfrog/fsproperties.h | 50 +++++ libfrog/fsprops.c | 214 ++++++++++++++++++++++ libfrog/fsprops.h | 34 +++ libxfs/Makefile | 2 libxfs/listxattr.c | 42 ++-- libxfs/listxattr.h | 17 ++ man/man8/xfs_db.8 | 68 +++++++ man/man8/xfs_property.8 | 52 +++++ man/man8/xfs_spaceman.8 | 27 +++ repair/Makefile | 2 repair/listxattr.h | 15 -- repair/pptr.c | 9 + spaceman/Makefile | 7 + spaceman/init.c | 1 spaceman/properties.c | 342 +++++++++++++++++++++++++++++++++++ spaceman/space.h | 1 spaceman/xfs_property | 77 ++++++++ 20 files changed, 1422 insertions(+), 47 deletions(-) create mode 100644 libfrog/fsproperties.c create mode 100644 libfrog/fsproperties.h create mode 100644 libfrog/fsprops.c create mode 100644 libfrog/fsprops.h rename repair/listxattr.c => libxfs/listxattr.c (84%) create mode 100644 libxfs/listxattr.h create mode 100644 man/man8/xfs_property.8 delete mode 100644 repair/listxattr.h create mode 100644 spaceman/properties.c create mode 100755 spaceman/xfs_property ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/7] xfs_db: improve getting and setting extended attributes 2024-07-30 3:18 [PATCHSET v30.9 1/3] xfsprogs: " Darrick J. Wong @ 2024-07-30 3:20 ` Darrick J. Wong 2024-07-30 21:37 ` Christoph Hellwig 0 siblings, 1 reply; 30+ messages in thread From: Darrick J. Wong @ 2024-07-30 3:20 UTC (permalink / raw) To: djwong, cem; +Cc: linux-xfs From: Darrick J. Wong <djwong@kernel.org> Add an attr_get command to retrieve the value of an xattr from a file; and extend the attr_set command to allow passing of string values. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- db/attrset.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++++++- man/man8/xfs_db.8 | 40 ++++++++ 2 files changed, 293 insertions(+), 9 deletions(-) diff --git a/db/attrset.c b/db/attrset.c index 81d530055193..4e7cca450322 100644 --- a/db/attrset.c +++ b/db/attrset.c @@ -17,20 +17,44 @@ #include "inode.h" #include "malloc.h" #include <sys/xattr.h> +#include "libfrog/fsproperties.h" +static int attr_get_f(int argc, char **argv); static int attr_set_f(int argc, char **argv); static int attr_remove_f(int argc, char **argv); +static void attrget_help(void); static void attrset_help(void); +static const cmdinfo_t attr_get_cmd = + { "attr_get", "aget", attr_get_f, 1, -1, 0, + N_("[-r|-s|-u|-p|-Z] name"), + N_("get the named attribute on the current inode"), attrget_help }; static const cmdinfo_t attr_set_cmd = { "attr_set", "aset", attr_set_f, 1, -1, 0, - N_("[-r|-s|-u|-p] [-n] [-R|-C] [-v n] name"), + N_("[-r|-s|-u|-p|-Z] [-n] [-R|-C] [-v n] name"), N_("set the named attribute on the current inode"), attrset_help }; static const cmdinfo_t attr_remove_cmd = { "attr_remove", "aremove", attr_remove_f, 1, -1, 0, - N_("[-r|-s|-u|-p] [-n] name"), + N_("[-r|-s|-u|-p|-Z] [-n] name"), N_("remove the named attribute from the current inode"), attrset_help }; +static void +attrget_help(void) +{ + dbprintf(_( +"\n" +" The attr_get command provide interfaces for retrieving the values of extended\n" +" attributes of a file. This command requires attribute names to be specified.\n" +" There are 4 namespace flags:\n" +" -r -- 'root'\n" +" -u -- 'user' (default)\n" +" -s -- 'secure'\n" +" -p -- 'parent'\n" +" -f -- 'fs-verity'\n" +" -Z -- fs property\n" +"\n")); +} + static void attrset_help(void) { @@ -45,10 +69,15 @@ attrset_help(void) " -u -- 'user' (default)\n" " -s -- 'secure'\n" " -p -- 'parent'\n" +" -Z -- fs property\n" "\n" " For attr_set, these options further define the type of set operation:\n" " -C -- 'create' - create attribute, fail if it already exists\n" " -R -- 'replace' - replace attribute, fail if it does not exist\n" +"\n" +" If the attribute value is a string, it can be specified after the\n" +" attribute name.\n" +"\n" " The backward compatibility mode 'noattr2' can be emulated (-n) also.\n" "\n")); } @@ -59,6 +88,7 @@ attrset_init(void) if (!expert_mode) return; + add_command(&attr_get_cmd); add_command(&attr_set_cmd); add_command(&attr_remove_cmd); } @@ -106,6 +136,57 @@ get_buf_from_file( LIBXFS_ATTR_ROOT | \ LIBXFS_ATTR_PARENT) +static bool +adjust_fsprop_attr_name( + struct xfs_da_args *args, + bool *free_name) +{ + const char *o = args->name; + char *p; + int ret; + + if ((args->attr_filter & LIBXFS_ATTR_NS) != LIBXFS_ATTR_ROOT) { + dbprintf(_("fs properties must be ATTR_ROOT\n")); + return false; + } + + ret = fsprop_name_to_attr_name(o, &p); + if (ret < 0) { + dbprintf(_("could not allocate fs property name string\n")); + return false; + } + args->namelen = ret; + args->name = p; + + if (*free_name) + free((void *)o); + *free_name = true; + + if (args->namelen > MAXNAMELEN) { + dbprintf(_("%s: name too long\n"), args->name); + return false; + } + + if (args->valuelen > ATTR_MAX_VALUELEN) { + dbprintf(_("%s: value too long\n"), args->name); + return false; + } + + return true; +} + +static void +print_fsprop( + struct xfs_da_args *args) +{ + const char *p = attr_name_to_fsprop_name(args->name); + + if (p) + printf("%s=%.*s\n", p, args->valuelen, (char *)args->value); + else + fprintf(stderr, _("%s: not a fs property?\n"), args->name); +} + static int attr_set_f( int argc, @@ -119,7 +200,9 @@ attr_set_f( char *sp; char *name_from_file = NULL; char *value_from_file = NULL; + bool free_name = false; enum xfs_attr_update op = XFS_ATTRUPDATE_UPSERT; + bool fsprop = false; int c; int error; @@ -132,9 +215,12 @@ attr_set_f( return 0; } - while ((c = getopt(argc, argv, "ruspCRnN:v:V:")) != EOF) { + while ((c = getopt(argc, argv, "ruspCRnN:v:V:Z")) != EOF) { switch (c) { /* namespaces */ + case 'Z': + fsprop = true; + fallthrough; case 'r': args.attr_filter &= ~LIBXFS_ATTR_NS; args.attr_filter |= LIBXFS_ATTR_ROOT; @@ -213,9 +299,10 @@ attr_set_f( if (!args.name) return 0; + free_name = true; args.namelen = namelen; } else { - if (optind != argc - 1) { + if (optind != argc - 1 && optind != argc - 2) { dbprintf(_("too few options for attr_set (no name given)\n")); return 0; } @@ -250,6 +337,25 @@ attr_set_f( goto out; } memset(args.value, 'v', args.valuelen); + } else if (optind == argc - 2) { + args.valuelen = strlen(argv[optind + 1]); + args.value = strdup(argv[optind + 1]); + if (!args.value) { + dbprintf(_("cannot allocate buffer (%d)\n"), + args.valuelen); + goto out; + } + } + + if (fsprop) { + if (!fsprop_validate(args.name, args.value)) { + dbprintf(_("%s: invalid value \"%s\"\n"), + args.name, args.value); + goto out; + } + + if (!adjust_fsprop_attr_name(&args, &free_name)) + goto out; } if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &args.dp)) { @@ -269,6 +375,9 @@ attr_set_f( goto out; } + if (fsprop) + print_fsprop(&args); + /* refresh with updated inode contents */ set_cur_inode(iocur_top->ino); @@ -277,7 +386,7 @@ attr_set_f( libxfs_irele(args.dp); if (args.value) free(args.value); - if (name_from_file) + if (free_name) free((void *)args.name); return 0; } @@ -293,6 +402,8 @@ attr_remove_f( .op_flags = XFS_DA_OP_OKNOENT, }; char *name_from_file = NULL; + bool free_name = false; + bool fsprop = false; int c; int error; @@ -305,9 +416,12 @@ attr_remove_f( return 0; } - while ((c = getopt(argc, argv, "ruspnN:")) != EOF) { + while ((c = getopt(argc, argv, "ruspnN:Z")) != EOF) { switch (c) { /* namespaces */ + case 'Z': + fsprop = true; + fallthrough; case 'r': args.attr_filter &= ~LIBXFS_ATTR_NS; args.attr_filter |= LIBXFS_ATTR_ROOT; @@ -354,6 +468,7 @@ attr_remove_f( if (!args.name) return 0; + free_name = true; args.namelen = namelen; } else { if (optind != argc - 1) { @@ -374,6 +489,9 @@ attr_remove_f( } } + if (fsprop && !adjust_fsprop_attr_name(&args, &free_name)) + goto out; + if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &args.dp)) { dbprintf(_("failed to iget inode %llu\n"), (unsigned long long)iocur_top->ino); @@ -398,7 +516,137 @@ attr_remove_f( out: if (args.dp) libxfs_irele(args.dp); - if (name_from_file) + if (free_name) + free((void *)args.name); + return 0; +} + +static int +attr_get_f( + int argc, + char **argv) +{ + struct xfs_da_args args = { + .geo = mp->m_attr_geo, + .whichfork = XFS_ATTR_FORK, + .op_flags = XFS_DA_OP_OKNOENT, + }; + char *name_from_file = NULL; + bool free_name = false; + bool fsprop = false; + int c; + int error; + + if (cur_typ == NULL) { + dbprintf(_("no current type\n")); + return 0; + } + if (cur_typ->typnm != TYP_INODE) { + dbprintf(_("current type is not inode\n")); + return 0; + } + + while ((c = getopt(argc, argv, "ruspN:Z")) != EOF) { + switch (c) { + /* namespaces */ + case 'Z': + fsprop = true; + fallthrough; + case 'r': + args.attr_filter &= ~LIBXFS_ATTR_NS; + args.attr_filter |= LIBXFS_ATTR_ROOT; + break; + case 'u': + args.attr_filter &= ~LIBXFS_ATTR_NS; + break; + case 's': + args.attr_filter &= ~LIBXFS_ATTR_NS; + args.attr_filter |= LIBXFS_ATTR_SECURE; + break; + case 'p': + args.attr_filter &= ~LIBXFS_ATTR_NS; + args.attr_filter |= XFS_ATTR_PARENT; + break; + + case 'N': + name_from_file = optarg; + break; + default: + dbprintf(_("bad option for attr_get command\n")); + return 0; + } + } + + if (name_from_file) { + int namelen; + + if (optind != argc) { + dbprintf(_("too many options for attr_get (no name needed)\n")); + return 0; + } + + args.name = get_buf_from_file(name_from_file, MAXNAMELEN, + &namelen); + if (!args.name) + return 0; + + free_name = true; + args.namelen = namelen; + } else { + if (optind != argc - 1) { + dbprintf(_("too few options for attr_get (no name given)\n")); + return 0; + } + + args.name = (const unsigned char *)argv[optind]; + if (!args.name) { + dbprintf(_("invalid name\n")); + return 0; + } + + args.namelen = strlen(argv[optind]); + if (args.namelen >= MAXNAMELEN) { + dbprintf(_("name too long\n")); + goto out; + } + } + + if (libxfs_iget(mp, NULL, iocur_top->ino, 0, &args.dp)) { + dbprintf(_("failed to iget inode %llu\n"), + (unsigned long long)iocur_top->ino); + goto out; + } + + if (fsprop && !adjust_fsprop_attr_name(&args, &free_name)) + goto out; + + args.owner = iocur_top->ino; + libxfs_attr_sethash(&args); + + /* + * Look up attr value with a maximally long length and a null buffer + * to return the value and the correct length. + */ + args.valuelen = XATTR_SIZE_MAX; + error = -libxfs_attr_get(&args); + if (error) { + dbprintf(_("failed to get attr %s on inode %llu: %s\n"), + args.name, (unsigned long long)iocur_top->ino, + strerror(error)); + goto out; + } + + if (fsprop) + print_fsprop(&args); + else + printf("%.*s\n", args.valuelen, (char *)args.value); + +out: + if (args.dp) + libxfs_irele(args.dp); + if (args.value) + free(args.value); + if (free_name) free((void *)args.name); return 0; } diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8 index 9f6fea5748d4..f0865b2df4ec 100644 --- a/man/man8/xfs_db.8 +++ b/man/man8/xfs_db.8 @@ -184,7 +184,35 @@ Displays the length, free block count, per-AG reservation size, and per-AG reservation usage for a given AG. If no argument is given, display information for all AGs. .TP -.BI "attr_remove [\-p|\-r|\-u|\-s] [\-n] [\-N " namefile "|" name "] " +.BI "attr_get [\-p|\-r|\-u|\-s|\-Z] [\-N " namefile "|" name "] " +Print the value of the specified extended attribute from the current file. +.RS 1.0i +.TP 0.4i +.B \-p +Sets the attribute in the parent namespace. +Only one namespace option can be specified. +.TP +.B \-r +Sets the attribute in the root namespace. +Only one namespace option can be specified. +.TP +.B \-u +Sets the attribute in the user namespace. +Only one namespace option can be specified. +.TP +.B \-s +Sets the attribute in the secure namespace. +Only one namespace option can be specified. +.TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP +.B \-N +Read the name from this file. +.RE +.TP +.BI "attr_remove [\-p|\-r|\-u|\-s|\-Z] [\-n] [\-N " namefile "|" name "] " Remove the specified extended attribute from the current file. .RS 1.0i .TP 0.4i @@ -204,6 +232,10 @@ Only one namespace option can be specified. Sets the attribute in the secure namespace. Only one namespace option can be specified. .TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP .B \-N Read the name from this file. .TP @@ -211,7 +243,7 @@ Read the name from this file. Do not enable 'noattr2' mode on V4 filesystems. .RE .TP -.BI "attr_set [\-p\-r|\-u|\-s] [\-n] [\-R|\-C] [\-v " valuelen "|\-V " valuefile "] [\-N " namefile "|" name "] " +.BI "attr_set [\-p\-r|\-u|\-s|\-Z] [\-n] [\-R|\-C] [\-v " valuelen "|\-V " valuefile "] [\-N " namefile "|" name "] [" value "]" Sets an extended attribute on the current file with the given name. .RS 1.0i .TP 0.4i @@ -231,6 +263,10 @@ Only one namespace option can be specified. Sets the attribute in the secure namespace. Only one namespace option can be specified. .TP +.B \-Z +Sets a filesystem property in the root namespace. +Only one namespace option can be specified. +.TP .B \-N Read the name from this file. .TP ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] xfs_db: improve getting and setting extended attributes 2024-07-30 3:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong @ 2024-07-30 21:37 ` Christoph Hellwig 0 siblings, 0 replies; 30+ messages in thread From: Christoph Hellwig @ 2024-07-30 21:37 UTC (permalink / raw) To: Darrick J. Wong; +Cc: cem, linux-xfs > +static int attr_get_f(int argc, char **argv); > static int attr_set_f(int argc, char **argv); > static int attr_remove_f(int argc, char **argv); > +static void attrget_help(void); > static void attrset_help(void); I always hate these forward declarations, but I guess it matches the existing code.. Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2024-08-07 16:43 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-06 18:14 [PATCHBOMB v30.10] xfsprogs-6.10: filesystem properties Darrick J. Wong 2024-08-06 18:18 ` [PATCHSET v30.10 1/3] xfsprogs: " Darrick J. Wong 2024-08-06 18:19 ` [PATCH 1/7] libfrog: support editing filesystem property sets Darrick J. Wong 2024-08-06 18:19 ` [PATCH 2/7] xfs_io: edit filesystem properties Darrick J. Wong 2024-08-07 16:08 ` Christoph Hellwig 2024-08-06 18:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong 2024-08-07 16:10 ` Christoph Hellwig 2024-08-07 16:43 ` Darrick J. Wong 2024-08-06 18:20 ` [PATCH 4/7] libxfs: hoist listxattr from xfs_repair Darrick J. Wong 2024-08-06 18:20 ` [PATCH 5/7] libxfs: pass a transaction context through listxattr Darrick J. Wong 2024-08-06 18:20 ` [PATCH 6/7] xfs_db: add a command to list xattrs Darrick J. Wong 2024-08-06 18:21 ` [PATCH 7/7] xfs_property: add a new tool to administer fs properties Darrick J. Wong 2024-08-07 16:10 ` Christoph Hellwig 2024-08-06 18:18 ` [PATCHSET v30.10 2/3] xfs_scrub: admin control of automatic fsck Darrick J. Wong 2024-08-06 18:21 ` [PATCH 1/4] libfrog: define a autofsck filesystem property Darrick J. Wong 2024-08-07 16:11 ` Christoph Hellwig 2024-08-06 18:21 ` [PATCH 2/4] xfs_scrub: allow sysadmin to control background scrubs Darrick J. Wong 2024-08-07 16:12 ` Christoph Hellwig 2024-08-06 18:21 ` [PATCH 3/4] xfs_scrub: use the autofsck fsproperty to select mode Darrick J. Wong 2024-08-07 16:12 ` Christoph Hellwig 2024-08-06 18:22 ` [PATCH 4/4] mkfs: set autofsck filesystem property Darrick J. Wong 2024-08-07 16:13 ` Christoph Hellwig 2024-08-06 18:19 ` [PATCHSET v30.10 3/3] debian: enable xfs_scrub_all by default Darrick J. Wong 2024-08-06 18:22 ` [PATCH 1/1] debian: enable xfs_scrub_all systemd timer services " Darrick J. Wong 2024-08-07 16:13 ` Christoph Hellwig 2024-08-06 18:19 ` [PATCHSET v30.10] fstests: xfs filesystem properties Darrick J. Wong 2024-08-06 18:22 ` [PATCH 1/1] xfs: functional testing for " Darrick J. Wong 2024-08-07 16:13 ` Christoph Hellwig -- strict thread matches above, loose matches on Subject: below -- 2024-07-30 3:18 [PATCHSET v30.9 1/3] xfsprogs: " Darrick J. Wong 2024-07-30 3:20 ` [PATCH 3/7] xfs_db: improve getting and setting extended attributes Darrick J. Wong 2024-07-30 21:37 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox