Re: [PATCH 4/6] common/atomicwrites: adjust a few more things

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, May 14, 2025 at 02:11:00PM +0100, John Garry wrote:
> On 14/05/2025 01:29, Catherine Hoang wrote:
> > From: "Darrick J. Wong"<djwong@kernel.org>
> > 
> > Always export STATX_WRITE_ATOMIC so anyone can use it, make the "cp
> > reflink" logic work for any filesystem, not just xfs, and create a
> > separate helper to check that the necessary xfs_io support is present.
> > 
> > Signed-off-by: "Darrick J. Wong"<djwong@kernel.org>
> > Signed-off-by: Catherine Hoang<catherine.hoang@oracle.com>
> 
> Just a small comment query below.
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> 
> > ---
> >   common/atomicwrites | 18 +++++++++++-------
> >   tests/generic/765   |  2 +-
> >   2 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/common/atomicwrites b/common/atomicwrites
> > index fd3a9b71..9ec1ca68 100644
> > --- a/common/atomicwrites
> > +++ b/common/atomicwrites
> > @@ -4,6 +4,8 @@
> >   #
> >   # Routines for testing atomic writes.
> > +export STATX_WRITE_ATOMIC=0x10000
> > +
> >   _get_atomic_write_unit_min()
> >   {
> >   	$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $1 | \
> > @@ -26,8 +28,6 @@ _require_scratch_write_atomic()
> >   {
> >   	_require_scratch
> > -	export STATX_WRITE_ATOMIC=0x10000
> > -
> >   	awu_min_bdev=$(_get_atomic_write_unit_min $SCRATCH_DEV)
> >   	awu_max_bdev=$(_get_atomic_write_unit_max $SCRATCH_DEV)
> > @@ -51,6 +51,14 @@ _require_scratch_write_atomic()
> >   	fi
> >   }
> > +# Check for xfs_io commands required to run _test_atomic_file_writes
> > +_require_atomic_write_test_commands()
> > +{
> > +	_require_xfs_io_command "falloc"
> > +	_require_xfs_io_command "fpunch"
> > +	_require_xfs_io_command pwrite -A
> > +}
> > +
> >   _test_atomic_file_writes()
> >   {
> >       local bsize="$1"
> > @@ -64,11 +72,7 @@ _test_atomic_file_writes()
> >       test $bytes_written -eq $bsize || echo "atomic write len=$bsize failed"
> >       # Check that we can perform an atomic single-block cow write
> > -    if [ "$FSTYP" == "xfs" ]; then
> > -        testfile_cp=$SCRATCH_MNT/testfile_copy
> > -        if _xfs_has_feature $SCRATCH_MNT reflink; then
> > -            cp --reflink $testfile $testfile_cp
> > -        fi
> > +    if cp --reflink=always $testfile $testfile_cp 2>> $seqres.full; then
> 
> I suppose that previously for xfs where the cp --reflink failed, we would
> pointlessly try the write - am I correct?

Correct.

> If so, now seems much better.

That and any filesystem that supports reflink and atomic writes will now
test this. :)

Thanks for review!

--D

> >           bytes_written=$($XFS_IO_PROG -dc "pwrite -A -D -V1 -b $bsize 0 $bsize" $testfile_cp | \
> 
>