[PATCH 0/1] xfs_fsr fixes

[PATCH 0/1] xfs_fsr fixes

[Eric Sandeen]

This is to fix the problem w/ xfs_fsr demonstrated by the
testcase I just sent.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfs_fsr: extra debugging info

[Eric Sandeen]

Provide 2 new pieces of information when -d is specified to
xfs_fsr:

* If we needed to grow the forkoffset and couldn't
* If we were unable to match the fork offset

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 7596834..c949f07 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1022,6 +1022,7 @@ fsr_setup_attr_fork(
 {
 	struct stat64	tstatbuf;
 	int		i;
+	int		diff = 0;
 	int		last_forkoff = 0;
 	int		no_change_cnt = 0;
 	int		ret;
@@ -1057,7 +1058,6 @@ fsr_setup_attr_fork(
 		xfs_bstat_t	tbstat;
 		xfs_ino_t	ino;
 		char		name[64];
-		int		diff;
 
 		/*
 		 * bulkstat the temp inode  to see what the forkoff is. Use
@@ -1123,6 +1123,8 @@ fsr_setup_attr_fork(
 			 * non-contiguous offsets.
 			 */
 			/* XXX: unimplemented! */
+			if (dflag)
+				printf(_("data fork growth unimplemented\n"));
 			goto out;
 		}
 
@@ -1138,6 +1140,10 @@ fsr_setup_attr_fork(
 out:
 	if (dflag)
 		fsrprintf(_("set temp attr\n"));
+	/* We failed to resolve the fork difference */
+	if (dflag && diff)
+		fsrprintf(_("failed to match fork offset\n"));;
+
 	return 0;
 }
 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs_fsr: extra debugging info

[Christoph Hellwig]

On Fri, Oct 18, 2013 at 05:09:07PM -0500, Eric Sandeen wrote:
> Provide 2 new pieces of information when -d is specified to
> xfs_fsr:
> 
> * If we needed to grow the forkoffset and couldn't
> * If we were unable to match the fork offset
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs_fsr: extra debugging info

[Rich Johnston]

This has been committed.

Thanks
--Rich

commit 275077754611e5a15672624e674265b54f1cb7ed
Author: Eric Sandeen <sandeen@redhat.com>
Date:   Fri Oct 18 22:09:07 2013 +0000

     xfs_fsr: extra debugging info

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfs_fsr: fix SWAPEXT failures under selinux

[Eric Sandeen]

If we run xfs_fsr on a system which creates selinux extended
attributes, the temp file created by xfs_fsr may have a
large-ish local extended attribute as soon as it is created.

If the target file has NON-local extended attributes, it may
have a fork offset larger than the temp file, because i.e.
FMT_EXTENTS attributes take up less space.  We currently
have no mechanism to grow the temp file's fork offset.
So in this case, the SWAPEXT ioctl will fail.

(With systems using selinux and lots of xattrs, this becomes
fairly common in the real world.)

After testing the target file for a non-local extent, and
checking to see if the temp forkoff needs to be grown on the
first pass, we can add a large attr to knock all attributes on
the temp file out of local format, and grow the fork offset for
this particular case.

This passes xfstest 227, and also resolves issues seen on
a metadata image provided by Gabriel.

Reported-by: Gabriel VLASIU <gabriel@vlasiu.net>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index c949f07..6f00b41 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1060,7 +1060,7 @@ fsr_setup_attr_fork(
 		char		name[64];
 
 		/*
-		 * bulkstat the temp inode  to see what the forkoff is. Use
+		 * bulkstat the temp inode to see what the forkoff is.  Use
 		 * this to compare against the target and determine what we
 		 * need to do.
 		 */
@@ -1073,6 +1073,11 @@ fsr_setup_attr_fork(
 		if (dflag)
 			fsrprintf(_("orig forkoff %d, temp forkoff %d\n"),
 					bstatp->bs_forkoff, tbstat.bs_forkoff);
+		diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
+
+		/* if they are equal, we are done */
+		if (!diff)
+			goto out;
 
 		snprintf(name, sizeof(name), "user.%d", i);
 
@@ -1081,12 +1086,62 @@ fsr_setup_attr_fork(
 		 * an attribute fork at the default location.
 		 */
 		if (!tbstat.bs_forkoff) {
+			ASSERT(i == 0);
 			ret = fsetxattr(tfd, name, "XX", 2, XATTR_CREATE);
 			if (ret) {
 				fsrprintf(_("could not set ATTR\n"));
 				return -1;
 			}
 			continue;
+		} else if (i == 0) {
+			struct fsxattr	fsx;
+			/*
+			 * First pass, and temp file already has an inline
+			 * xattr, probably due to selinux.
+			 *
+			 * It's *possible* that the temp file attr area
+			 * is larger than the target file's, if the 
+			 * target file's attrs are not inline:
+			 *
+			 *  Target		 Temp
+			 * +-------+ 0		+-------+ 0
+			 * |	   |		|       |
+			 * |	   |		| Data  |
+			 * | Data  |		|       |
+			 * |	   |		v-------v forkoff
+			 * |	   |		|       |
+			 * v-------v forkoff	| Attr  | local
+			 * | Attr  | ext/btree	|       |
+			 * +-------+		+-------+
+			 *
+			 * FSGETXATTRA will tell us nr of attr extents in
+			 * target, if any.  If none, it's local:
+			 */
+
+			memset(&fsx, 0, sizeof(fsx));
+			if (ioctl(fd, XFS_IOC_FSGETXATTRA, &fsx)) {
+				fsrprintf(_("FSGETXATTRA failed on target\n"));
+				return -1;
+			}
+
+			/*
+			 * If target attr area is less than the temp's (diff < 0)
+			 * and the target is not local, write a big attr to
+			 * the temp file to knock the attr out of local format,
+			 * to match the target.  (This should actually *increase*
+			 * the temp file's forkoffset when the attr moves out
+			 * of the inode)
+			 */
+ 			if (diff < 0 && fsx.fsx_nextents > 0) {
+				char val[2048];
+				memset(val, 'X', 2048);
+				if (fsetxattr(tfd, name, val, 2048, 0)) {
+					fsrprintf(_("big ATTR set failed\n"));
+					return -1;
+				}
+				/* Go back & see where we're at now */
+				continue;
+			}
 		}
 
 		/*
@@ -1101,19 +1156,14 @@ fsr_setup_attr_fork(
 		last_forkoff = tbstat.bs_forkoff;
 
 		/* work out which way to grow the fork */
-		diff = tbstat.bs_forkoff - bstatp->bs_forkoff;
 		if (abs(diff) > fsgeom.inodesize - sizeof(struct xfs_dinode)) {
 			fsrprintf(_("forkoff diff %d too large!\n"), diff);
 			return -1;
 		}
 
-		/* if they are equal, we are done */
-		if (!diff)
-			goto out;
-
 		/*
-		 * if the temp inode fork offset is smaller then we have to
-		 * grow the data fork
+		 * if the temp inode fork offset is still smaller then we have
+		 * to grow the data fork
 		 */
 		if (diff < 0) {
 			/*

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs_fsr: fix SWAPEXT failures under selinux

[Christoph Hellwig]

Looks good,

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs_fsr: fix SWAPEXT failures under selinux

[Rich Johnston]

This has been committed.

Thanks
--Rich

commit 1adfe5c6296d3ea6c182f31a6728fc94af9146f7
Author: Eric Sandeen <sandeen@redhat.com>
Date:   Fri Oct 18 22:30:18 2013 +0000

     xfs_fsr: fix SWAPEXT failures under selinux

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/1] xfs_fsr fixes

[Eric Sandeen]

On 10/18/13, 5:07 PM, Eric Sandeen wrote:
> This is to fix the problem w/ xfs_fsr demonstrated by the
> testcase I just sent.

Ping?

> -Eric
> 
>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs