Re: [PATCH] generic/108: use sysfs values for logical,physical block size in scsi_debug

["Darrick J. Wong" <djwong@xxxxxxxxxx>]

On Tue, Mar 22, 2022 at 01:56:29PM +0530, Nitesh Shetty wrote:
>  Mon, Mar 21, 2022 at 01:21:33PM -0700, Darrick J. Wong wrote:
> > On Wed, Mar 02, 2022 at 02:59:47AM +0530, Nitesh Shetty wrote:
> > > scsi_debug device used for test, is created with assumption of 512 bytes
> > > logical and physical block size.
> > > This causes error in lvcreate step, when SCRATCH_DEV device lba is not
> > > 512 bytes. This can be solved by reading block size from sysfs of device.
> > > If sysfs is missing fallback to 512 bytes as default.
> > > 
> > > Signed-off-by: Nitesh Shetty <nj.shetty@xxxxxxxxxxx>
> > > ---
> > >  tests/generic/108 | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/generic/108 b/tests/generic/108
> > > index ad43269f..db0e9bd0 100755
> > > --- a/tests/generic/108
> > > +++ b/tests/generic/108
> > > @@ -42,8 +42,11 @@ _require_non_zoned_device $SCRATCH_DEV
> > >  lvname=lv_$seq
> > >  vgname=vg_$seq
> > >  
> > > +physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
> > > +logical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/logical_block_size)
> > 
> > This causes a regression if $SCRATCH_DEV is not a raw block device:
> > 
> Acked, I was testing for NVMe device, missed out sda sysfs.
> Let me see, if I can use other sysfs/utility to get block size of device.
> 
> Also I see many 4k LBA tests failing mainly because of setup failure in
> format command. This is not actually test failure, rather setup failure.
> So what do you suggest for those type of tests?

Ideally, fix the ones that can be fixed, and _notrun the rest.

> Should I put them in not run status or just report to community so that
> person with relevant expertise can add a fix?

For this specific problem I suggest creating a function that finds the
/sys/block/XXX path for any given block device or partition, and then
update all the open coded logic to use it.  Something like:

# Map a block device to its counterpart in sysfs
_sysfs_block() {
	local dev="$1"
	local shortdev="$(_short_dev "$dev")"

	# Full block devices are simple
	local ret="/sys/block/$shortdev"
	if [ -e "$ret" ]; then
		readlink -m "$ret"
		return 0
	fi

	# Partitions are a little trickier
	ret="/sys/class/block/$shortdev"
	if [ -e "$ret/partition" ]; then
		readlink -m "$ret/.."
		return 0
	fi

	# ???
	return 1
}

sysfsb=$(_sysfs_block $SCRATCH_DEV)
physical=$(cat $sysfsb/queue/physical_block_size)

--D

> 
> --Nitesh Shetty
> 
> 
> > # SCRATCH_DEV=/dev/sda4 ./check generic/108
> > ...
> > --- generic/108.out
> > +++ generic/108.out.bad
> > @@ -1,2 +1,4 @@
> >  QA output created by 108
> > +cat: /sys/block/sda4/queue/physical_block_size: No such file or directory
> > +cat: /sys/block/sda4/queue/logical_block_size: No such file or directory
> >  fsync: Input/output error
> > 
> > --D
> > 
> > > +
> > >  # _get_scsi_debug_dev returns a scsi debug device with 128M in size by default
> > > -SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 300`
> > > +SCSI_DEBUG_DEV=`_get_scsi_debug_dev ${physical:-512} ${logical:-512} 0 300`
> > >  test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
> > >  echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full
> > >  
> > > 
> > > base-commit: 2ea74ba4e70b546279896e2a733c8c7f4b206193
> > > -- 
> > > 2.25.1
> > > 
> >