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
> [email protected]
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to