diff mbox series

[4/4] fuzzy: don't fail on compressed metadumps

Message ID 167096073394.1750373.2942809607367883189.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series fstests: fix broken fuzzing xfs_mdrestore calls | expand

Commit Message

Darrick J. Wong Dec. 13, 2022, 7:45 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

This line in __scratch_xfs_fuzz_mdrestore:

	test -e "${POPULATE_METADUMP}"

Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
the metadump files, because the metadump files get the compression
program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
is wrong, and since the naming policy is an implementation detail of
_xfs_mdrestore, let's get rid of the -e test.

However, we still need a way to fail the test if the metadump cannot be
restored.  _xfs_mdrestore returns nonzero on failure, so use that
instead.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/fuzzy |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Zorro Lang Dec. 17, 2022, 7:03 a.m. UTC | #1 
On Tue, Dec 13, 2022 at 11:45:33AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This line in __scratch_xfs_fuzz_mdrestore:
> 
> 	test -e "${POPULATE_METADUMP}"
> 
> Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
> the metadump files, because the metadump files get the compression
> program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
> is wrong, and since the naming policy is an implementation detail of
> _xfs_mdrestore, let's get rid of the -e test.
> 
> However, we still need a way to fail the test if the metadump cannot be
> restored.  _xfs_mdrestore returns nonzero on failure, so use that
> instead.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/fuzzy |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/common/fuzzy b/common/fuzzy
> index e634815eec..49c850f2d5 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
>  # Restore metadata to scratch device prior to field-fuzzing.
>  __scratch_xfs_fuzz_mdrestore()
>  {
> -	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> -
>  	__scratch_xfs_fuzz_unmount
> -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \
> +		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
>  }
>  
>  __fuzz_notify() {
>
Zorro Lang Dec. 17, 2022, 10:33 a.m. UTC | #2 
On Sat, Dec 17, 2022 at 03:03:29PM +0800, Zorro Lang wrote:
> On Tue, Dec 13, 2022 at 11:45:33AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > This line in __scratch_xfs_fuzz_mdrestore:
> > 
> > 	test -e "${POPULATE_METADUMP}"
> > 
> > Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
> > the metadump files, because the metadump files get the compression
> > program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
> > is wrong, and since the naming policy is an implementation detail of
> > _xfs_mdrestore, let's get rid of the -e test.
> > 
> > However, we still need a way to fail the test if the metadump cannot be
> > restored.  _xfs_mdrestore returns nonzero on failure, so use that
> > instead.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> Looks good to me,
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> >  common/fuzzy |    5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/common/fuzzy b/common/fuzzy
> > index e634815eec..49c850f2d5 100644
> > --- a/common/fuzzy
> > +++ b/common/fuzzy
> > @@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
> >  # Restore metadata to scratch device prior to field-fuzzing.
> >  __scratch_xfs_fuzz_mdrestore()
> >  {
> > -	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> > -
> >  	__scratch_xfs_fuzz_unmount
> > -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> > +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \

FYI, I've also removed the "compress" parameter according to:
  [PATCH v1.1 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore
When I merged this patch.

> > +		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
> >  }
> >  
> >  __fuzz_notify() {
> >
Darrick J. Wong Dec. 17, 2022, 8:06 p.m. UTC | #3 
On Sat, Dec 17, 2022 at 06:33:33PM +0800, Zorro Lang wrote:
> On Sat, Dec 17, 2022 at 03:03:29PM +0800, Zorro Lang wrote:
> > On Tue, Dec 13, 2022 at 11:45:33AM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > This line in __scratch_xfs_fuzz_mdrestore:
> > > 
> > > 	test -e "${POPULATE_METADUMP}"
> > > 
> > > Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
> > > the metadump files, because the metadump files get the compression
> > > program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
> > > is wrong, and since the naming policy is an implementation detail of
> > > _xfs_mdrestore, let's get rid of the -e test.
> > > 
> > > However, we still need a way to fail the test if the metadump cannot be
> > > restored.  _xfs_mdrestore returns nonzero on failure, so use that
> > > instead.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > 
> > Looks good to me,
> > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > 
> > >  common/fuzzy |    5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > 
> > > diff --git a/common/fuzzy b/common/fuzzy
> > > index e634815eec..49c850f2d5 100644
> > > --- a/common/fuzzy
> > > +++ b/common/fuzzy
> > > @@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
> > >  # Restore metadata to scratch device prior to field-fuzzing.
> > >  __scratch_xfs_fuzz_mdrestore()
> > >  {
> > > -	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> > > -
> > >  	__scratch_xfs_fuzz_unmount
> > > -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> > > +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \
> 
> FYI, I've also removed the "compress" parameter according to:
>   [PATCH v1.1 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore
> When I merged this patch.

Thank you!  Sorry for the mess. :/

--D

> > > +		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
> > >  }
> > >  
> > >  __fuzz_notify() {
> > > 
>
diff mbox series

Patch

diff --git a/common/fuzzy b/common/fuzzy
index e634815eec..49c850f2d5 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -156,10 +156,9 @@  __scratch_xfs_fuzz_unmount()
 # Restore metadata to scratch device prior to field-fuzzing.
 __scratch_xfs_fuzz_mdrestore()
 {
-	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
-
 	__scratch_xfs_fuzz_unmount
-	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
+	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \
+		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
 }
 
 __fuzz_notify() {