xfs_fsr: Improve handling of attribute forks V2

If the file being defragmented has attributes, then fsr puts a dummy
attribute on the temporary file to try to ensure that the inode
attribute fork offset is set correctly. This works perfectly well
for the old style of attributes that use a fixed fork offset - the
presence of any attribute of any size or shape will result in fsr
doing the correct thing.

However, for attr2 filesystems, the attribute fork offset is
dependent on the size and shape of both the data and attribute
forks. Hence setting a small attribute on the file does not
guarantee that the two inodes have the same fork offset and
therefore compatible for a data fork swap.

This patch improves the attribute fork handling of fsr. It checks
the filesystem version to see if the old style attributes are in
use, and if so uses the current method.

If attr2 is in use, fsr uses bulkstat output to determine what the
fork offset is. If the attribute fork offsets differ then fsr will
try to create attributes that will result in the correct offset. If
that fails, or the attribute fork is too large, it will give up and just
attempt the swap.

This fork offset value in bulkstat new functionality in the kernel,
so if there are attributes and a zero fork offset, then the kernel
does not support this feature and we simply fall back to the existing,
less effective code.

Version 2:
- simplify the attribute creation to use a small fixed size attribute
- handle the fork offset not changing as attributes are added - it can take a
  few attributes to move it from one offset to another
- comment the code better
- passes test 226 and reduces the number of unswappable inode pairs passed to
  the (fixed) kernel to zero

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

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 1f933c77487f8d567b304b4da9fe69cbcabcc267..56196767f108f48acc6584861660dd1cef3c64ff 100644 (file)
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -17,8 +17,13 @@
  */
 
 #include <xfs/xfs.h>
+#include <libxfs.h>
+#include <xfs/xfs_types.h>
 #include <xfs/jdm.h>
 #include <xfs/xfs_dfrag.h>
+#include <xfs/xfs_bmap_btree.h>
+#include <xfs/xfs_dinode.h>
+#include <xfs/xfs_attr_sf.h>
 
 #include <fcntl.h>
 #include <errno.h>
@@ -946,6 +951,147 @@ fsrfile_common(
        return -1; /* no error */
 }
 
