[RFC PATCHSET] xfsprogs: filesystem properties

[RFC PATCHSET] xfsprogs: filesystem properties

[Darrick J. Wong]

Hi all,

After last week's discussion about how to allow sysadmins to opt in or
out of autonomous self healing XFS[1], I now have an RFC patchset 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:self_healing" as the
property that controls the amount of autonomous self healing.  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 self_healing /dev/sda

# xfs_property /dev/sda get self_healing
repair

# mount /dev/sda /mnt

# xfs_property /mnt set self_healing=check

# xfs_scrub -o fsprops_advise /mnt
Info: /mnt: Checking per self_healing directive.

--D

[1] https://lore.kernel.org/linux-xfs/20240724213852.GA612460@frogsfrogsfrogs/

[PATCHSET v30.9 1/3] xfsprogs: filesystem properties

[Darrick J. Wong]

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

[PATCH 1/7] libfrog: support editing filesystem property sets

[Darrick J. Wong]

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>
---
 libfrog/Makefile       |    7 ++
 libfrog/fsproperties.c |   39 +++++++++
 libfrog/fsproperties.h |   50 +++++++++++
 libfrog/fsprops.c      |  214 ++++++++++++++++++++++++++++++++++++++++++++++++
 libfrog/fsprops.h      |   34 ++++++++
 5 files changed, 344 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 0b5b23893a13..acddc894ee93 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 000000000000..c317d15c1de0
--- /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 000000000000..6dee8259a437
--- /dev/null
+++ b/libfrog/fsproperties.h
@@ -0,0 +1,50 @@
+/* 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 XFS_FSPROPS_NS		"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 XFS_FSPROPS_PREFIX	"xfs:"
+
+static inline int
+fsprop_name_to_attr_name(
+	const char		*prop_name,
+	char			**attr_name)
+{
+	return asprintf(attr_name, XFS_FSPROPS_PREFIX "%s", prop_name);
+}
+
+static inline const char *
+attr_name_to_fsprop_name(
+	const char		*attr_name)
+{
+	const size_t		bytes = sizeof(XFS_FSPROPS_PREFIX) - 1;
+	unsigned int		i;
+
+	for (i = 0; i < bytes; i++) {
+		if (attr_name[i] == 0)
+			return NULL;
+	}
+
+	if (memcmp(attr_name, XFS_FSPROPS_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 000000000000..d6d143efe027
--- /dev/null
+++ b/libfrog/fsprops.c
@@ -0,0 +1,214 @@
+// 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			fd;
+	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;
+
+	fd = open(fs_path->fs_dir, O_RDONLY | O_DIRECTORY);
+	if (fd < 0)
+		return -1;
+
+	ret = fstat(fd, &sb);
+	if (ret)
+		goto out_fd;
+
+	if (sb.st_ino != bulkstat.bs_ino) {
+		errno = ESRMNT;
+		ret = -1;
+		goto out_fd;
+	}
+
+	ret = fd_to_handle(fd, &fph->hanp, &fph->hlen);
+	if (ret)
+		goto out_fd;
+
+out_fd:
+	close(fd);
+	return ret;
+}
+
+/* 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 000000000000..9276f2425191
--- /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__ */

Re: [PATCH 1/7] libfrog: support editing filesystem property sets

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/7] xfs_spaceman: edit filesystem properties

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Add some new subcommands to xfs_spaceman so that we can examine
filesystem properties.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 man/man8/xfs_spaceman.8 |   27 ++++
 spaceman/Makefile       |    4 +
 spaceman/init.c         |    1 
 spaceman/properties.c   |  342 +++++++++++++++++++++++++++++++++++++++++++++++
 spaceman/space.h        |    1 
 5 files changed, 375 insertions(+)
 create mode 100644 spaceman/properties.c


diff --git a/man/man8/xfs_spaceman.8 b/man/man8/xfs_spaceman.8
index 0d299132a788..4615774de59e 100644
--- a/man/man8/xfs_spaceman.8
+++ b/man/man8/xfs_spaceman.8
@@ -217,3 +217,30 @@ Do not trim free space extents shorter than this length.
 Units can be appended to this argument.
 .PD
 .RE
+
+.SH FILESYSTEM PROPERTIES
+If the opened file is the root directory of a filesystem, the following
+commands can be used to read and write filesystem properties.
+These properties allow system administrators to express their preferences for
+the filesystem in question.
+.TP
+.BI "getfsprops name [ " names "... ]"
+Retrieve the values of the given filesystem properties.
+.TP
+.BI "listfsprops"
+List all filesystem properties that have been stored in the filesystem.
+.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.
+
+.RE
+Currently supported filesystem properties are:
+.TP
+.B self_healing
+See the
+.BR xfs_scrub (8)
+manual for more information.
+.RE
diff --git a/spaceman/Makefile b/spaceman/Makefile
index 358db9edf5cb..2688b37c770d 100644
--- a/spaceman/Makefile
+++ b/spaceman/Makefile
@@ -30,6 +30,10 @@ ifeq ($(HAVE_GETFSMAP),yes)
 CFILES += freesp.c
 endif
 
+ifeq ($(HAVE_LIBATTR),yes)
+CFILES += properties.c
+endif
+
 default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
diff --git a/spaceman/init.c b/spaceman/init.c
index cf1ff3cbb0ee..aff666cdb670 100644
--- a/spaceman/init.c
+++ b/spaceman/init.c
@@ -35,6 +35,7 @@ init_commands(void)
 	trim_init();
 	freesp_init();
 	health_init();
+	fsprops_init();
 }
 
 static int
diff --git a/spaceman/properties.c b/spaceman/properties.c
new file mode 100644
index 000000000000..dbe628c1184a
--- /dev/null
+++ b/spaceman/properties.c
@@ -0,0 +1,342 @@
+// 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 "space.h"
+#include "libfrog/fsprops.h"
+#include "libfrog/fsproperties.h"
+
+#include <attr/attributes.h>
+
+static void
+listfsprops_help(void)
+{
+	printf(_(
+"Print the names of the filesystem properties stored in this filesystem.\n"
+"\n"));
+}
+
+static int
+print_fsprop(
+	struct fsprops_handle	*fph,
+	const char		*name,
+	size_t			unused,
+	void			*priv)
+{
+	bool			*print_values = priv;
+	char			valuebuf[ATTR_MAX_VALUELEN];
+	size_t			valuelen = ATTR_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 = fsprops_open_handle(&file->xfd, &file->fs_path, &fph);
+	if (ret) {
+		if (errno == ESRMNT)
+			fprintf(stderr,
+ _("%s: Cannot find alleged XFS mount point %s.\n"),
+					file->name, file->fs_path.fs_dir);
+		else
+			perror(file->name);
+		exitcode = 1;
+		return 0;
+	}
+
+	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_FLAG_ONESHOT,
+	.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 = fsprops_open_handle(&file->xfd, &file->fs_path, &fph);
+	if (ret) {
+		if (errno == ESRMNT)
+			fprintf(stderr,
+ _("%s: Cannot find alleged XFS mount point %s.\n"),
+					file->name, file->fs_path.fs_dir);
+		else
+			perror(file->name);
+		exitcode = 1;
+		return 0;
+	}
+
+	for (c = optind; c < argc; c++) {
+		char		valuebuf[ATTR_MAX_VALUELEN];
+		size_t		valuelen = ATTR_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_FLAG_ONESHOT,
+	.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 = fsprops_open_handle(&file->xfd, &file->fs_path, &fph);
+	if (ret) {
+		if (errno == ESRMNT)
+			fprintf(stderr,
+ _("%s: Cannot find alleged XFS mount point %s.\n"),
+					file->name, file->fs_path.fs_dir);
+		else
+			perror(file->name);
+		exitcode = 1;
+		return 0;
+	}
+
+	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_FLAG_ONESHOT,
+	.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 = fsprops_open_handle(&file->xfd, &file->fs_path, &fph);
+	if (ret) {
+		if (errno == ESRMNT)
+			fprintf(stderr,
+ _("%s: Cannot find alleged XFS mount point %s.\n"),
+					file->name, file->fs_path.fs_dir);
+		else
+			perror(file->name);
+		exitcode = 1;
+		return 0;
+	}
+
+	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_FLAG_ONESHOT,
+	.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/spaceman/space.h b/spaceman/space.h
index 28fa35a30479..c4beb5f489ff 100644
--- a/spaceman/space.h
+++ b/spaceman/space.h
@@ -36,5 +36,6 @@ extern void	freesp_init(void);
 #endif
 extern void	info_init(void);
 extern void	health_init(void);
+void		fsprops_init(void);
 
 #endif /* XFS_SPACEMAN_SPACE_H_ */

Re: [PATCH 2/7] xfs_spaceman: edit filesystem properties

[Christoph Hellwig]

Is this really a xfs_spaceman thingy?  The code itself looks fine,
but the association with space management is kinda weird.

Re: [PATCH 2/7] xfs_spaceman: edit filesystem properties

[Darrick J. Wong]

On Tue, Jul 30, 2024 at 02:41:41PM -0700, Christoph Hellwig wrote:
> Is this really a xfs_spaceman thingy?  The code itself looks fine,
> but the association with space management is kinda weird.

I dunno.  It could just as easily go into xfs_io I suppose; the tiny
advantage of putting it in spaceman is that spaceman grabs the
fsgeometry structure on file open so we don't have to do that again.

--D

Re: [PATCH 2/7] xfs_spaceman: edit filesystem properties

[Christoph Hellwig]

On Tue, Jul 30, 2024 at 03:37:25PM -0700, Darrick J. Wong wrote:
> On Tue, Jul 30, 2024 at 02:41:41PM -0700, Christoph Hellwig wrote:
> > Is this really a xfs_spaceman thingy?  The code itself looks fine,
> > but the association with space management is kinda weird.
> 
> I dunno.  It could just as easily go into xfs_io I suppose; the tiny
> advantage of putting it in spaceman is that spaceman grabs the
> fsgeometry structure on file open so we don't have to do that again.

For the online changes xfs_io seems ok, and for the offline ones xfs_db
seems like a perfevt fit anyway.

Re: [PATCH 2/7] xfs_spaceman: edit filesystem properties

[Dave Chinner]

On Wed, Jul 31, 2024 at 08:41:58AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 30, 2024 at 03:37:25PM -0700, Darrick J. Wong wrote:
> > On Tue, Jul 30, 2024 at 02:41:41PM -0700, Christoph Hellwig wrote:
> > > Is this really a xfs_spaceman thingy?  The code itself looks fine,
> > > but the association with space management is kinda weird.
> > 
> > I dunno.  It could just as easily go into xfs_io I suppose; the tiny
> > advantage of putting it in spaceman is that spaceman grabs the
> > fsgeometry structure on file open so we don't have to do that again.
> 
> For the online changes xfs_io seems ok, and for the offline ones xfs_db
> seems like a perfevt fit anyway.

If fsprops can be managed both online and offline,
then xfs_admin is probably the right user facing interface
to document.  i.e. We already have xfs_admin vectoring between
xfs_io when the filesystem is online and xfs_db when the filesystem
is offline to do things like change UUIDs and labels. This would
make setting fsprops somewhat simpler for admins and scripts as they
then only have to learn/code one mechanism instead of two...

-Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 3/7] xfs_db: improve getting and setting extended attributes

[Darrick J. Wong]

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

Re: [PATCH 3/7] xfs_db: improve getting and setting extended attributes

[Christoph Hellwig]

> +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>

[PATCH 4/7] libxfs: hoist listxattr from xfs_repair

[Darrick J. Wong]

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>
---
 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 4e8f9a135818..2f2791cae587 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 2af77b7b2195..bedaca678439 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 2d26fce0f323..cddd96af7c0c 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 e7445d53e918..a36a95e353a5 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 8ec6a51d2c3d..cc66e637217f 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"

Re: [PATCH 4/7] libxfs: hoist listxattr from xfs_repair

[Christoph Hellwig]

On Mon, Jul 29, 2024 at 08:20:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Hoist the listxattr code from xfs_repair so that we can use it in
> xfs_db.

I guess there isn't much of a point in sharing with the kernel listattr
code?

But maybe that is for later, for now this trivial move looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/7] libxfs: hoist listxattr from xfs_repair

