On Tue, Mar 22, 2022 at 09:07:56AM -0700, Darrick J. Wong wrote:
> 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.
>
Sure, will send a different patchset.
> > 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
>
As a alternate, using blockdev utility might reduce and simplify the
changes required. how about below changes ?
-physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
-logical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/logical_block_size)
+physical=`blockdev --getpbsz $SCRATCH_DEV`
+logical=`blockdev --getss $SCRATCH_DEV`
--Nitesh Shetty
