xfs: move all scrub input checking to xfs_scrub_validate

There were ad-hoc checks for some scrub types but not others;
mark each scrub type with ... it's type, and use that to validate
the allowed and/or required input fields.

Moving these checks out of xfs_scrub_setup_ag_header makes it
a thin wrapper, so unwrap it in the process.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
[darrick: add xfs_ prefix to enum, check scrub args after checking type]
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 2a9b4f9e93c64be83edad15c0e19edb45269f406..b599358c379659d4d832cf3cd7e5407ae8f1a60c 100644 (file)
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -37,23 +37,6 @@
 #include "scrub/common.h"
 #include "scrub/trace.h"
 
-/*
- * Set up scrub to check all the static metadata in each AG.
- * This means the SB, AGF, AGI, and AGFL headers.
- */
-int
-xfs_scrub_setup_ag_header(
-       struct xfs_scrub_context        *sc,
-       struct xfs_inode                *ip)
-{
-       struct xfs_mount                *mp = sc->mp;
-
-       if (sc->sm->sm_agno >= mp->m_sb.sb_agcount ||
-           sc->sm->sm_ino || sc->sm->sm_gen)
-               return -EINVAL;
-       return xfs_scrub_setup_fs(sc, ip);
-}
-
 /* Walk all the blocks in the AGFL. */
 int
 xfs_scrub_walk_agfl(

diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index ac95fe911d96aacfd545aaa0ff594f3c783e5009..98452ad58cffec9cf5ebabb3a6f2ebe843575e7d 100644 (file)
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -472,7 +472,7 @@ xfs_scrub_setup_ag_btree(
                        return error;
        }
 
-       error = xfs_scrub_setup_ag_header(sc, ip);
+       error = xfs_scrub_setup_fs(sc, ip);
        if (error)
                return error;
 
@@ -507,14 +507,6 @@ xfs_scrub_get_inode(
        struct xfs_inode                *ip = NULL;
        int                             error;
 
-       /*
-        * If userspace passed us an AG number or a generation number
-        * without an inode number, they haven't got a clue so bail out
-        * immediately.
-        */
-       if (sc->sm->sm_agno || (sc->sm->sm_gen && !sc->sm->sm_ino))
-               return -EINVAL;
-
        /* We want to scan the inode we already had opened. */
        if (sc->sm->sm_ino == 0 || sc->sm->sm_ino == ip_in->i_ino) {
                sc->ip = ip_in;

diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index 5c043855570e43b0eaf11f52e22e57d985040ab0..fe12053aa0e7740a5886e592076dadb8fef333dd 100644 (file)
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -78,8 +78,6 @@ int xfs_scrub_checkpoint_log(struct xfs_mount *mp);
 
 /* Setup functions */
 int xfs_scrub_setup_fs(struct xfs_scrub_context *sc, struct xfs_inode *ip);
-int xfs_scrub_setup_ag_header(struct xfs_scrub_context *sc,
-                             struct xfs_inode *ip);
 int xfs_scrub_setup_ag_allocbt(struct xfs_scrub_context *sc,
                               struct xfs_inode *ip);
 int xfs_scrub_setup_ag_iallocbt(struct xfs_scrub_context *sc,

diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index 3d9037eceaf1b81c2848e056bdc8e9309bc4d87f..51daa4ae2627af7b4f8988ecf423d0680b9f519a 100644 (file)
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -67,13 +67,6 @@ xfs_scrub_setup_quota(
 {
        uint                            dqtype;
 
-       /*
-        * If userspace gave us an AG number or inode data, they don't
-        * know what they're doing.  Get out.
-        */
-       if (sc->sm->sm_agno || sc->sm->sm_ino || sc->sm->sm_gen)
-               return -EINVAL;
-
        dqtype = xfs_scrub_quota_to_dqtype(sc);
        if (dqtype == 0)
                return -EINVAL;

diff --git a/fs/xfs/scrub/rtbitmap.c b/fs/xfs/scrub/rtbitmap.c
index c6fedb698008146b6842a511d49c6f7dbb1f9eb2..6860d5d925157077ba3a223374a9642f721cacda 100644 (file)
--- a/fs/xfs/scrub/rtbitmap.c
+++ b/fs/xfs/scrub/rtbitmap.c
@@ -43,22 +43,14 @@ xfs_scrub_setup_rt(
        struct xfs_scrub_context        *sc,
        struct xfs_inode                *ip)
 {
-       struct xfs_mount                *mp = sc->mp;
-       int                             error = 0;
-
-       /*
-        * If userspace gave us an AG number or inode data, they don't
-        * know what they're doing.  Get out.
-        */
-       if (sc->sm->sm_agno || sc->sm->sm_ino || sc->sm->sm_gen)
-               return -EINVAL;
+       int                             error;
 
        error = xfs_scrub_setup_fs(sc, ip);
        if (error)
                return error;
 
        sc->ilock_flags = XFS_ILOCK_EXCL | XFS_ILOCK_RTBITMAP;
-       sc->ip = mp->m_rbmip;
+       sc->ip = sc->mp->m_rbmip;
        xfs_ilock(sc->ip, sc->ilock_flags);
 
        return 0;

diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index b0667420fcd573bdf8efba0fc5762f4cb13f7a8c..cd4607782a19bbd155bb1b0763dcc9da71eb6530 100644 (file)
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -128,8 +128,6 @@ xfs_scrub_probe(
 {
        int                             error = 0;
 
-       if (sc->sm->sm_ino || sc->sm->sm_agno)
-               return -EINVAL;
        if (xfs_scrub_should_terminate(sc, &error))
                return error;
 
@@ -168,105 +166,129 @@ xfs_scrub_teardown(
 
 static const struct xfs_scrub_meta_ops meta_scrub_ops[] = {
        [XFS_SCRUB_TYPE_PROBE] = {      /* ioctl presence test */
+               .type   = ST_NONE,
                .setup  = xfs_scrub_setup_fs,
                .scrub  = xfs_scrub_probe,
        },
        [XFS_SCRUB_TYPE_SB] = {         /* superblock */
-               .setup  = xfs_scrub_setup_ag_header,
+               .type   = ST_PERAG,
+               .setup  = xfs_scrub_setup_fs,
                .scrub  = xfs_scrub_superblock,
        },
        [XFS_SCRUB_TYPE_AGF] = {        /* agf */
-               .setup  = xfs_scrub_setup_ag_header,
+               .type   = ST_PERAG,
+               .setup  = xfs_scrub_setup_fs,
                .scrub  = xfs_scrub_agf,
        },
        [XFS_SCRUB_TYPE_AGFL]= {        /* agfl */
-               .setup  = xfs_scrub_setup_ag_header,
+               .type   = ST_PERAG,
+               .setup  = xfs_scrub_setup_fs,
                .scrub  = xfs_scrub_agfl,
        },
        [XFS_SCRUB_TYPE_AGI] = {        /* agi */
-               .setup  = xfs_scrub_setup_ag_header,
+               .type   = ST_PERAG,
+               .setup  = xfs_scrub_setup_fs,
                .scrub  = xfs_scrub_agi,
        },
        [XFS_SCRUB_TYPE_BNOBT] = {      /* bnobt */
+               .type   = ST_PERAG,
                .setup  = xfs_scrub_setup_ag_allocbt,
                .scrub  = xfs_scrub_bnobt,
        },
        [XFS_SCRUB_TYPE_CNTBT] = {      /* cntbt */
+               .type   = ST_PERAG,
                .setup  = xfs_scrub_setup_ag_allocbt,
                .scrub  = xfs_scrub_cntbt,
        },
        [XFS_SCRUB_TYPE_INOBT] = {      /* inobt */
+               .type   = ST_PERAG,
                .setup  = xfs_scrub_setup_ag_iallocbt,
                .scrub  = xfs_scrub_inobt,
        },
        [XFS_SCRUB_TYPE_FINOBT] = {     /* finobt */
+               .type   = ST_PERAG,
                .setup  = xfs_scrub_setup_ag_iallocbt,
                .scrub  = xfs_scrub_finobt,
                .has    = xfs_sb_version_hasfinobt,
        },
        [XFS_SCRUB_TYPE_RMAPBT] = {     /* rmapbt */
+               .type   = ST_PERAG,
                .setup  = xfs_scrub_setup_ag_rmapbt,
                .scrub  = xfs_scrub_rmapbt,
                .has    = xfs_sb_version_hasrmapbt,
        },
        [XFS_SCRUB_TYPE_REFCNTBT] = {   /* refcountbt */
+               .type   = ST_PERAG,
                .setup  = xfs_scrub_setup_ag_refcountbt,
                .scrub  = xfs_scrub_refcountbt,
                .has    = xfs_sb_version_hasreflink,
        },
        [XFS_SCRUB_TYPE_INODE] = {      /* inode record */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_inode,
                .scrub  = xfs_scrub_inode,
        },
        [XFS_SCRUB_TYPE_BMBTD] = {      /* inode data fork */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_inode_bmap,
                .scrub  = xfs_scrub_bmap_data,
        },
        [XFS_SCRUB_TYPE_BMBTA] = {      /* inode attr fork */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_inode_bmap,
                .scrub  = xfs_scrub_bmap_attr,
        },
        [XFS_SCRUB_TYPE_BMBTC] = {      /* inode CoW fork */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_inode_bmap,
                .scrub  = xfs_scrub_bmap_cow,
        },
        [XFS_SCRUB_TYPE_DIR] = {        /* directory */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_directory,
                .scrub  = xfs_scrub_directory,
        },
        [XFS_SCRUB_TYPE_XATTR] = {      /* extended attributes */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_xattr,
                .scrub  = xfs_scrub_xattr,
        },
        [XFS_SCRUB_TYPE_SYMLINK] = {    /* symbolic link */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_symlink,
                .scrub  = xfs_scrub_symlink,
        },
        [XFS_SCRUB_TYPE_PARENT] = {     /* parent pointers */
+               .type   = ST_INODE,
                .setup  = xfs_scrub_setup_parent,
                .scrub  = xfs_scrub_parent,
        },
        [XFS_SCRUB_TYPE_RTBITMAP] = {   /* realtime bitmap */
+               .type   = ST_FS,
                .setup  = xfs_scrub_setup_rt,
                .scrub  = xfs_scrub_rtbitmap,
                .has    = xfs_sb_version_hasrealtime,
        },
        [XFS_SCRUB_TYPE_RTSUM] = {      /* realtime summary */
+               .type   = ST_FS,
                .setup  = xfs_scrub_setup_rt,
                .scrub  = xfs_scrub_rtsummary,
                .has    = xfs_sb_version_hasrealtime,
        },
        [XFS_SCRUB_TYPE_UQUOTA] = {     /* user quota */
-               .setup = xfs_scrub_setup_quota,
-               .scrub = xfs_scrub_quota,
+               .type   = ST_FS,
+               .setup  = xfs_scrub_setup_quota,
+               .scrub  = xfs_scrub_quota,
        },
        [XFS_SCRUB_TYPE_GQUOTA] = {     /* group quota */
-               .setup = xfs_scrub_setup_quota,
-               .scrub = xfs_scrub_quota,
+               .type   = ST_FS,
+               .setup  = xfs_scrub_setup_quota,
+               .scrub  = xfs_scrub_quota,
        },
        [XFS_SCRUB_TYPE_PQUOTA] = {     /* project quota */
-               .setup = xfs_scrub_setup_quota,
-               .scrub = xfs_scrub_quota,
+               .type   = ST_FS,
+               .setup  = xfs_scrub_setup_quota,
+               .scrub  = xfs_scrub_quota,
        },
 };
 
@@ -297,6 +319,7 @@ xfs_scrub_validate_inputs(
        sm->sm_flags &= ~XFS_SCRUB_FLAGS_OUT;
        if (sm->sm_flags & ~XFS_SCRUB_FLAGS_IN)
                goto out;
+       /* sm_reserved[] must be zero */
        if (memchr_inv(sm->sm_reserved, 0, sizeof(sm->sm_reserved)))
                goto out;
 
@@ -311,6 +334,27 @@ xfs_scrub_validate_inputs(
        if (ops->has && !ops->has(&mp->m_sb))
                goto out;
 
+       error = -EINVAL;
+       /* restricting fields must be appropriate for type */
+       switch (ops->type) {
+       case ST_NONE:
+       case ST_FS:
+               if (sm->sm_ino || sm->sm_gen || sm->sm_agno)
+                       goto out;
+               break;
+       case ST_PERAG:
+               if (sm->sm_ino || sm->sm_gen ||
+                   sm->sm_agno >= mp->m_sb.sb_agcount)
+                       goto out;
+               break;
+       case ST_INODE:
+               if (sm->sm_agno || (sm->sm_gen && !sm->sm_ino))
+                       goto out;
+               break;
+       default:
+               goto out;
+       }
+
        error = -EOPNOTSUPP;
        /*
         * We won't scrub any filesystem that doesn't have the ability

diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
index e9ec041cf71374cfc80bec221089f4321e93a95d..2a7961405f0296866a5c6d54f4c04089cb3a16cc 100644 (file)
--- a/fs/xfs/scrub/scrub.h
+++ b/fs/xfs/scrub/scrub.h
@@ -22,6 +22,14 @@
 
 struct xfs_scrub_context;
 
+/* Type info and names for the scrub types. */
+enum xfs_scrub_type {
+       ST_NONE = 1,    /* disabled */
+       ST_PERAG,       /* per-AG metadata */
+       ST_FS,          /* per-FS metadata */
+       ST_INODE,       /* per-inode metadata */
+};
+
 struct xfs_scrub_meta_ops {
        /* Acquire whatever resources are needed for the operation. */
        int             (*setup)(struct xfs_scrub_context *,
@@ -32,6 +40,9 @@ struct xfs_scrub_meta_ops {
 
        /* Decide if we even have this piece of metadata. */
        bool            (*has)(struct xfs_sb *);
+
+       /* type describing required/allowed inputs */
+       enum xfs_scrub_type     type;
 };
 
 /* Buffer pointers and btree cursors for an entire AG. */