[Darrick J. Wong]

On Tue, Jul 30, 2024 at 02:38:08PM -0700, Christoph Hellwig wrote:
> On Mon, Jul 29, 2024 at 08:20:45PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Hoist the listxattr code from xfs_repair so that we can use it in
> > xfs_db.
> 
> I guess there isn't much of a point in sharing with the kernel listattr
> code?
> 
> But maybe that is for later, for now this trivial move looks good:

I think we could do it.  The major difference between the two is the
prefix we use for the dabno "have I seen this block before?" bitmap
functions.  But that and the scrub/listxattr.c code would both have to
be lifted to libxfs, and as long as we're doing that we might as well
clean up the userspace bitmap implementation.

(IOWs that's a somewhat lengthy cleanup for userspace either for after
we get the rt modernization stuff merged or if we get really bored
sitting in meetings.)

> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thank you!

--D

[PATCH 5/7] libxfs: pass a transaction context through listxattr

[Darrick J. Wong]

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>
---
 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 bedaca678439..34205682f022 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 cddd96af7c0c..933e0f529ec4 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 cc66e637217f..ee29e47a87bd 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,

Re: [PATCH 5/7] libxfs: pass a transaction context through listxattr

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 6/7] xfs_db: add a command to list xattrs

[Darrick J. Wong]

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>
---
 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 4e7cca450322..9a7074069e27 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)
 {
@@ -88,6 +114,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 f0865b2df4ec..291ec1c5827b 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

Re: [PATCH 6/7] xfs_db: add a command to list xattrs

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 7/7] xfs_property: add a new tool to administer fs properties

[Darrick J. Wong]

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>
---
 man/man8/xfs_property.8 |   52 ++++++++++++++++++++++++++++++++
 spaceman/Makefile       |    3 +-
 spaceman/xfs_property   |   77 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 man/man8/xfs_property.8
 create mode 100755 spaceman/xfs_property


diff --git a/man/man8/xfs_property.8 b/man/man8/xfs_property.8
new file mode 100644
index 000000000000..63245331bd86
--- /dev/null
+++ b/man/man8/xfs_property.8
@@ -0,0 +1,52 @@
+.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.
+
+.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.
diff --git a/spaceman/Makefile b/spaceman/Makefile
index 2688b37c770d..e914b921de8b 100644
--- a/spaceman/Makefile
+++ b/spaceman/Makefile
@@ -16,7 +16,7 @@ CFILES = \
 	init.c \
 	prealloc.c \
 	trim.c
-LSRCFILES = xfs_info.sh
+LSRCFILES = xfs_info.sh xfs_property
 
 LLDLIBS = $(LIBHANDLE) $(LIBXCMD) $(LIBFROG)
 LTDEPENDENCIES = $(LIBHANDLE) $(LIBXCMD) $(LIBFROG)
@@ -42,6 +42,7 @@ install: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 xfs_info.sh $(PKG_SBIN_DIR)/xfs_info
+	$(INSTALL) -m 755 xfs_property $(PKG_SBIN_DIR)/xfs_property
 install-dev:
 
 -include .dep
diff --git a/spaceman/xfs_property b/spaceman/xfs_property
new file mode 100755
index 000000000000..57185faa38db
--- /dev/null
+++ b/spaceman/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_spaceman -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=()
+spaceman_args=()
+
+case "$subcommand" in
+"list")
+	vparam=
+	if [ $# -eq 1 ] && [ "$1" = "-v" ]; then
+		vparam=" -v"
+	fi
+	db_args+=('-c' "attr_list -Z${vparam}")
+	spaceman_args+=('-c' "listfsprops${vparam}")
+	;;
+"get"|"remove"|"set")
+	for arg in "$@"; do
+		db_args+=('-c' "attr_${subcommand} -Z ${arg/=/ }")
+		spaceman_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_spaceman -p xfs_property "${spaceman_args[@]}" "${mountpt}"
+else
+	exec xfs_db -p xfs_property -x -c 'path /' "${db_args[@]}" "${target}"
+fi

Re: [PATCH 7/7] xfs_property: add a new tool to administer fs properties

[Christoph Hellwig]

On Mon, Jul 29, 2024 at 08:21:32PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a tool to list, get, set, and remove filesystem properties.

Ok, so this wraps spaceman.  Can we make the property manipulation only
available through this interface to cause less confusion?

Re: [PATCH 7/7] xfs_property: add a new tool to administer fs properties

[Darrick J. Wong]

On Tue, Jul 30, 2024 at 02:43:30PM -0700, Christoph Hellwig wrote:
> On Mon, Jul 29, 2024 at 08:21:32PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a tool to list, get, set, and remove filesystem properties.
> 
> Ok, so this wraps spaceman.  Can we make the property manipulation only
> available through this interface to cause less confusion?

I think you're asking if I could make xfs_spaceman expose the
{list,get,set}fsprops commands and db expose the -Z flag to
attr_{get,set,list} when someone starts them up with -p xfs_property ?

If so, then yes, I can do that.

--D

Re: [PATCH 7/7] xfs_property: add a new tool to administer fs properties

[Christoph Hellwig]

On Tue, Jul 30, 2024 at 03:28:15PM -0700, Darrick J. Wong wrote:
> I think you're asking if I could make xfs_spaceman expose the
> {list,get,set}fsprops commands and db expose the -Z flag to
> attr_{get,set,list} when someone starts them up with -p xfs_property ?

I was mostly concerned about spaceman.  I don't think it actually is so
bad, just a bit confusing.  But if we do xfs_io and xfs_db that's not
really an issue anyway.

Re: [PATCH 7/7] xfs_property: add a new tool to administer fs properties

[Darrick J. Wong]

On Wed, Jul 31, 2024 at 08:42:41AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 30, 2024 at 03:28:15PM -0700, Darrick J. Wong wrote:
> > I think you're asking if I could make xfs_spaceman expose the
> > {list,get,set}fsprops commands and db expose the -Z flag to
> > attr_{get,set,list} when someone starts them up with -p xfs_property ?
> 
> I was mostly concerned about spaceman.  I don't think it actually is so
> bad, just a bit confusing.  But if we do xfs_io and xfs_db that's not
> really an issue anyway.

Fair 'enough.  I'll see about switching spaceman -> io today.

--D

Re: [PATCH 7/7] xfs_property: add a new tool to administer fs properties

[Dave Chinner]

On Mon, Jul 29, 2024 at 08:21:32PM -0700, Darrick J. Wong wrote:
> 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>
> ---
>  man/man8/xfs_property.8 |   52 ++++++++++++++++++++++++++++++++
>  spaceman/Makefile       |    3 +-
>  spaceman/xfs_property   |   77 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 man/man8/xfs_property.8
>  create mode 100755 spaceman/xfs_property

Ah, there's the admin wrapper. :)

I should have read the whole patch set before commenting.

> diff --git a/man/man8/xfs_property.8 b/man/man8/xfs_property.8
> new file mode 100644
> index 000000000000..63245331bd86
> --- /dev/null
> +++ b/man/man8/xfs_property.8
> @@ -0,0 +1,52 @@
> +.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.
> +
> +.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.

