On Wed, Mar 23, 2022 at 05:37:18PM +0530, Nitesh Shetty wrote:
> 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`
Good idea! :)
--D
> --Nitesh Shetty
