Re: [PATCH] xfs/273: check thoroughness of the fsmappings

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

On Sat, Nov 09, 2024 at 10:45:16PM +0800, Zorro Lang wrote:
> On Fri, Nov 08, 2024 at 09:41:46AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Enhance this test to make sure that there are no gaps in the fsmap
> > records, and (especially) that they we report all the way to the end of
> > the device.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/273 |   47 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> > 
> > diff --git a/tests/xfs/273 b/tests/xfs/273
> > index d7fb80c4033429..ecfe5e7760a092 100755
> > --- a/tests/xfs/273
> > +++ b/tests/xfs/273
> > @@ -24,6 +24,8 @@ _require_scratch
> >  _require_populate_commands
> >  _require_xfs_io_command "fsmap"
> >  
> > +_fixed_by_git_commit kernel XXXXXXXXXXXXXX "xfs: fix off-by-one error in fsmap's end_daddr usage"
> 
> The _fixed_by_kernel_commit can replace the "_fixed_by_git_commit kernel".

<nod>

> > +
> >  rm -f "$seqres.full"
> >  
> >  echo "Format and mount"
> > @@ -37,6 +39,51 @@ cat $TEST_DIR/a $TEST_DIR/b >> $seqres.full
> >  
> >  diff -uw $TEST_DIR/a $TEST_DIR/b
> >  
> > +# Do we have mappings for every sector on the device?
> > +ddev_fsblocks=$(_xfs_statfs_field "$SCRATCH_MNT" geom.datablocks)
> > +rtdev_fsblocks=$(_xfs_statfs_field "$SCRATCH_MNT" geom.rtblocks)
> > +fsblock_bytes=$(_xfs_statfs_field "$SCRATCH_MNT" geom.bsize)
> > +
> > +ddev_daddrs=$((ddev_fsblocks * fsblock_bytes / 512))
> > +rtdev_daddrs=$((rtdev_fsblocks * fsblock_bytes / 512))
> > +
> > +ddev_devno=$(stat -c '%t:%T' $SCRATCH_DEV)
> > +if [ "$USE_EXTERNAL" = "yes" ] && [ -n "$SCRATCH_RTDEV" ]; then
> > +	rtdev_devno=$(stat -c '%t:%T' $SCRATCH_RTDEV)
> > +fi
> > +
> > +$XFS_IO_PROG -c 'fsmap -m -n 65536' $SCRATCH_MNT | awk -F ',' \
> > +	-v data_devno=$ddev_devno \
> > +	-v rt_devno=$rtdev_devno \
> > +	-v data_daddrs=$ddev_daddrs \
> > +	-v rt_daddrs=$rtdev_daddrs \
> > +'BEGIN {
> > +	next_daddr[data_devno] = 0;
> > +	next_daddr[rt_devno] = 0;
> > +}
> > +{
> > +	if ($1 == "EXT")
> > +		next
> > +	devno = sprintf("%x:%x", $2, $3);
> > +	if (devno != data_devno && devno != rt_devno)
> > +		next
> > +
> > +	if (next_daddr[devno] < $4)
> > +		printf("%sh: expected daddr %d, saw \"%s\"\n", devno,
> > +				next_daddr[devno], $0);
> > +		next = $5 + 1;
> 
> Ahaha, awk expert Darrick :) I tried this patch, but got below error when
> I tried this patch:
> 
>   +awk: cmd. line:15:             next = $5 + 1;
>   +awk: cmd. line:15:                  ^ syntax error

Aha, I forgot to commit the change renaming next to n before sending. :(

--D

> Thanks,
> Zorro
> 
> > +		if (next > next_daddr[devno])
> > +		       next_daddr[devno] = next;
> > +}
> > +END {
> > +	if (data_daddrs != next_daddr[data_devno])
> > +		printf("%sh: fsmap stops at %d, expected %d\n",
> > +				data_devno, next_daddr[data_devno], data_daddrs);
> > +	if (rt_devno != "" && rt_daddrs != next_daddr[rt_devno])
> > +		printf("%sh: fsmap stops at %d, expected %d\n",
> > +				rt_devno, next_daddr[rt_devno], rt_daddrs);
> > +}'
> > +
> >  # success, all done
> >  status=0
> >  exit
> > 
>