What does the -V option do? It's documented as existing, but not
explained.

Otherwise looks fine.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCHSET v30.9 2/3] xfs_scrub: control of autonomous self healing

[Darrick J. Wong]

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.

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=self-healing
---
Commits in this patchset:
 * libfrog: define a self_healing filesystem property
 * xfs_scrub: allow sysadmin to control background scrubs
 * mkfs: set self_healing property
---
 libfrog/fsproperties.c |   38 +++++++++++++++
 libfrog/fsproperties.h |   13 +++++
 man/man8/mkfs.xfs.8.in |    6 ++
 man/man8/xfs_scrub.8   |   44 +++++++++++++++++
 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/phase1.c         |   81 ++++++++++++++++++++++++++++++++
 scrub/xfs_scrub.c      |   14 ++++++
 scrub/xfs_scrub.h      |    7 +++
 14 files changed, 330 insertions(+), 1 deletion(-)

[PATCH 1/3] libfrog: define a self_healing filesystem property

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

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 c317d15c1de0..4ccd0edd8453 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))
 
+/* Self-healing fs property */
+
+static const char *fsprop_self_healing_values[] = {
+	[FSPROP_SELFHEAL_UNSET]		= NULL,
+	[FSPROP_SELFHEAL_NONE]		= "none",
+	[FSPROP_SELFHEAL_CHECK]		= "check",
+	[FSPROP_SELFHEAL_OPTIMIZE]	= "optimize",
+	[FSPROP_SELFHEAL_REPAIR]	= "repair",
+};
+
+/* Convert the self_healing property enum to a string. */
+const char *
+fsprop_write_self_healing(
+	enum fsprop_self_healing	x)
+{
+	if (x <= FSPROP_SELFHEAL_UNSET ||
+	    x >= ARRAY_SIZE(fsprop_self_healing_values))
+		return NULL;
+	return fsprop_self_healing_values[x];
+}
+
+/*
+ * Turn a self_healing value string into an enumerated value, or _UNSET if it's
+ * not recognized.
+ */
+enum fsprop_self_healing
+fsprop_read_self_healing(
+	const char	*value)
+{
+	int ret = fsprops_lookup(fsprop_self_healing_values, value);
+	if (ret < 0)
+		return FSPROP_SELFHEAL_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_SELF_HEALING_NAME))
+		return fsprops_lookup(fsprop_self_healing_values, value) >= 0;
+
 	return true;
 }
diff --git a/libfrog/fsproperties.h b/libfrog/fsproperties.h
index 6dee8259a437..7004d339715a 100644
--- a/libfrog/fsproperties.h
+++ b/libfrog/fsproperties.h
@@ -47,4 +47,17 @@ bool fsprop_validate(const char *name, const char *value);
 
 /* Specific Filesystem Properties */
 
+#define FSPROP_SELF_HEALING_NAME	"self_healing"
+
+enum fsprop_self_healing {
+	FSPROP_SELFHEAL_UNSET = 0,	/* do not set property */
+	FSPROP_SELFHEAL_NONE,		/* no background scrubs */
+	FSPROP_SELFHEAL_CHECK,		/* allow only background checking */
+	FSPROP_SELFHEAL_OPTIMIZE,	/* allow background optimization */
+	FSPROP_SELFHEAL_REPAIR,		/* allow background repair & optimization */
+};
+
+const char *fsprop_write_self_healing(enum fsprop_self_healing x);
+enum fsprop_self_healing fsprop_read_self_healing(const char *value);
+
 #endif /* __LIBFROG_FSPROPERTIES_H__ */

Re: [PATCH 1/3] libfrog: define a self_healing filesystem property

[Christoph Hellwig]

self-healing sounds a little imposterous :)

The code itself looks fine though.

Re: [PATCH 1/3] libfrog: define a self_healing filesystem property

[Darrick J. Wong]

On Tue, Jul 30, 2024 at 02:56:50PM -0700, Christoph Hellwig wrote:
> self-healing sounds a little imposterous :)

Do you have a better suggestion?

"auto_fsck"?

"auto_unf***"? ;)

"background_fsck"?

"background_scrub"?

"patrol_scrub"? <ugh>

> The code itself looks fine though.

--D

Re: [PATCH 1/3] libfrog: define a self_healing filesystem property

[Christoph Hellwig]

On Tue, Jul 30, 2024 at 04:51:03PM -0700, Darrick J. Wong wrote:
> On Tue, Jul 30, 2024 at 02:56:50PM -0700, Christoph Hellwig wrote:
> > self-healing sounds a little imposterous :)
> 
> Do you have a better suggestion?

Heh.  Maybe actually make it to separate ones?
auto_scrub and auto_repair?

Re: [PATCH 1/3] libfrog: define a self_healing filesystem property

[Darrick J. Wong]

On Wed, Jul 31, 2024 at 08:43:19AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 30, 2024 at 04:51:03PM -0700, Darrick J. Wong wrote:
> > On Tue, Jul 30, 2024 at 02:56:50PM -0700, Christoph Hellwig wrote:
> > > self-healing sounds a little imposterous :)
> > 
> > Do you have a better suggestion?
> 
> Heh.  Maybe actually make it to separate ones?
> auto_scrub and auto_repair?

I don't want multiple attributes because that enables sysadmins to give
us mixed messages.  What happens if we encounter this:

auto_scrub=no
auto_repair=yes
auto_optimize=paula

?

Better just to have one attribute and make them pick the one they want.

How about auto_fsck={none,check,optimize,repair} ?

--D

Re: [PATCH 1/3] libfrog: define a self_healing filesystem property

[Christoph Hellwig]

On Wed, Jul 31, 2024 at 10:45:31AM -0700, Darrick J. Wong wrote:
> Better just to have one attribute and make them pick the one they want.
> 
> How about auto_fsck={none,check,optimize,repair} ?

Sounds good.

[PATCH 2/3] xfs_scrub: allow sysadmin to control background scrubs

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Define a "self_healing" filesystem property so that sysadmins can
indicate their preferences for background online fsck.  Add an extended
option to xfs_scrub so that it selects the operation mode from the self
healing fs property.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 man/man8/xfs_scrub.8 |   44 +++++++++++++++++++++++++++
 scrub/phase1.c       |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++
 scrub/xfs_scrub.c    |   14 +++++++++
 scrub/xfs_scrub.h    |    7 ++++
 4 files changed, 146 insertions(+)


diff --git a/man/man8/xfs_scrub.8 b/man/man8/xfs_scrub.8
index 1fd122f2a242..1e017078019c 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 fsprops_advise
+Decide the operating mode from the value of the
+.I self_healing
+filesystem property.
+See the
+.B filesytem 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,42 @@ 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 self_healing
+via the
+.B setfsprops
+subcommand of the
+.B xfs_spaceman
+on the filesystem.
+These preferences will be honored if the
+.B -o fsprops_advise
+option is specified.
+
+Recognized values for the
+.B self_healing
+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
+.IR check .
+
 .SH EXIT CODE
 The exit code returned by
 .B xfs_scrub
diff --git a/scrub/phase1.c b/scrub/phase1.c
index 091b59e57e7b..5fa215a5bb79 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,77 @@ enable_force_repair(
 	return error;
 }
 
+#define MAX_SELFHEAL_LEN		128
+/*
+ * Decide the operating mode from filesystem properties.  No fs property or
+ * system errors means we only check.
+ */
+static void
+mode_from_fsprops(
+	struct scrub_ctx		*ctx)
+{
+	struct fsprops_handle		fph = { };
+	char				valuebuf[MAX_SELFHEAL_LEN + 1] = { 0 };
+	size_t				valuelen = MAX_SELFHEAL_LEN;
+	enum fsprop_self_healing	shval;
+	int				ret;
+
+	ret = fsprops_open_handle(&ctx->mnt, &ctx->fsinfo, &fph);
+	if (ret) {
+		ctx->mode = SCRUB_MODE_DRY_RUN;
+		goto summarize;
+	}
+
+	ret = fsprops_get(&fph, FSPROP_SELF_HEALING_NAME, valuebuf, &valuelen);
+	if (ret) {
+		ctx->mode = SCRUB_MODE_DRY_RUN;
+		goto summarize;
+	}
+
+	shval = fsprop_read_self_healing(valuebuf);
+	switch (shval) {
+	case FSPROP_SELFHEAL_NONE:
+		ctx->mode = SCRUB_MODE_NONE;
+		break;
+	case FSPROP_SELFHEAL_OPTIMIZE:
+		ctx->mode = SCRUB_MODE_PREEN;
+		break;
+	case FSPROP_SELFHEAL_REPAIR:
+		ctx->mode = SCRUB_MODE_REPAIR;
+		break;
+	case FSPROP_SELFHEAL_UNSET:
+		str_info(ctx, ctx->mntpoint,
+ _("Unknown self_healing directive \"%s\"."),
+				valuebuf);
+		fallthrough;
+	case FSPROP_SELFHEAL_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 self_healing directive."));
+		break;
+	case SCRUB_MODE_DRY_RUN:
+		str_info(ctx, ctx->mntpoint,
+ _("Checking per self_healing directive."));
+		break;
+	case SCRUB_MODE_PREEN:
+		str_info(ctx, ctx->mntpoint,
+ _("Optimizing per self_healing directive."));
+		break;
+	case SCRUB_MODE_REPAIR:
+		str_info(ctx, ctx->mntpoint,
+ _("Checking and repairing per self_healing directive."));
+		break;
+	}
+}
+
 /*
  * 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 +279,14 @@ _("Not an XFS filesystem."));
 		return error;
 	}
 
+	/*
+	 * If we've been instructed to decide the operating mode from the
+	 * fs properties set on the mount point, do that now before we start
+	 * downgrading based on actual fs/kernel capabilities.
+	 */
+	if (ctx->mode == SCRUB_MODE_NONE)
+		mode_from_fsprops(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 f5b58de12812..a9d7e5ffe6d7 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,
+	FSPROPS_ADVISE,
 	O_MAX_OPTS,
 };
 
 static char *o_opts[] = {
 	[IWARN]			= "iwarn",
 	[FSTRIM_PCT]		= "fstrim_pct",
+	[FSPROPS_ADVISE]	= "fsprops_advise",
 	[O_MAX_OPTS]		= NULL,
 };
 
