]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
xfs_scrub: remove moveon from filemap iteration
authorDarrick J. Wong <darrick.wong@oracle.com>
Wed, 6 Nov 2019 22:27:17 +0000 (17:27 -0500)
committerEric Sandeen <sandeen@sandeen.net>
Wed, 6 Nov 2019 22:27:17 +0000 (17:27 -0500)
Remove the moveon and descr clutter from filemap iteration in favor of
returning errors directly and passing error domain descriptions around
through the existing void *arg.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
scrub/filemap.c
scrub/filemap.h
scrub/phase6.c

index f92e962013c8785874ec807d6441f68cde1c352b..bad2e9e1bb149a16cf3812b0c0b197a8253b3e67 100644 (file)
 
 #define BMAP_NR                2048
 
-/* Iterate all the extent block mappings between the key and fork end. */
-bool
-xfs_iterate_filemaps(
+/*
+ * Iterate all the extent block mappings between the key and fork end.
+ * Returns 0 or a positive error number.
+ */
+int
+scrub_iterate_filemaps(
        struct scrub_ctx        *ctx,
-       const char              *descr,
        int                     fd,
        int                     whichfork,
-       struct xfs_bmap         *key,
-       xfs_bmap_iter_fn        fn,
+       struct file_bmap        *key,
+       scrub_bmap_iter_fn      fn,
        void                    *arg)
 {
        struct fsxattr          fsx;
        struct getbmapx         *map;
        struct getbmapx         *p;
-       struct xfs_bmap         bmap;
-       char                    bmap_descr[DESCR_BUFSZ];
-       bool                    moveon = true;
+       struct file_bmap        bmap;
        xfs_off_t               new_off;
        int                     getxattr_type;
        int                     i;
-       int                     error;
-
-       switch (whichfork) {
-       case XFS_ATTR_FORK:
-               snprintf(bmap_descr, DESCR_BUFSZ, _("%s attr"), descr);
-               break;
-       case XFS_COW_FORK:
-               snprintf(bmap_descr, DESCR_BUFSZ, _("%s CoW"), descr);
-               break;
-       case XFS_DATA_FORK:
-               snprintf(bmap_descr, DESCR_BUFSZ, _("%s data"), descr);
-               break;
-       default:
-               abort();
-       }
+       int                     ret;
 
        map = calloc(BMAP_NR, sizeof(struct getbmapx));
-       if (!map) {
-               str_errno(ctx, bmap_descr);
-               return false;
-       }
+       if (!map)
+               return errno;
 
        map->bmv_offset = BTOBB(key->bm_offset);
        map->bmv_block = BTOBB(key->bm_physical);
@@ -89,34 +73,28 @@ xfs_iterate_filemaps(
                abort();
        }
 
-       error = ioctl(fd, getxattr_type, &fsx);
-       if (error < 0) {
-               str_errno(ctx, bmap_descr);
-               moveon = false;
+       ret = ioctl(fd, getxattr_type, &fsx);
+       if (ret < 0) {
+               ret = errno;
                goto out;
        }
 
-       if (fsx.fsx_nextents == 0) {
-               moveon = true;
+       if (fsx.fsx_nextents == 0)
                goto out;
-       }
 
        map->bmv_count = min(fsx.fsx_nextents + 1, BMAP_NR);
 
-       while ((error = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) {
+       while ((ret = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) {
                for (i = 0, p = &map[i + 1]; i < map->bmv_entries; i++, p++) {
                        bmap.bm_offset = BBTOB(p->bmv_offset);
                        bmap.bm_physical = BBTOB(p->bmv_block);
                        bmap.bm_length = BBTOB(p->bmv_length);
                        bmap.bm_flags = p->bmv_oflags;
-                       moveon = fn(ctx, bmap_descr, fd, whichfork, &fsx,
-                                       &bmap, arg);
-                       if (!moveon)
+                       ret = fn(ctx, fd, whichfork, &fsx, &bmap, arg);
+                       if (ret)
                                goto out;
-                       if (xfs_scrub_excessive_errors(ctx)) {
-                               moveon = false;
+                       if (xfs_scrub_excessive_errors(ctx))
                                goto out;
-                       }
                }
 
                if (map->bmv_entries == 0)
@@ -129,17 +107,16 @@ xfs_iterate_filemaps(
                map->bmv_length -= new_off - map->bmv_offset;
                map->bmv_offset = new_off;
        }
+       if (ret < 0)
+               ret = errno;
 
        /*
         * Pre-reflink filesystems don't know about CoW forks, so don't
         * be too surprised if it fails.
         */
-       if (whichfork == XFS_COW_FORK && error && errno == EINVAL)
-               error = 0;
-
-       if (error)
-               str_errno(ctx, bmap_descr);
+       if (whichfork == XFS_COW_FORK && ret == EINVAL)
+               ret = 0;
 out:
        free(map);
-       return moveon;
+       return ret;
 }
index 219be57511f050ce037f5accd8e21e4b835fad63..133e860bb82676f15252369fb061423b5144369c 100644 (file)
@@ -7,7 +7,7 @@
 #define XFS_SCRUB_FILEMAP_H_
 
 /* inode fork block mapping */
-struct xfs_bmap {
+struct file_bmap {
        uint64_t        bm_offset;      /* file offset of segment in bytes */
        uint64_t        bm_physical;    /* physical starting byte  */
        uint64_t        bm_length;      /* length of segment, bytes */
@@ -15,15 +15,13 @@ struct xfs_bmap {
 };
 
 /*
- * Visit each inode fork mapping.  Return true to continue iteration or false
- * to stop iterating and return to the caller.
+ * Visit each inode fork mapping.  Return 0 to continue iteration or a positive
+ * error code to stop iterating and return to the caller.
  */
-typedef bool (*xfs_bmap_iter_fn)(struct scrub_ctx *ctx, const char *descr,
-               int fd, int whichfork, struct fsxattr *fsx,
-               struct xfs_bmap *bmap, void *arg);
+typedef int (*scrub_bmap_iter_fn)(struct scrub_ctx *ctx, int fd, int whichfork,
+               struct fsxattr *fsx, struct file_bmap *bmap, void *arg);
 
-bool xfs_iterate_filemaps(struct scrub_ctx *ctx, const char *descr, int fd,
-               int whichfork, struct xfs_bmap *key, xfs_bmap_iter_fn fn,
-               void *arg);
+int scrub_iterate_filemaps(struct scrub_ctx *ctx, int fd, int whichfork,
+               struct file_bmap *key, scrub_bmap_iter_fn fn, void *arg);
 
 #endif /* XFS_SCRUB_FILEMAP_H_ */
index aae6b7d80fbddb86056403c6f412bee9262638e1..f57ce339fa060ad58cae27fae107192cddb97765 100644 (file)
@@ -133,9 +133,10 @@ xfs_decode_special_owner(
 /* Routines to translate bad physical extents into file paths and offsets. */
 
 struct badfile_report {
-       struct scrub_ctx        *ctx;
-       const char              *descr;
-       struct xfs_bmap         *bmap;
+       struct scrub_ctx                *ctx;
+       const char                      *descr;
+       struct media_verify_state       *vs;
+       struct file_bmap                *bmap;
 };
 
 /* Report on bad extents found during a media scan. */
@@ -168,75 +169,66 @@ _("media error at data offset %llu length %llu."),
 }
 
 /* Report if this extent overlaps a bad region. */
-static bool
+static int
 report_data_loss(
        struct scrub_ctx                *ctx,
-       const char                      *descr,
        int                             fd,
        int                             whichfork,
        struct fsxattr                  *fsx,
-       struct xfs_bmap                 *bmap,
+       struct file_bmap                *bmap,
        void                            *arg)
 {
-       struct badfile_report           br = {
-               .ctx                    = ctx,
-               .descr                  = descr,
-               .bmap                   = bmap,
-       };
-       struct media_verify_state       *vs = arg;
+       struct badfile_report           *br = arg;
+       struct media_verify_state       *vs = br->vs;
        struct bitmap                   *bmp;
-       int                             ret;
+
+       br->bmap = bmap;
 
        /* Only report errors for real extents. */
        if (bmap->bm_flags & (BMV_OF_PREALLOC | BMV_OF_DELALLOC))
-               return true;
+               return 0;
 
        if (fsx->fsx_xflags & FS_XFLAG_REALTIME)
                bmp = vs->r_bad;
        else
                bmp = vs->d_bad;
 
-       ret = bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
-                       report_badfile, &br);
-       if (ret) {
-               str_liberror(ctx, ret, descr);
-               return false;
-       }
-       return true;
+       return bitmap_iterate_range(bmp, bmap->bm_physical, bmap->bm_length,
+                       report_badfile, br);
 }
 
 /* Report if the extended attribute data overlaps a bad region. */
-static bool
+static int
 report_attr_loss(
        struct scrub_ctx                *ctx,
-       const char                      *descr,
        int                             fd,
        int                             whichfork,
        struct fsxattr                  *fsx,
-       struct xfs_bmap                 *bmap,
+       struct file_bmap                *bmap,
        void                            *arg)
 {
-       struct media_verify_state       *vs = arg;
+       struct badfile_report           *br = arg;
+       struct media_verify_state       *vs = br->vs;
        struct bitmap                   *bmp = vs->d_bad;
 
        /* Complain about attr fork extents that don't look right. */
        if (bmap->bm_flags & (BMV_OF_PREALLOC | BMV_OF_DELALLOC)) {
-               str_info(ctx, descr,
+               str_info(ctx, br->descr,
 _("found unexpected unwritten/delalloc attr fork extent."));
-               return true;
+               return 0;
        }
 
        if (fsx->fsx_xflags & FS_XFLAG_REALTIME) {
-               str_info(ctx, descr,
+               str_info(ctx, br->descr,
 _("found unexpected realtime attr fork extent."));
-               return true;
+               return 0;
        }
 
        if (bitmap_test(bmp, bmap->bm_physical, bmap->bm_length))
-               str_corrupt(ctx, descr,
+               str_corrupt(ctx, br->descr,
 _("media error in extended attribute data."));
 
-       return true;
+       return 0;
 }
 
 /* Iterate the extent mappings of a file to report errors. */
@@ -247,18 +239,30 @@ xfs_report_verify_fd(
        int                             fd,
        void                            *arg)
 {
-       struct xfs_bmap                 key = {0};
-       bool                            moveon;
+       struct badfile_report           br = {
+               .ctx                    = ctx,
+               .vs                     = arg,
+               .descr                  = descr,
+       };
+       struct file_bmap                key = {0};
+       int                             ret;
 
        /* data fork */
-       moveon = xfs_iterate_filemaps(ctx, descr, fd, XFS_DATA_FORK, &key,
-                       report_data_loss, arg);
-       if (!moveon)
+       ret = scrub_iterate_filemaps(ctx, fd, XFS_DATA_FORK, &key,
+                       report_data_loss, &br);
+       if (ret) {
+               str_liberror(ctx, ret, descr);
                return false;
+       }
 
        /* attr fork */
-       return xfs_iterate_filemaps(ctx, descr, fd, XFS_ATTR_FORK, &key,
-                       report_attr_loss, arg);
+       ret = scrub_iterate_filemaps(ctx, fd, XFS_ATTR_FORK, &key,
+                       report_attr_loss, &br);
+       if (ret) {
+               str_liberror(ctx, ret, descr);
+               return false;
+       }
+       return true;
 }
 
 /* Report read verify errors in unlinked (but still open) files. */