+/*
+ * Attempt to set the attr fork up correctly. This is simple for attr1
+ * filesystems as they have a fixed inode fork offset. In that case
+ * just create an attribute and that's all we need to do.
+ *
+ * For attr2 filesystems, see if we have the actual fork offset in
+ * the bstat structure. If so, just create additional attributes on
+ * the temporary inode until the offset matches.
+ *
+ * If it doesn't exist, we can only do best effort. Add an attribute at a time
+ * to move the inode fork around, but take into account that the attribute
+ * might be too small to move the fork every time we add one.  This should
+ * hopefully put the fork offset in the right place. It's not a big deal if we
+ * don't get it right - the kernel will reject it when we try to swap extents.
+ */
+static int
+fsr_setup_attr_fork(
+       int             fd,
+       int             tfd,
+       xfs_bstat_t     *bstatp)
+{
+       struct stat64   tstatbuf;
+       int             i;
+       int             last_forkoff = 0;
+       int             no_change_cnt = 0;
+       int             ret;
+
+       if (!(bstatp->bs_xflags & XFS_XFLAG_HASATTR))
+               return 0;
+
+       /*
+        * use the old method if we have attr1 or the kernel does not yet
+        * support passing the fork offset in the bulkstat data.
+        */
+       if (!(fsgeom.flags & XFS_FSOP_GEOM_FLAGS_ATTR2) ||
+           bstatp->bs_forkoff == 0) {
+               /* attr1 */
+               ret = fsetxattr(tfd, "user.X", "X", 1, XATTR_CREATE);
+               if (ret) {
+                       fsrprintf(_("could not set ATTR\n"));
+                       return -1;
+               }
+               goto out;
+       }
+
+       /* attr2 w/ fork offsets */
+
+       if (fstat64(tfd, &tstatbuf) < 0) {
+               fsrprintf(_("unable to stat temp file: %s\n"),
+                                       strerror(errno));
+               return -1;
+       }
+
+       i = 0;
+       do {
+               xfs_bstat_t     tbstat;
+               xfs_ino_t       ino;
+               char            name[64];
+               int             diff;
+
+               /*
+                * bulkstat the temp inode  to see what the forkoff is. Use
+                * this to compare against the target and determine what we
+                * need to do.
+                */
+               ino = tstatbuf.st_ino;
+               if ((xfs_bulkstat_single(tfd, &ino, &tbstat)) < 0) {
+                       fsrprintf(_("unable to get bstat on temp file: %s\n"),
+                                               strerror(errno));
+                       return -1;
+               }
+               if (dflag)
+                       fsrprintf(_("orig forkoff %d, temp forkoff %d\n"),
+                                       bstatp->bs_forkoff, tbstat.bs_forkoff);
+
+               snprintf(name, sizeof(name), "user.%d", i);
+
+               /*
+                * If there is no attribute, then we need to create one to get
+                * an attribute fork at the default location.
+                */
+               if (!tbstat.bs_forkoff) {
+                       ret = fsetxattr(tfd, name, "XX", 2, XATTR_CREATE);
+                       if (ret) {
+                               fsrprintf(_("could not set ATTR\n"));
+                               return -1;
+                       }
+                       continue;
+               }
+
+               /*
+                * make a progress check so we don't get stuck trying to extend
+                * a large btree form attribute fork.
+                */
+               if (last_forkoff == tbstat.bs_forkoff) {
+                       if (no_change_cnt++ > 10)
+                               break;
+               }
+               no_change_cnt = 0;
+               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 (diff < 0) {
+                       /*
+                        * create some temporary extents in the inode to move
+                        * the fork in the direction we need. This can be done
+                        * by preallocating some single block extents at
+                        * non-contiguous offsets.
+                        */
+                       /* XXX: unimplemented! */
+                       goto out;
+               }
+
+               /* we need to grow the attr fork, so create another attr */
+               ret = fsetxattr(tfd, name, "XX", 2, XATTR_CREATE);
+               if (ret) {
+                       fsrprintf(_("could not set ATTR\n"));
+                       return -1;
+               }
+
+       } while (++i < 100); /* don't go forever */
+
+out:
+       if (dflag)
+               fsrprintf(_("set temp attr\n"));
+       return 0;
+}
 
 /*
  * Do the defragmentation of a single file.
@@ -1000,14 +1146,10 @@ packfile(char *fname, char *tname, int fd,
        unlink(tname);
 
        /* Setup extended attributes */
-       if (statp->bs_xflags & XFS_XFLAG_HASATTR) {
-               if (fsetxattr(tfd, "user.X", "X", 1, XATTR_CREATE) != 0) {
-                       fsrprintf(_("could not set ATTR on tmp: %s:\n"), tname);
-                       close(tfd);
-                       return -1;
-               }
-               if (dflag)
-                       fsrprintf(_("%s set temp attr\n"), tname);
+       if (fsr_setup_attr_fork(fd, tfd, statp) != 0) {
+               fsrprintf(_("failed to set ATTR fork on tmp: %s:\n"), tname);
+               close(tfd);
+               return -1;
        }
 
        /* Setup extended inode flags, project identifier, etc */

diff --git a/include/xfs_fs.h b/include/xfs_fs.h
index 9aff7bb46574fa0b405db99793694b786cc96b24..2376abb05f525aa25f12fcb50561d977ef78053c 100644 (file)
--- a/include/xfs_fs.h
+++ b/include/xfs_fs.h
@@ -300,7 +300,8 @@ typedef struct xfs_bstat {
        __s32           bs_extents;     /* number of extents            */
        __u32           bs_gen;         /* generation count             */
        __u16           bs_projid;      /* project id                   */
-       unsigned char   bs_pad[14];     /* pad space, unused            */
+       __u16           bs_forkoff;     /* inode fork offset in bytes   */
+       unsigned char   bs_pad[12];     /* pad space, unused            */
        __u32           bs_dmevmask;    /* DMIG event mask              */
        __u16           bs_dmstate;     /* DMIG state info              */
        __u16           bs_aextents;    /* attribute number of extents  */