@@ -688,6 +694,14 @@ parse_o_opts(
 
 			ctx->fstrim_block_pct = dval / 100.0;
 			break;
+		case FSPROPS_ADVISE:
+			if (val) {
+				fprintf(stderr,
+ _("-o fsprops_advise 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 4d9a028921b5..582ec8e579e9 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
+	 * "self_healing" 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,

[PATCH 3/3] mkfs: set self_healing property

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Add a new mkfs options 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 d5a0783ac5d6..a66fd2a606ed 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 self_healing= value
+Set the self_healing 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 9fa1f9378f32..291200a1ff23 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
+self_healing=0
 
 [inode]
 sparse=1
diff --git a/mkfs/lts_5.10.conf b/mkfs/lts_5.10.conf
index d64bcdf8c46b..7c95dcf4c1ce 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
+self_healing=0
 
 [inode]
 sparse=1
diff --git a/mkfs/lts_5.15.conf b/mkfs/lts_5.15.conf
index 775fd9ab91b8..8797078e406a 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
+self_healing=0
 
 [inode]
 sparse=1
diff --git a/mkfs/lts_5.4.conf b/mkfs/lts_5.4.conf
index 6f43a6c6d469..c741b8260d90 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
+self_healing=0
 
 [inode]
 sparse=1
diff --git a/mkfs/lts_6.1.conf b/mkfs/lts_6.1.conf
index a78a4f9e35dc..834facc1d5fb 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
+self_healing=0
 
 [inode]
 sparse=1
diff --git a/mkfs/lts_6.6.conf b/mkfs/lts_6.6.conf
index 91a25bd8121f..10e965942e38 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
+self_healing=0
 
 [inode]
 sparse=1
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 394a35771246..ea4e97725541 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_SELFHEAL,
 	M_MAX_OPTS,
 };
 
@@ -809,6 +811,7 @@ static struct opt_params mopts = {
 		[M_REFLINK] = "reflink",
 		[M_INOBTCNT] = "inobtcount",
 		[M_BIGTIME] = "bigtime",
+		[M_SELFHEAL] = "self_healing",
 		[M_MAX_OPTS] = NULL,
 	},
 	.subopt_params = {
@@ -852,6 +855,12 @@ static struct opt_params mopts = {
 		  .maxval = 1,
 		  .defaultval = 1,
 		},
+		{ .index = M_SELFHEAL,
+		  .conflicts = { { NULL, LAST_CONFLICT } },
+		  .minval = 0,
+		  .maxval = 1,
+		  .defaultval = 1,
+		},
 	},
 };
 
@@ -917,6 +926,8 @@ struct cli_params {
 	char	*cfgfile;
 	char	*protofile;
 
+	enum fsprop_self_healing self_healing;
+
 	/* 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,self_healing=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_SELFHEAL:
+		if (!value || value[0] == 0 || isdigit(value[0])) {
+			long long	ival = getnum(value, opts, subopt);
+
+			if (ival)
+				cli->self_healing = FSPROP_SELFHEAL_REPAIR;
+			else
+				cli->self_healing = FSPROP_SELFHEAL_NONE;
+		} else {
+			cli->self_healing = fsprop_read_self_healing(value);
+			if (cli->self_healing == FSPROP_SELFHEAL_UNSET)
+				illegal(value, "m self_heal");
+		}
+		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->self_healing >= FSPROP_SELFHEAL_CHECK) {
+			if (!cli->sb_feat.rmapbt) {
+				if (cli_opt_set(&mopts, M_RMAPBT)) {
+					fprintf(stdout,
+_("self-healing=%s is less effective without reverse mapping\n"),
+						fsprop_write_self_healing(cli->self_healing));
+				} else {
+					cli->sb_feat.rmapbt = true;
+				}
+			}
+			if (!cli->sb_feat.parent_pointers) {
+				if (cli_opt_set(&nopts, N_PARENT)) {
+					fprintf(stdout,
+_("self-healing=%s is less effective without parent pointers\n"),
+						fsprop_write_self_healing(cli->self_healing));
+				} 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->self_healing != FSPROP_SELFHEAL_UNSET &&
+		    cli_opt_set(&mopts, M_SELFHEAL)) {
+			fprintf(stderr,
+_("self-healing not supported without CRC support\n"));
+			usage();
+		}
+		cli->self_healing = FSPROP_SELFHEAL_UNSET;
 	}
 
 	if (!cli->sb_feat.finobt) {
@@ -4332,6 +4391,63 @@ cfgfile_parse(
 		cli->cfgfile);
 }
 
+static void
+set_self_healing(
+	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_SELF_HEALING_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 = p;
+
+	word = fsprop_write_self_healing(cli->self_healing);
+	if (!word) {
+		fprintf(stderr,
+ _("%s: not sure what to do with self_healing value %u\n"),
+				progname, cli->self_healing);
+		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 self_healing 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 */
+		.self_healing = FSPROP_SELFHEAL_UNSET,
 	};
 	struct mkfs_params	cfg = {};
 
@@ -4669,6 +4786,9 @@ main(
 	if (mp->m_sb.sb_agcount > 1)
 		rewrite_secondary_superblocks(mp);
 
+	if (cli.self_healing != FSPROP_SELFHEAL_UNSET)
+		set_self_healing(mp, &cli);
+
 	/*
 	 * Dump all inodes and buffers before marking us all done.
 	 * Need to drop references to inodes we still hold, first.

[PATCHSET v30.9 3/3] xfs_scrub: separate package for self healing

[Darrick J. Wong]

Hi all,

Debian policy (section 9 iirc) says that services shipped in a package should
be started by default.  We don't necessarily want that from the base xfsprogs
package long term, so separate the self healing services into a separate
package.

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=scrub-self-healing-package
---
Commits in this patchset:
 * misc: shift install targets
 * xfs_scrub: use the self_healing fsproperty to select mode
 * debian: create a new package for automatic self-healing
---
 Makefile                          |   14 +++++++++++---
 copy/Makefile                     |    6 +++++-
 db/Makefile                       |    6 +++++-
 debian/Makefile                   |    6 +++++-
 debian/control                    |    8 ++++++++
 debian/rules                      |   15 ++++++++++-----
 doc/Makefile                      |    7 ++++++-
 estimate/Makefile                 |    6 +++++-
 fsck/Makefile                     |    7 ++++++-
 fsr/Makefile                      |    6 +++++-
 growfs/Makefile                   |    6 +++++-
 include/Makefile                  |    7 ++++++-
 io/Makefile                       |    6 +++++-
 libfrog/Makefile                  |    4 +++-
 libhandle/Makefile                |    6 +++++-
 libxcmd/Makefile                  |    4 +++-
 libxfs/Makefile                   |    7 ++++++-
 libxlog/Makefile                  |    4 +++-
 logprint/Makefile                 |    6 +++++-
 m4/Makefile                       |    4 +++-
 man/Makefile                      |   10 +++++++---
 man/man2/Makefile                 |    4 +++-
 man/man3/Makefile                 |    4 +++-
 man/man5/Makefile                 |    5 ++++-
 man/man8/Makefile                 |    4 +++-
 mdrestore/Makefile                |    6 +++++-
 mkfs/Makefile                     |    6 +++++-
 po/Makefile                       |    7 ++++++-
 quota/Makefile                    |    6 +++++-
 repair/Makefile                   |    6 +++++-
 rtcp/Makefile                     |    6 +++++-
 scrub/Makefile                    |   19 ++++++++++++++-----
 scrub/xfs_scrub@.service.in       |    2 +-
 scrub/xfs_scrub_media@.service.in |    2 +-
 spaceman/Makefile                 |    6 +++++-
 35 files changed, 182 insertions(+), 46 deletions(-)

[PATCH 1/3] misc: shift install targets

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Modify each Makefile so that "install-pkg" installs the main package
contents, and "install" just invokes "install-pkg".  We'll need this
indirection for the next patch where we add an install-selfheal target
to build the xfsprogs-self-healing package but will still want 'make
install' to install everything on a developer's workstation.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 Makefile           |    8 +++++---
 copy/Makefile      |    4 +++-
 db/Makefile        |    4 +++-
 debian/Makefile    |    4 +++-
 debian/rules       |    2 +-
 doc/Makefile       |    4 +++-
 estimate/Makefile  |    4 +++-
 fsck/Makefile      |    4 +++-
 fsr/Makefile       |    4 +++-
 growfs/Makefile    |    4 +++-
 include/Makefile   |    4 +++-
 io/Makefile        |    4 +++-
 libfrog/Makefile   |    2 +-
 libhandle/Makefile |    4 +++-
 libxcmd/Makefile   |    2 +-
 libxfs/Makefile    |    4 +++-
 libxlog/Makefile   |    2 +-
 logprint/Makefile  |    4 +++-
 m4/Makefile        |    2 +-
 man/Makefile       |    8 +++++---
 man/man2/Makefile  |    4 +++-
 man/man3/Makefile  |    4 +++-
 man/man5/Makefile  |    5 ++++-
 man/man8/Makefile  |    4 +++-
 mdrestore/Makefile |    4 +++-
 mkfs/Makefile      |    4 +++-
 po/Makefile        |    4 +++-
 quota/Makefile     |    4 +++-
 repair/Makefile    |    4 +++-
 rtcp/Makefile      |    4 +++-
 scrub/Makefile     |    4 +++-
 spaceman/Makefile  |    4 +++-
 32 files changed, 91 insertions(+), 36 deletions(-)


diff --git a/Makefile b/Makefile
index 4e768526c6fe..44b6c3501539 100644
--- a/Makefile
+++ b/Makefile
@@ -116,15 +116,17 @@ configure: configure.ac
 include/builddefs: configure
 	./configure $$LOCAL_CONFIGURE_OPTIONS
 
-install: $(addsuffix -install,$(SUBDIRS))
+install: install-pkg
+
+install-pkg: $(addsuffix -install-pkg,$(SUBDIRS))
 	$(INSTALL) -m 755 -d $(PKG_DOC_DIR)
 	$(INSTALL) -m 644 README $(PKG_DOC_DIR)
 
 install-dev: $(addsuffix -install-dev,$(SUBDIRS))
 
-%-install:
+%-install-pkg:
 	@echo "Installing $@"
-	$(Q)$(MAKE) $(MAKEOPTS) -C $* install
+	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-pkg
 
 %-install-dev:
 	@echo "Installing $@"
diff --git a/copy/Makefile b/copy/Makefile
index 55160f848225..446d38bea576 100644
--- a/copy/Makefile
+++ b/copy/Makefile
@@ -18,7 +18,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/db/Makefile b/db/Makefile
index 83389376c36c..91e259044beb 100644
--- a/db/Makefile
+++ b/db/Makefile
@@ -81,7 +81,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 xfs_admin.sh $(PKG_SBIN_DIR)/xfs_admin
diff --git a/debian/Makefile b/debian/Makefile
index 2f9cd38c2ea6..f6a996e91871 100644
--- a/debian/Makefile
+++ b/debian/Makefile
@@ -15,7 +15,9 @@ default:
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 ifeq ($(PKG_DISTRIBUTION), debian)
 	$(INSTALL) -m 755 -d $(PKG_DOC_DIR)
 	$(INSTALL) -m 644 changelog $(PKG_DOC_DIR)/changelog.Debian
diff --git a/debian/rules b/debian/rules
index 69a79fc67405..e4eb2499e768 100755
--- a/debian/rules
+++ b/debian/rules
@@ -100,7 +100,7 @@ binary-arch: checkroot built
 	@echo "== dpkg-buildpackage: binary-arch" 1>&2
 	$(checkdir)
 	-rm -rf $(dirme) $(dirdev) $(dirdi)
-	$(pkgme)  $(MAKE) -C . install
+	$(pkgme)  $(MAKE) -C . install-pkg
 	$(pkgdev) $(MAKE) -C . install-dev
 	$(pkgdi)  $(MAKE) -C debian install-d-i
 	$(pkgme)  $(MAKE) dist
diff --git a/doc/Makefile b/doc/Makefile
index 83dfa38bedd1..ad6749b8d0be 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -16,7 +16,9 @@ CHANGES.gz:
 	@echo "    [ZIP]    $@"
 	$(Q)$(ZIP) --best -c < CHANGES > $@
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_DOC_DIR)
 	$(INSTALL) -m 644 CHANGES.gz CREDITS $(PKG_DOC_DIR)
 ifeq ($(PKG_DISTRIBUTION), debian)
diff --git a/estimate/Makefile b/estimate/Makefile
index 1080129b3a62..d5f8a6d81d65 100644
--- a/estimate/Makefile
+++ b/estimate/Makefile
@@ -12,7 +12,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/fsck/Makefile b/fsck/Makefile
index 5ca529f53e42..ccba7f0b6892 100644
--- a/fsck/Makefile
+++ b/fsck/Makefile
@@ -11,7 +11,9 @@ default: $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 xfs_fsck.sh $(PKG_SBIN_DIR)/fsck.xfs
 install-dev:
diff --git a/fsr/Makefile b/fsr/Makefile
index d57f2de24c22..3ad9f6d824c6 100644
--- a/fsr/Makefile
+++ b/fsr/Makefile
@@ -15,7 +15,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/growfs/Makefile b/growfs/Makefile
index 2f4cc66a76f3..e0ab870bd6ba 100644
--- a/growfs/Makefile
+++ b/growfs/Makefile
@@ -23,7 +23,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/include/Makefile b/include/Makefile
index f7c40a5ce1a1..23727fccfdcd 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -57,7 +57,9 @@ install-headers: $(addsuffix -hdrs, $(DKHFILES) $(HFILES))
 %-hdrs:
 	$(Q)$(LN_S) -f $(CURDIR)/$* xfs/$*
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_INC_DIR)
 
 install-dev: install
diff --git a/io/Makefile b/io/Makefile
index 3192b813c740..d035420b555c 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -85,7 +85,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 xfs_bmap.sh $(PKG_SBIN_DIR)/xfs_bmap
diff --git a/libfrog/Makefile b/libfrog/Makefile
index acddc894ee93..5ebe36fb58c8 100644
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -79,6 +79,6 @@ crc32table.h: gen_crc32table.c crc32defs.h
 
 include $(BUILDRULES)
 
-install install-dev: default
+install install-pkg install-dev: default
 
 -include .ltdep
diff --git a/libhandle/Makefile b/libhandle/Makefile
index f297a59e47f9..7cfd0fa4f27e 100644
--- a/libhandle/Makefile
+++ b/libhandle/Makefile
@@ -19,7 +19,9 @@ default: ltdepend $(LTLIBRARY)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL_LTLIB)
 
 install-dev: default
diff --git a/libxcmd/Makefile b/libxcmd/Makefile
index 70e54308c34b..afd5349c8af3 100644
--- a/libxcmd/Makefile
+++ b/libxcmd/Makefile
@@ -23,6 +23,6 @@ default: ltdepend $(LTLIBRARY)
 
 include $(BUILDRULES)
 
-install install-dev: default
+install install-pkg install-dev: default
 
 -include .ltdep
diff --git a/libxfs/Makefile b/libxfs/Makefile
index 2f2791cae587..2c6b45953661 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -140,7 +140,9 @@ default: ltdepend $(LTLIBRARY)
 # set up include/xfs header directory
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_INC_DIR)
 
 install-headers: $(addsuffix -hdrs, $(PKGHFILES))
diff --git a/libxlog/Makefile b/libxlog/Makefile
index b0f5ef154133..3710729fe703 100644
--- a/libxlog/Makefile
+++ b/libxlog/Makefile
@@ -21,6 +21,6 @@ default: ltdepend $(LTLIBRARY)
 
 include $(BUILDRULES)
 
-install install-dev: default
+install install-pkg install-dev: default
 
 -include .ltdep
diff --git a/logprint/Makefile b/logprint/Makefile
index bbbed5d259af..5ec02539a7bb 100644
--- a/logprint/Makefile
+++ b/logprint/Makefile
@@ -21,7 +21,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/m4/Makefile b/m4/Makefile
index 84174c3d3e30..eda4c06f6864 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -33,7 +33,7 @@ default:
 
 include $(BUILDRULES)
 
-install install-dev install-lib: default
+install install-pkg install-dev install-lib: default
 
 realclean: distclean
 	rm -f $(CONFIGURE)
diff --git a/man/Makefile b/man/Makefile
index cd1aed6cf202..f62286e8339d 100644
--- a/man/Makefile
+++ b/man/Makefile
@@ -9,12 +9,14 @@ SUBDIRS = man2 man3 man5 man8
 
 default : $(SUBDIRS)
 
-install : $(addsuffix -install,$(SUBDIRS))
+install : install-pkg
+
+install-pkg : $(addsuffix -install-pkg,$(SUBDIRS))
 
 install-dev : $(addsuffix -install-dev,$(SUBDIRS))
 
-%-install:
-	$(Q)$(MAKE) $(MAKEOPTS) -C $* install
+%-install-pkg:
+	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-pkg
 
 %-install-dev:
 	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-dev
diff --git a/man/man2/Makefile b/man/man2/Makefile
index 8aecde3321b0..190ea18e7c6c 100644
--- a/man/man2/Makefile
+++ b/man/man2/Makefile
@@ -15,7 +15,9 @@ default : $(MAN_PAGES)
 
 include $(BUILDRULES)
 
-install :
+install : install-pkg
+
+install-pkg :
 
 install-dev : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
diff --git a/man/man3/Makefile b/man/man3/Makefile
index a7f607fcbad9..1553e2b2de82 100644
--- a/man/man3/Makefile
+++ b/man/man3/Makefile
@@ -15,7 +15,9 @@ default : $(MAN_PAGES)
 
 include $(BUILDRULES)
 
-install :
+install : install-pkg
+
+install-pkg :
 
 install-dev : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
diff --git a/man/man5/Makefile b/man/man5/Makefile
index fe0aef6f016b..1fcd3995036f 100644
--- a/man/man5/Makefile
+++ b/man/man5/Makefile
@@ -15,7 +15,10 @@ default : $(MAN_PAGES)
 
 include $(BUILDRULES)
 
-install : default
+install : install-pkg
+
+install-pkg : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
 	$(INSTALL_MAN)
+
 install-dev :
diff --git a/man/man8/Makefile b/man/man8/Makefile
index 5be76ab727a1..0b40a409a913 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -22,7 +22,9 @@ default : $(MAN_PAGES)
 
 include $(BUILDRULES)
 
-install : default
+install : install-pkg
+
+install-pkg : default
 	$(INSTALL) -m 755 -d $(MAN_DEST)
 	$(INSTALL_MAN)
 
diff --git a/mdrestore/Makefile b/mdrestore/Makefile
index 4a932efb8098..0d02fb383404 100644
--- a/mdrestore/Makefile
+++ b/mdrestore/Makefile
@@ -16,7 +16,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/mkfs/Makefile b/mkfs/Makefile
index a6173083e4c2..cf945aa10c25 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -27,7 +27,9 @@ default: depend $(LTCOMMAND) $(CFGFILES)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 -d $(MKFS_CFG_DIR)
diff --git a/po/Makefile b/po/Makefile
index 1d35f5191ba2..3cc0b4177c64 100644
--- a/po/Makefile
+++ b/po/Makefile
@@ -19,7 +19,9 @@ default: $(POTHEAD) $(LINGUAS:%=%.mo)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL_LINGUAS)
 
 install-dev install-lib:
diff --git a/quota/Makefile b/quota/Makefile
index da5a1489e468..01584635b3dd 100644
--- a/quota/Makefile
+++ b/quota/Makefile
@@ -23,7 +23,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/repair/Makefile b/repair/Makefile
index a36a95e353a5..096ae8c6a5b1 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -106,7 +106,9 @@ include $(BUILDRULES)
 #
 #CFLAGS += ...
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/rtcp/Makefile b/rtcp/Makefile
index 264b4f27b5fd..4adb58c4b783 100644
--- a/rtcp/Makefile
+++ b/rtcp/Makefile
@@ -16,7 +16,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
diff --git a/scrub/Makefile b/scrub/Makefile
index 885b43e9948d..653aafd171b5 100644
--- a/scrub/Makefile
+++ b/scrub/Makefile
@@ -138,7 +138,9 @@ phase5.o unicrash.o xfs.o: $(builddefs)
 
 include $(BUILDRULES)
 
-install: $(INSTALL_SCRUB)
+install: install-pkg
+
+install-pkg: $(INSTALL_SCRUB)
 
 %.service: %.service.in $(builddefs)
 	@echo "    [SED]    $@"
diff --git a/spaceman/Makefile b/spaceman/Makefile
index e914b921de8b..49fbc9290c02 100644
--- a/spaceman/Makefile
+++ b/spaceman/Makefile
@@ -38,7 +38,9 @@ default: depend $(LTCOMMAND)
 
 include $(BUILDRULES)
 
-install: default
+install: install-pkg
+
+install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 xfs_info.sh $(PKG_SBIN_DIR)/xfs_info

[PATCH 2/3] xfs_scrub: use the self_healing fsproperty to select mode

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Now that we can set properties on xfs filesystems, make the xfs_scrub
background service query the self_healing property to figure out which
mode (dry run, optimize, repair, none) 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 653aafd171b5..b0022cb7f005 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 fsprops_advise
 ifeq ($(HAVE_SYSTEMD),yes)
 INSTALL_SCRUB += install-systemd
 SYSTEMD_SERVICES=\
@@ -146,7 +146,6 @@ install-pkg: $(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 5fa5f328200e..fb38319e95c1 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 e670748ced51..98cd1ac44fbe 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.

[PATCH 3/3] debian: create a new package for automatic self-healing

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Create a new package for people who explicilty want self-healing turned
on by default for XFS.  This package is named xfsprogs-self-healing.

Note: This introduces a new "install-selfheal" target to install only
the files needed for enabling online fsck by default.  Other
distributions should take note of the new target if they choose to
create a package for enabling autonomous self healing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 Makefile           |    8 +++++++-
 copy/Makefile      |    2 ++
 db/Makefile        |    2 ++
 debian/Makefile    |    2 ++
 debian/control     |    8 ++++++++
 debian/rules       |   13 +++++++++----
 doc/Makefile       |    3 +++
 estimate/Makefile  |    2 ++
 fsck/Makefile      |    3 +++
 fsr/Makefile       |    2 ++
 growfs/Makefile    |    2 ++
 include/Makefile   |    3 +++
 io/Makefile        |    2 ++
 libfrog/Makefile   |    2 ++
 libhandle/Makefile |    2 ++
 libxcmd/Makefile   |    2 ++
 libxfs/Makefile    |    3 +++
 libxlog/Makefile   |    2 ++
 logprint/Makefile  |    2 ++
 m4/Makefile        |    2 ++
 man/Makefile       |    2 ++
 mdrestore/Makefile |    2 ++
 mkfs/Makefile      |    2 ++
 po/Makefile        |    3 +++
 quota/Makefile     |    2 ++
 repair/Makefile    |    2 ++
 rtcp/Makefile      |    2 ++
 scrub/Makefile     |   12 ++++++++++--
 spaceman/Makefile  |    2 ++
 29 files changed, 89 insertions(+), 7 deletions(-)


diff --git a/Makefile b/Makefile
index 44b6c3501539..4bd792d191be 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,7 @@ configure: configure.ac
 include/builddefs: configure
 	./configure $$LOCAL_CONFIGURE_OPTIONS
 
-install: install-pkg
+install: install-pkg install-selfheal
 
 install-pkg: $(addsuffix -install-pkg,$(SUBDIRS))
 	$(INSTALL) -m 755 -d $(PKG_DOC_DIR)
@@ -124,6 +124,8 @@ install-pkg: $(addsuffix -install-pkg,$(SUBDIRS))
 
 install-dev: $(addsuffix -install-dev,$(SUBDIRS))
 
+install-selfheal: $(addsuffix -install-selfheal,$(SUBDIRS))
+
 %-install-pkg:
 	@echo "Installing $@"
 	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-pkg
@@ -132,6 +134,10 @@ install-dev: $(addsuffix -install-dev,$(SUBDIRS))
 	@echo "Installing $@"
 	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-dev
 
+%-install-selfheal:
+	@echo "Installing $@"
+	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-selfheal
+
 distclean: clean
 	$(Q)rm -f $(LDIRT)
 
diff --git a/copy/Makefile b/copy/Makefile
index 446d38bea576..3013dc2dca27 100644
--- a/copy/Makefile
+++ b/copy/Makefile
@@ -25,4 +25,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/db/Makefile b/db/Makefile
index 91e259044beb..839c51f03593 100644
--- a/db/Makefile
+++ b/db/Makefile
@@ -91,4 +91,6 @@ install-pkg: default
 	$(INSTALL) -m 755 xfs_metadump.sh $(PKG_SBIN_DIR)/xfs_metadump
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/debian/Makefile b/debian/Makefile
index f6a996e91871..104d7c5d76d9 100644
--- a/debian/Makefile
+++ b/debian/Makefile
@@ -36,3 +36,5 @@ ifeq ($(PKG_DISTRIBUTION), debian)
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 $(BOOT_MKFS_BIN) $(PKG_SBIN_DIR)/mkfs.xfs
 endif
+
+install-selfheal: default
diff --git a/debian/control b/debian/control
index 31773e53a19a..aa7a920a7964 100644
--- a/debian/control
+++ b/debian/control
@@ -27,6 +27,14 @@ Description: Utilities for managing the XFS filesystem
  Refer to the documentation at https://xfs.wiki.kernel.org/
  for complete details.
 
+Package: xfsprogs-self-healing
+Depends: ${shlibs:Depends}, ${misc:Depends}, xfsprogs, systemd, udev
+Architecture: linux-any
+Description: Automatic self healing for the XFS filesystem
+ A set of background services for the XFS filesystem to make it
+ find and fix corruptions automatically.  These services are activated
+ automatically upon installation of this package.
+
 Package: xfslibs-dev
 Section: libdevel
 Depends: libc6-dev | libc-dev, uuid-dev, xfsprogs (>= 3.0.0), ${misc:Depends}
diff --git a/debian/rules b/debian/rules
index e4eb2499e768..c49aefd10e64 100755
--- a/debian/rules
+++ b/debian/rules
@@ -14,6 +14,7 @@ endif
 package = xfsprogs
 develop = xfslibs-dev
 bootpkg = xfsprogs-udeb
+healpkg = xfsprogs-self-healing
 
 DEB_BUILD_GNU_TYPE ?= $(shell dpkg-architecture -qDEB_BUILD_GNU_TYPE)
 DEB_HOST_GNU_TYPE ?= $(shell dpkg-architecture -qDEB_HOST_GNU_TYPE)
@@ -26,9 +27,11 @@ udebpkg = $(bootpkg)_$(version)_$(target).udeb
 dirme  = debian/$(package)
 dirdev = debian/$(develop)
 dirdi  = debian/$(bootpkg)
-pkgme  = DIST_ROOT=`pwd`/$(dirme);  export DIST_ROOT;
-pkgdev = DIST_ROOT=`pwd`/$(dirdev); export DIST_ROOT;
-pkgdi  = DIST_ROOT=`pwd`/$(dirdi); export DIST_ROOT;
+dirheal= debian/$(healpkg)
+pkgme  = DIST_ROOT=`pwd`/$(dirme);   export DIST_ROOT;
+pkgdev = DIST_ROOT=`pwd`/$(dirdev);  export DIST_ROOT;
+pkgdi  = DIST_ROOT=`pwd`/$(dirdi);   export DIST_ROOT;
+pkgheal= DIST_ROOT=`pwd`/$(dirheal); export DIST_ROOT;
 stdenv = @GZIP=-q; export GZIP;
 
 configure_options = \
@@ -103,6 +106,7 @@ binary-arch: checkroot built
 	$(pkgme)  $(MAKE) -C . install-pkg
 	$(pkgdev) $(MAKE) -C . install-dev
 	$(pkgdi)  $(MAKE) -C debian install-d-i
+	$(pkgheal) $(MAKE) -C . install-selfheal
 	$(pkgme)  $(MAKE) dist
 	install -D -m 0755 debian/local/initramfs.hook debian/xfsprogs/usr/share/initramfs-tools/hooks/xfs
 	rmdir debian/xfslibs-dev/usr/share/doc/xfsprogs
@@ -114,7 +118,8 @@ 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
+	dh_installsystemd -p xfsprogs-self-healing --no-restart-after-upgrade --no-stop-on-upgrade xfs_scrub_all.timer
 	dh_installdeb
 	dh_shlibdeps
 	dh_gencontrol
diff --git a/doc/Makefile b/doc/Makefile
index ad6749b8d0be..a6ec65f5edc0 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -26,3 +26,6 @@ ifeq ($(PKG_DISTRIBUTION), debian)
 endif
 
 install-dev:
+
+install-selfheal:
+
diff --git a/estimate/Makefile b/estimate/Makefile
index d5f8a6d81d65..4fce3463d55c 100644
--- a/estimate/Makefile
+++ b/estimate/Makefile
@@ -19,4 +19,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/fsck/Makefile b/fsck/Makefile
index ccba7f0b6892..33fca9e18fb0 100644
--- a/fsck/Makefile
+++ b/fsck/Makefile
@@ -17,3 +17,6 @@ install-pkg: default
 	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
 	$(INSTALL) -m 755 xfs_fsck.sh $(PKG_SBIN_DIR)/fsck.xfs
 install-dev:
+
+install-selfheal:
+
diff --git a/fsr/Makefile b/fsr/Makefile
index 3ad9f6d824c6..da32bc531970 100644
--- a/fsr/Makefile
+++ b/fsr/Makefile
@@ -22,4 +22,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/growfs/Makefile b/growfs/Makefile
index e0ab870bd6ba..61d184328cf6 100644
--- a/growfs/Makefile
+++ b/growfs/Makefile
@@ -30,4 +30,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/include/Makefile b/include/Makefile
index 23727fccfdcd..c18ab35a9861 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -64,3 +64,6 @@ install-pkg: default
 
 install-dev: install
 	$(INSTALL) -m 644 $(HFILES) $(PKG_INC_DIR)
+
+install-selfheal:
+
diff --git a/io/Makefile b/io/Makefile
index d035420b555c..70c95a5c1425 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -95,4 +95,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 xfs_mkfile.sh $(PKG_SBIN_DIR)/xfs_mkfile
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/libfrog/Makefile b/libfrog/Makefile
index 5ebe36fb58c8..262a4a0f8cda 100644
--- a/libfrog/Makefile
+++ b/libfrog/Makefile
@@ -81,4 +81,6 @@ include $(BUILDRULES)
 
 install install-pkg install-dev: default
 
+install-selfheal:
+
 -include .ltdep
diff --git a/libhandle/Makefile b/libhandle/Makefile
index 7cfd0fa4f27e..28c104a77f95 100644
--- a/libhandle/Makefile
+++ b/libhandle/Makefile
@@ -27,4 +27,6 @@ install-pkg: default
 install-dev: default
 	$(INSTALL_LTLIB_DEV)
 
+install-selfheal:
+
 -include .ltdep
diff --git a/libxcmd/Makefile b/libxcmd/Makefile
index afd5349c8af3..118ecd3cfc88 100644
--- a/libxcmd/Makefile
+++ b/libxcmd/Makefile
@@ -25,4 +25,6 @@ include $(BUILDRULES)
 
 install install-pkg install-dev: default
 
+install-selfheal:
+
 -include .ltdep
diff --git a/libxfs/Makefile b/libxfs/Makefile
index 2c6b45953661..65d4c7619758 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -162,3 +162,6 @@ install-dev: install
 ifndef NODEP
 -include .ltdep
 endif
+
+install-selfheal:
+
diff --git a/libxlog/Makefile b/libxlog/Makefile
index 3710729fe703..37281eb058f2 100644
--- a/libxlog/Makefile
+++ b/libxlog/Makefile
@@ -23,4 +23,6 @@ include $(BUILDRULES)
 
 install install-pkg install-dev: default
 
+install-selfheal:
+
 -include .ltdep
diff --git a/logprint/Makefile b/logprint/Makefile
index 5ec02539a7bb..f1fccbdc8ca7 100644
--- a/logprint/Makefile
+++ b/logprint/Makefile
@@ -28,4 +28,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/m4/Makefile b/m4/Makefile
index eda4c06f6864..b894dd3b0222 100644
--- a/m4/Makefile
+++ b/m4/Makefile
@@ -35,5 +35,7 @@ include $(BUILDRULES)
 
 install install-pkg install-dev install-lib: default
 
+install-selfheal:
+
 realclean: distclean
 	rm -f $(CONFIGURE)
diff --git a/man/Makefile b/man/Makefile
index f62286e8339d..258e8732a923 100644
--- a/man/Makefile
+++ b/man/Makefile
@@ -21,4 +21,6 @@ install-dev : $(addsuffix -install-dev,$(SUBDIRS))
 %-install-dev:
 	$(Q)$(MAKE) $(MAKEOPTS) -C $* install-dev
 
+install-selfheal:
+
 include $(BUILDRULES)
diff --git a/mdrestore/Makefile b/mdrestore/Makefile
index 0d02fb383404..c75d5875929b 100644
--- a/mdrestore/Makefile
+++ b/mdrestore/Makefile
@@ -23,4 +23,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/mkfs/Makefile b/mkfs/Makefile
index cf945aa10c25..515c2db0639d 100644
--- a/mkfs/Makefile
+++ b/mkfs/Makefile
@@ -37,4 +37,6 @@ install-pkg: default
 
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/po/Makefile b/po/Makefile
index 3cc0b4177c64..cb6ef954b695 100644
--- a/po/Makefile
+++ b/po/Makefile
@@ -25,3 +25,6 @@ install-pkg: default
 	$(INSTALL_LINGUAS)
 
 install-dev install-lib:
+
+install-selfheal:
+
diff --git a/quota/Makefile b/quota/Makefile
index 01584635b3dd..e1193bd203ad 100644
--- a/quota/Makefile
+++ b/quota/Makefile
@@ -30,4 +30,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/repair/Makefile b/repair/Makefile
index 096ae8c6a5b1..e7979a817d4f 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -113,4 +113,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/rtcp/Makefile b/rtcp/Makefile
index 4adb58c4b783..ac638bcd1029 100644
--- a/rtcp/Makefile
+++ b/rtcp/Makefile
@@ -23,4 +23,6 @@ install-pkg: default
 	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
 install-dev:
 
+install-selfheal:
+
 -include .dep
diff --git a/scrub/Makefile b/scrub/Makefile
index b0022cb7f005..2c349c682d3a 100644
--- a/scrub/Makefile
+++ b/scrub/Makefile
@@ -20,6 +20,7 @@ XFS_SCRUB_ARGS = -p
 XFS_SCRUB_SERVICE_ARGS = -b -o fsprops_advise
 ifeq ($(HAVE_SYSTEMD),yes)
 INSTALL_SCRUB += install-systemd
+INSTALL_SELFHEAL += install-systemd-selfheal
 SYSTEMD_SERVICES=\
 	$(scrub_svcname) \
 	xfs_scrub_fail@.service \
@@ -27,9 +28,10 @@ SYSTEMD_SERVICES=\
 	xfs_scrub_media_fail@.service \
 	xfs_scrub_all.service \
 	xfs_scrub_all_fail.service \
-	xfs_scrub_all.timer \
 	system-xfs_scrub.slice
-OPTIONAL_TARGETS += $(SYSTEMD_SERVICES)
+SYSTEMD_SERVICES_SELFHEAL=\
+	xfs_scrub_all.timer
+OPTIONAL_TARGETS += $(SYSTEMD_SERVICES) $(SYSTEMD_SERVICES_SELFHEAL)
 endif
 ifeq ($(HAVE_CROND),yes)
 INSTALL_SCRUB += install-crond
@@ -163,6 +165,10 @@ install-systemd: default $(SYSTEMD_SERVICES)
 	$(INSTALL) -m 755 -d $(PKG_LIBEXEC_DIR)
 	$(INSTALL) -m 755 $(XFS_SCRUB_FAIL_PROG) $(PKG_LIBEXEC_DIR)
 
+install-systemd-selfheal: default $(SYSTEMD_SERVICES_SELFHEAL)
+	$(INSTALL) -m 755 -d $(SYSTEMD_SYSTEM_UNIT_DIR)
+	$(INSTALL) -m 644 $(SYSTEMD_SERVICES_SELFHEAL) $(SYSTEMD_SYSTEM_UNIT_DIR)
+
 install-crond: default $(CRONTABS)
 	$(INSTALL) -m 755 -d $(CROND_DIR)
 	$(INSTALL) -m 644 $(CRONTABS) $(CROND_DIR)
@@ -181,4 +187,6 @@ install-udev: $(UDEV_RULES)
 
 install-dev:
 
+install-selfheal: $(INSTALL_SELFHEAL)
+
 -include .dep
diff --git a/spaceman/Makefile b/spaceman/Makefile
index 49fbc9290c02..ce1650f99b7c 100644
--- a/spaceman/Makefile
+++ b/spaceman/Makefile
@@ -47,4 +47,6 @@ install-pkg: default
 	$(INSTALL) -m 755 xfs_property $(PKG_SBIN_DIR)/xfs_property
 install-dev:
 
+install-selfheal:
+
 -include .dep

[PATCHSET v30.9] fstests: xfs filesystem properties

[Darrick J. Wong]

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         |    4 +-
 tests/xfs/1886     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1886.out |   53 ++++++++++++++++++++
 tests/xfs/1887     |  124 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1887.out |   46 ++++++++++++++++++
 tests/xfs/1888     |   66 +++++++++++++++++++++++++
 tests/xfs/1888.out |    9 +++
 tests/xfs/1889     |   67 ++++++++++++++++++++++++++
 tests/xfs/1889.out |    9 +++
 10 files changed, 512 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

[PATCH 1/1] xfs: functional testing for filesystem properties

[Darrick J. Wong]

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         |    4 +-
 tests/xfs/1886     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1886.out |   53 ++++++++++++++++++++
 tests/xfs/1887     |  124 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1887.out |   46 ++++++++++++++++++
 tests/xfs/1888     |   66 +++++++++++++++++++++++++
 tests/xfs/1888.out |    9 +++
 tests/xfs/1889     |   67 ++++++++++++++++++++++++++
 tests/xfs/1889.out |    9 +++
 10 files changed, 512 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..a642646345 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
 		;;
 	*)
diff --git a/tests/xfs/1886 b/tests/xfs/1886
new file mode 100755
index 0000000000..eca76f51d6
--- /dev/null
+++ b/tests/xfs/1886
@@ -0,0 +1,135 @@
+#! /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
+
+. ./common/filter
+. ./common/attr
+
+_require_test
+_require_user fsgqa
+_require_attrs
+_require_xfs_spaceman_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
+}
+
+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 "*** SPACEMAN TEST ***"
+
+echo empty get property
+$XFS_SPACEMAN_PROG -c "getfsprops $propname" $TEST_DIR
+
+echo pointless remove property
+$XFS_SPACEMAN_PROG -c "removefsprops $propname" $TEST_DIR
+
+echo list property
+$XFS_SPACEMAN_PROG -c "listfsprops" $TEST_DIR | grep $propname
+
+echo set property
+$XFS_SPACEMAN_PROG -c "setfsprops $propname=$propval" $TEST_DIR
+
+echo list property
+$XFS_SPACEMAN_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_SPACEMAN_PROG -c "getfsprops $propname" $TEST_DIR
+
+echo list property
+$XFS_SPACEMAN_PROG -c "listfsprops" $TEST_DIR | grep $propname
+
+echo child file rejected
+touch $TEST_DIR/$seq.somefile
+$XFS_SPACEMAN_PROG -c "listfsprops $propname" $TEST_DIR/$seq.somefile 2>&1 | \
+	_filter_test_dir
+
+echo child dir accepted
+mkdir -p $TEST_DIR/$seq.somedir
+$XFS_SPACEMAN_PROG -c "listfsprops $propname" $TEST_DIR/$seq.somedir | grep $propname
+
+echo remove property
+$XFS_SPACEMAN_PROG -c "removefsprops $propname" $TEST_DIR
+
+echo pointless remove property
+$XFS_SPACEMAN_PROG -c "removefsprops $propname" $TEST_DIR
+
+echo set too long name
+$XFS_SPACEMAN_PROG -c "setfsprops $longpropname=$propval" $TEST_DIR
+
+echo set too long value
+$XFS_SPACEMAN_PROG -c "setfsprops $propname=$longpropval" $TEST_DIR
+
+echo not enough permissions
+su - "$qa_user" -c "$XFS_SPACEMAN_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..2f05c8b7c5
--- /dev/null
+++ b/tests/xfs/1886.out
@@ -0,0 +1,53 @@
+QA output created by 1886
+*** SPACEMAN 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 directory
+child dir accepted
+fakeproperty
+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..fe75abe2c5
--- /dev/null
+++ b/tests/xfs/1887
@@ -0,0 +1,124 @@
+#! /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
+
+. ./common/filter
+. ./common/attr
+
+_require_test
+_require_attrs
+
+# XXX yeah
+test -z "$XFS_PROPERTY_PROG" && \
+	XFS_PROPERTY_PROG="$(type -P xfs_property.sh)"
+
+_require_command "$XFS_PROPERTY_PROG" xfs_property
+_require_xfs_spaceman_command listfsprops	# actually detect support
+
+_cleanup()
+
+{
+	cd /
+	rm -r -f $tmp.*
+}
+
+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..8d54343cc2
--- /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 self_healing fs property.
+#
+. ./common/preamble
+_begin_fstest auto
+
+. ./common/filter
+. ./common/attr
+
+_require_test
+_require_xfs_spaceman_command listfsprops	# needed for fs props
+_require_xfs_db_command attr_get
+
+_cleanup()
+
+{
+	cd /
+	rm -r -f $tmp.*
+	rm -f $dummyfile
+	rmdir $dummymnt
+}
+
+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' "self_healing$value")
+	fi
+
+	echo "testing ${mkfs_args[*]}" >> $seqres.full
+
+	$MKFS_XFS_PROG $MKFS_OPTIONS "${mkfs_args[@]}" $dummyfile >> $seqres.full || \
+		_notrun "mkfs.xfs ${mkfs_args[*]} failed?"
+
+	$XFS_DB_PROG -x -c 'path /' -c "attr_get -Z self_healing" $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..d386d01b8f
--- /dev/null
+++ b/tests/xfs/1888.out
@@ -0,0 +1,9 @@
+QA output created by 1888
+self_healing=repair
+failed to get attr xfs:self_healing on inode XXX: No data available
+self_healing=none
+self_healing=check
+self_healing=optimize
+self_healing=repair
+self_healing=none
+self_healing=repair
diff --git a/tests/xfs/1889 b/tests/xfs/1889
new file mode 100755
index 0000000000..623f004631
--- /dev/null
+++ b/tests/xfs/1889
@@ -0,0 +1,67 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test 1889
+#
+# Functional testing for mkfs applying self_healing fs property and xfs_scrub
+# changing its behavior accordingly.  Or at least claiming to.
+#
+. ./common/preamble
+_begin_fstest auto
+
+. ./common/filter
+. ./common/fuzzy
+
+_require_test
+_require_xfs_spaceman_command listfsprops	# needed for fs props
+_require_xfs_db_command attr_get
+_require_scrub
+
+_cleanup()
+
+{
+	cd /
+	rm -r -f $tmp.*
+	rm -f $dummyfile
+	_umount $dummymnt &>/dev/null
+	rmdir $dummymnt
+}
+
+dummyfile=$TEST_DIR/$seq.somefile
+dummymnt=$TEST_DIR/$seq.mount
+
+truncate -s 10g $dummyfile
+mkdir -p $dummymnt
+
+testme() {
+	local mkfs_args=('-f')
+	local value="$1"
+	test -n "$value" && value="=$value"
+
+	if [ $# -gt 0 ]; then
+		mkfs_args+=('-m' "self_healing$value")
+	fi
+
+	echo "testing ${mkfs_args[*]}" >> $seqres.full
+
+	$MKFS_XFS_PROG $MKFS_OPTIONS "${mkfs_args[@]}" $dummyfile >> $seqres.full || \
+		_notrun "mkfs.xfs ${mkfs_args[*]} failed?"
+
+	_mount -o loop $dummyfile $dummymnt
+	XFS_SCRUB_PHASE=7 $XFS_SCRUB_PROG -d -o fsprops_advise $dummymnt 2>&1 | \
+		grep self_healing | _filter_test_dir | sed -e 's/\(directive.\).*$/\1/g'
+	_umount $dummymnt
+}
+
+testme ''
+testme
+testme none
+testme check
+testme optimize
+testme repair
+testme 0
+testme 1
+
+status=0
+exit
diff --git a/tests/xfs/1889.out b/tests/xfs/1889.out
new file mode 100644
index 0000000000..707c37edd0
--- /dev/null
+++ b/tests/xfs/1889.out
@@ -0,0 +1,9 @@
+QA output created by 1889
+Info: TEST_DIR/1889.mount: Checking and repairing per self_healing directive.
+Info: TEST_DIR/1889.mount: Checking per self_healing directive.
+Info: TEST_DIR/1889.mount: Disabling scrub per self_healing directive.
+Info: TEST_DIR/1889.mount: Checking per self_healing directive.
+Info: TEST_DIR/1889.mount: Optimizing per self_healing directive.
+Info: TEST_DIR/1889.mount: Checking and repairing per self_healing directive.
+Info: TEST_DIR/1889.mount: Disabling scrub per self_healing directive.
+Info: TEST_DIR/1889.mount: Checking and repairing per self_healing directive.

Re: [PATCHSET v30.9] fstests: xfs filesystem properties

[Darrick J. Wong]

On Mon, Jul 29, 2024 at 08:19:43PM -0700, Darrick J. Wong wrote:
> 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.

Heh, I forgot to cc the patchbomb coverletter -- this is an RFC, not for
merging at this time.

--D

Re: [RFC PATCHSET] xfsprogs: filesystem properties

[Dave Chinner]

On Mon, Jul 29, 2024 at 08:10:30PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> After last week's discussion about how to allow sysadmins to opt in or
> out of autonomous self healing XFS[1], I now have an RFC patchset 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:self_healing" as the
> property that controls the amount of autonomous self healing.  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.

Overall this approach looks fine to me.

Acked-by: Dave Chinner <dchinner@redhat.com>

-Dave.

-- 
Dave Chinner
david@fromorbit.com

[PATCHSET v30.10 1/3] xfsprogs: filesystem properties

[Darrick J. Wong]

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

[PATCH 6/7] xfs_db: add a command to list xattrs

[Darrick J. Wong]

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