Re: [PATCH 6/8] punch: skip fpunch tests when op length not congruent with file allocation unit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 14, 2022 at 01:51:59AM +0800, Zorro Lang wrote:
> On Wed, Jul 13, 2022 at 10:33:05AM -0700, Darrick J. Wong wrote:
> > On Thu, Jul 14, 2022 at 01:04:26AM +0800, Zorro Lang wrote:
> > > On Tue, Jul 12, 2022 at 05:57:07PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > > 
> > > > Skip the generic fpunch tests on a file when the file's allocation unit
> > > > size is not congruent with the proposed testing operations.
> > > > 
> > > > This can be the case when we're testing reflink and fallocate on the XFS
> > > > realtime device.  For those configurations, the file allocation unit is
> > > > a realtime extent, which can be any integer multiple of the block size.
> > > > If the request length isn't an exact multiple of the allocation unit
> > > > size, reflink and fallocate will fail due to alignment issues, so
> > > > there's no point in running these tests.
> > > > 
> > > > Assuming this edgecase configuration of an edgecase feature is
> > > > vanishingly rare, let's just _notrun the tests instead of rewriting a
> > > > ton of tests to do their integrity checking by hand.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > > ---
> > > >  common/punch |    1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > 
> > > > diff --git a/common/punch b/common/punch
> > > > index 4d16b898..7560edf8 100644
> > > > --- a/common/punch
> > > > +++ b/common/punch
> > > > @@ -250,6 +250,7 @@ _test_generic_punch()
> > > >  	_8k="$((multiple * 8))k"
> > > >  	_12k="$((multiple * 12))k"
> > > >  	_20k="$((multiple * 20))k"
> > > > +	_require_congruent_file_oplen $TEST_DIR $((multiple * 4096))
> > > 
> > > Should the $TEST_DIR be $testfile, or $(dirname $testfile) ?
> > 
> > Ah, right, that ought to be $(dirname $testfile), thanks for catching
> > that.  I guess I didn't catch that because all the current callers pass
> > in $TEST_DIR/<somefile>, which is functionally the same, but a landmine
> > nonetheless.
> 
> Yeah, I checked all cases which call _test_generic_punch(), they all use
> $TEST_DIR. In this patchset (e.g. patch 2/8) you sometimes use $testdir
> for _require_congruent_file_oplen, sometimes use $TEST_DIR or $SCRATCH_MNT
> directly (even there's $testdir too), although they're not wrong :)

<nod> I'll clean that up for the V2 submission tomorrow.

> This patchset really touch many cases, they looks good mostly, but to avoid
> bringing in regressions, I have to give them more tests before merging. And
> welcome more reviewing from others :)

Agreed.

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > >  
> > > >  	# initial test state must be defined, otherwise the first test can fail
> > > >  	# due ot stale file state left from previous tests.
> > > > 
> > > 
> > 
> 



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux