Re: [PATCH 1/2] common/rc: helper functions to handle block devices via sysfs

[Anand Jain <anajain.sg@gmail.com>]

On 26/9/25 23:57, Zorro Lang wrote:
> On Thu, Sep 18, 2025 at 08:32:46AM +0800, Anand Jain wrote:
>> From: Anand Jain <anand.jain@oracle.com>
>>
>> _bdev_handle(dev)
>>      get sysfs handle for a given block device.
>>
>> _has_bdev_sysfs_delete(dev_path)
>>      Checks if the block device supports sysfs-based delete.
>>
>> _require_scratch_bdev_delete()
>>      Test if the scratch device does not support sysfs delete.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   common/rc | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 81587dad500c..627ddcc02fb8 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -4388,6 +4388,32 @@ _get_file_extent_sector()
>>      echo "$result"
>>   }
>>
>> +_bdev_handle()
>> +{
>> +    local device=$(echo $1 | rev | cut -d"/" -f1 | rev)
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> basename $1 ?
>
> if you hope to make sure it's not link, you can
>

> basename $(_real_dev $1)

Whoops, not sure what I was thinking. There's actually a helper for
this, _short_dev().
The changes below will fix it. I can send v2 if needed, unless it can
be fixed at merge
time. Let me know what works best.
-----------------------------------
$ git diff
diff --git a/common/rc b/common/rc
index 627ddcc02fb8..085fab6e34f0 100644
--- a/common/rc
+++ b/common/rc
@@ -4390,7 +4390,7 @@ _get_file_extent_sector()

 _bdev_handle()
 {
-       local device=$(echo $1 | rev | cut -d"/" -f1 | rev)
+       local device=$(_short_dev $1)

        test -e /sys/class/block/${device}/device/scsi_disk/ || \
                        _notrun "Failed to obtain sys block handle"
@@ -4401,7 +4401,7 @@ _bdev_handle()
 _has_bdev_sysfs_delete()
 {
        local dev_path=$1
-       local device=$(echo $dev_path | rev | cut -d"/" -f1 | rev)
+       local device=$(_short_dev $1)
        local delete_path=/sys/class/block/${device}/device/delete

        test -e $delete_path
---------------------------------------------


>> +
>> +    test -e /sys/class/block/${device}/device/scsi_disk/ || \
>> +                    _notrun "Failed to obtain sys block handle"
>> +
>> +    ls /sys/class/block/${device}/device/scsi_disk/
>> +}
>> +
>> +_has_bdev_sysfs_delete()
>> +{
>> +    local dev_path=$1
>> +    local device=$(echo $dev_path | rev | cut -d"/" -f1 | rev)
>> +    local delete_path=/sys/class/block/${device}/device/delete
>> +
>> +    test -e $delete_path
>> +}
>> +
>> +_require_scratch_bdev_delete()
>> +{
>
> I'm wondering if it's better to call "_require_block_device" at first, before
> checking the /sys ?

There’s no harm in using _require_block_device() here, but I don’t
think it will help,
a non block device won't reach this place anyway.
config: _check_device() will fail for non-block devices.

Thanks, Anand

>> +    if ! _has_bdev_sysfs_delete $SCRATCH_DEV; then
>> +            _notrun "require scratch device sys delete support"
>> +    fi
>> +}
>> +
>>   # arg 1 is dev to remove and is output of the below eg.
>>   # ls -l /sys/class/block/sdd | rev | cut -d "/" -f 3 | rev
>>   _devmgt_remove()
>> --
>> 2.51.0
>>
>>
>