[PATCH] xfs_repair: don't fail on INCOMPLETE attrs in leaf blocks

[PATCH] xfs_repair: don't fail on INCOMPLETE attrs in leaf blocks

[Darrick J. Wong]

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

It's far too drastic to delete the entire attr fork because doing that
destroys things like security labels.  The kernel won't show incomplete
attrs so it's not a big deal.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 repair/attr_repair.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 50159b9a533875..fe4089026cae74 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -571,7 +571,13 @@ process_leaf_attr_remote(
 	    !libxfs_attr_namecheck(entry->flags, remotep->name,
 				   remotep->namelen) ||
 	    be32_to_cpu(entry->hashval) != computed ||
-	    be32_to_cpu(entry->hashval) < last_hashval ||
+	    be32_to_cpu(entry->hashval) < last_hashval) {
+		do_warn(
+	_("inconsistent remote attribute entry %d in attr block %u, ino %" PRIu64 "\n"), i, da_bno, ino);
+		return -1;
+	}
+
+	if (!(entry->flags & XFS_ATTR_INCOMPLETE) &&
 	    be32_to_cpu(remotep->valueblk) == 0) {
 		do_warn(
 	_("inconsistent remote attribute entry %d in attr block %u, ino %" PRIu64 "\n"), i, da_bno, ino);
@@ -592,6 +598,9 @@ process_leaf_attr_remote(
 		return -1;
 	}
 
+	if (entry->flags & XFS_ATTR_INCOMPLETE)
+		goto out;
+
 	value = malloc(be32_to_cpu(remotep->valuelen));
 	if (value == NULL) {
 		do_warn(
@@ -708,12 +717,15 @@ process_leaf_attr_block(
 		}
 
 		if (entry->flags & XFS_ATTR_INCOMPLETE) {
-			/* we are inconsistent state. get rid of us */
-			do_warn(
+			/*
+			 * Warn about incomplete xattrs but don't zap the
+			 * entire attr fork because that causes loss of
+			 * security labels.  The kernel can handle stray
+			 * incomplete attr entries.
+			 */
+			do_log(
 	_("attribute entry #%d in attr block %u, inode %" PRIu64 " is INCOMPLETE\n"),
 				i, da_bno, ino);
-			clearit = 1;
-			break;
 		}
 
 		/* mark the entry used */

Re: [PATCH] xfs_repair: don't fail on INCOMPLETE attrs in leaf blocks

[Christoph Hellwig]

On Mon, Mar 16, 2026 at 03:50:33PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> It's far too drastic to delete the entire attr fork because doing that
> destroys things like security labels.  The kernel won't show incomplete
> attrs so it's not a big deal.

This sounds a bit lax.  Incomplete attributes are very much expected
given how the non-logged attr code works.  Maybe reword this a bit to
not make it sound like we just ignore a problem?

Re: [PATCH] xfs_repair: don't fail on INCOMPLETE attrs in leaf blocks

[Darrick J. Wong]

On Tue, Mar 17, 2026 at 02:14:22AM -0700, Christoph Hellwig wrote:
> On Mon, Mar 16, 2026 at 03:50:33PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > It's far too drastic to delete the entire attr fork because doing that
> > destroys things like security labels.  The kernel won't show incomplete
> > attrs so it's not a big deal.
> 
> This sounds a bit lax.  Incomplete attributes are very much expected
> given how the non-logged attr code works.  Maybe reword this a bit to
> not make it sound like we just ignore a problem?

Ok will do.

--D

[PATCH v2] xfs_repair: don't fail on INCOMPLETE attrs in leaf blocks

[Darrick J. Wong]

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

While trying to fix problems in generic/753, I noticed test failures on
account of xfs_repair:

 attribute entry #4 in attr block 0, inode 131 is INCOMPLETE
 problem with attribute contents in inode 131
 would clear attr fork
 bad nblocks 4 for inode 131, would reset to 0
 bad anextents 1 for inode 131, would reset to 0

Looking at the dumped filesystem, inode 131 is a linked file, and the
"incomplete" xattr was clearly part of an xfs_attr_set operation that
failed midway through because the induced log shutdown prevented xfs
from finishing the creation of a remote xattr.  This kind of thing is
expected, but instead xfs_repair deletes the entire attr fork!

It's far too drastic to delete every xattr because doing that destroys
things like security labels.  The kernel won't show incomplete attrs so
it's not a big deal to leave them attached to the file.  Note that
xfs_scrub can fix such things.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
v2: improve commit message to explain how we got to this acceptable
place
---
 repair/attr_repair.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 50159b9a533875..fe4089026cae74 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -571,7 +571,13 @@ process_leaf_attr_remote(
 	    !libxfs_attr_namecheck(entry->flags, remotep->name,
 				   remotep->namelen) ||
 	    be32_to_cpu(entry->hashval) != computed ||
-	    be32_to_cpu(entry->hashval) < last_hashval ||
+	    be32_to_cpu(entry->hashval) < last_hashval) {
+		do_warn(
+	_("inconsistent remote attribute entry %d in attr block %u, ino %" PRIu64 "\n"), i, da_bno, ino);
+		return -1;
+	}
+
+	if (!(entry->flags & XFS_ATTR_INCOMPLETE) &&
 	    be32_to_cpu(remotep->valueblk) == 0) {
 		do_warn(
 	_("inconsistent remote attribute entry %d in attr block %u, ino %" PRIu64 "\n"), i, da_bno, ino);
@@ -592,6 +598,9 @@ process_leaf_attr_remote(
 		return -1;
 	}
 
+	if (entry->flags & XFS_ATTR_INCOMPLETE)
+		goto out;
+
 	value = malloc(be32_to_cpu(remotep->valuelen));
 	if (value == NULL) {
 		do_warn(
@@ -708,12 +717,15 @@ process_leaf_attr_block(
 		}
 
 		if (entry->flags & XFS_ATTR_INCOMPLETE) {
-			/* we are inconsistent state. get rid of us */
-			do_warn(
+			/*
+			 * Warn about incomplete xattrs but don't zap the
+			 * entire attr fork because that causes loss of
+			 * security labels.  The kernel can handle stray
+			 * incomplete attr entries.
+			 */
+			do_log(
 	_("attribute entry #%d in attr block %u, inode %" PRIu64 " is INCOMPLETE\n"),
 				i, da_bno, ino);
-			clearit = 1;
-			break;
 		}
 
 		/* mark the entry used */

Re: [PATCH v2] xfs_repair: don't fail on INCOMPLETE attrs in leaf blocks

[Christoph Hellwig]

Looks good:

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