Re: [PATCH 1/2] generic/067: add a testcase to check some inline truncation behaviors

[Jaegeuk Kim <jaegeuk@kernel.org>]

Hi Dave,

Thank you for the review.
I'll resend this series with precise description and fixes as you pointed.

Thanks,

On Wed, Mar 18, 2015 at 02:46:53PM +1100, Dave Chinner wrote:
> On Tue, Mar 17, 2015 at 10:32:51AM -0700, Jaegeuk Kim wrote:
> > This patch introduces a new testcase, generic/067, to test several truncation
> > cases under the inline feature supported by filesystems.
> 
> "inline feature" means what, exactly?
> 
> > 
> > +_supported_os Linux
> > +_require_scratch
> > +
> > +testfile=$SCRATCH_MNT/testfile
> > +
> > +_scratch_mkfs > /dev/null 2>&1
> > +_scratch_mount
> > +
> > +# aligned inline truncation
> 
> Aligned to what, exactly?
> 
> > +$XFS_IO_PROG -t -f \
> > +-c "pwrite -S 0x58 0 120"       `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
> > +-c "fsync"	\
> > +-c "truncate 0"                 `# truncate     |                        |` \
> > +-c "truncate 160"               `# truncate     |                        |` \
> > +-c "close"      \
> > +$testfile | _filter_xfs_io
> 
> Please indent multiple line xfs_io command a little more clearly,
> and align the "\" so it's clear it's amultiple line command
> 
> $XFS_IO_PROG -t -f								    \
> 	-c "pwrite -S 0x58 0 120"       `# write        |XXXXXXXXXXXXXXXXXXXXXXXX|` \
> 	-c "fsync"								    \
> 	-c "truncate 0"                 `# truncate     |                        |` \
> 	-c "truncate 160"               `# truncate     |                        |` \
> 	-c "close"								    \
> 	$testfile | _filter_xfs_io
> 
> (not sure the comment on the second truncate is correct, either)
> 
> Also, why do you need the "close" command?
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com