xfs
[Top] [All Lists]

Re: [PATCH 08/10] xfs_db: enable blocktrash for checksummed filesystems

To: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>, david@xxxxxxxxxxxxx
Subject: Re: [PATCH 08/10] xfs_db: enable blocktrash for checksummed filesystems
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Tue, 18 Aug 2015 14:26:37 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150815014429.1839.69933.stgit@xxxxxxxxxxxxxxxx>
References: <20150815014338.1839.37405.stgit@xxxxxxxxxxxxxxxx> <20150815014429.1839.69933.stgit@xxxxxxxxxxxxxxxx>
On 8/14/15 8:44 PM, Darrick J. Wong wrote:
> Disable the write verifiers when we're trashing a block.  With this
> in place, create a xfs fuzzer script that formats, populates, corrupts,
> tries to use, repairs, and tries again to use a crash test xfs image.
> Hopefully this will shake out some v5 filesystem bugs.

Maybe "we can create an xfs fuzzer script ..." (since it's not in this
patch)

> v2: Drop xfsfuzz, don't assume every block is an AGF when blocktrashing.
> Don't trash log blocks by default, because that skews the blocktrash
> heavily towards damaging only log blocks.

and skip DBM_SB by default as well, right?

And you added log blocks & symlinks to the allowed mask.

So I think something like:

Allow trashing of symlink & log blocks.
By default, do not trash superblocks (why?) or log blocks (because ...)

> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
>  db/check.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/db/check.c b/db/check.c
> index afeea32..965d0f5 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -944,6 +944,7 @@ blocktrash_b(
>       int             mask;
>       int             newbit;
>       int             offset;
> +     const struct xfs_buf_ops *stashed_ops;
>       static char     *modestr[] = {
>               N_("zeroed"), N_("set"), N_("flipped"), N_("randomized")
>       };
> @@ -952,8 +953,10 @@ blocktrash_b(
>       offset = (int)(random() % (int)(mp->m_sb.sb_blocksize * NBBY));
>       newbit = 0;
>       push_cur();
> -     set_cur(&typtab[DBM_UNKNOWN],
> +     set_cur(NULL,
>               XFS_AGB_TO_DADDR(mp, agno, agbno), blkbb, DB_RING_IGN, NULL);

Ok, you talked about this back on 5/28/15.  Weird.

But calling it with NULL is odd, too; nothing else does that.  What about 
TYP_NONE,

        { TYP_NONE, NULL }

its ops are NULL, as well... does that work?
Huh, ok, no callers w/ TYP_NONE, either.  I guess NULL works.

> +     stashed_ops = iocur_top->bp->b_ops;
> +     iocur_top->bp->b_ops = NULL;
>       if ((buf = iocur_top->data) == NULL) {
>               dbprintf(_("can't read block %u/%u for trashing\n"), agno, 
> agbno);
>               pop_cur();
> @@ -984,6 +987,7 @@ blocktrash_b(
>                       buf[byte] &= ~mask;
>       }
>       write_cur();
> +     iocur_top->bp->b_ops = stashed_ops;

*nod*

>       pop_cur();
>       printf(_("blocktrash: %u/%u %s block %d bit%s starting %d:%d %s\n"),
>               agno, agbno, typename[type], len, len == 1 ? "" : "s",
> @@ -1040,9 +1044,11 @@ blocktrash_f(
>                  (1 << DBM_BTINO) |
>                  (1 << DBM_DIR) |
>                  (1 << DBM_INODE) |
> +                (1 << DBM_LOG) |

Ok, so you allow log blocks to be specified,

>                  (1 << DBM_QUOTA) |
>                  (1 << DBM_RTBITMAP) |
>                  (1 << DBM_RTSUM) |
> +                (1 << DBM_SYMLINK) |

and symlink blocks too, but...

>                  (1 << DBM_SB);
>       while ((c = getopt(argc, argv, "0123n:s:t:x:y:")) != EOF) {
>               switch (c) {
> @@ -1106,7 +1112,7 @@ blocktrash_f(
>               return 0;
>       }
>       if (tmask == 0)
> -             tmask = goodmask;
> +             tmask = goodmask & ~((1 << DBM_LOG) | (1 << DBM_SB));

you disable log & superblocks by default if no mask is specified.

I'm not 100% sure why you want to change this, what did you run into,
in practice, if they were allowed?

If the change stays, then the xfs_db manpage needs an update:

"If no -t options are given then all metadata types can be trashed."

Thanks,
-Eric


>       lentab = xmalloc(sizeof(ltab_t));
>       lentab->min = lentab->max = min;
>       lentablen = 1;
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>