Re: [PATCH v2 2/3] common/zoned: add _create_zloop

[Damien Le Moal <dlemoal@kernel.org>]

On 2025/10/11 18:34, Johannes Thumshirn wrote:
> On 10/8/25 5:08 PM, Darrick J. Wong wrote:
>> On Wed, Oct 08, 2025 at 04:38:16PM +0200, Carlos Maiolino wrote:
>>> On Tue, Oct 07, 2025 at 02:58:02PM +0200, Johannes Thumshirn wrote:
>>>> Add _create_zloop a helper function for creating a zloop device.
>>>>
>>>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>>> ---
>>>>   common/zoned | 23 +++++++++++++++++++++++
>>>>   1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/common/zoned b/common/zoned
>>>> index 41697b08..33d3543b 100644
>>>> --- a/common/zoned
>>>> +++ b/common/zoned
>>>> @@ -45,3 +45,26 @@ _require_zloop()
>>>>   	    _notrun "This test requires zoned loopback device support"
>>>>       fi
>>>>   }
>>>> +
>>>> +# Create a zloop device
>>>> +# useage: _create_zloop [id] <base_dir> <zone_size> <nr_conv_zones>
>>>> +_create_zloop()
>>>> +{
>>>> +    local id=$1
>>>> +
>>>> +    if [ -n "$2" ]; then
>>>> +        local base_dir=",base_dir=$2"
>>>> +    fi
>>>> +
>>>> +    if [ -n "$3" ]; then
>>>> +        local zone_size=",zone_size_mb=$3"
>>>> +    fi
>>>> +
>>>> +    if [ -n "$4" ]; then
>>>> +        local conv_zones=",conv_zones=$4"
>>>> +    fi
>>>> +
>>>> +    local zloop_args="add id=$id$base_dir$zone_size$conv_zones"
>>>> +
>>>> +    echo "$zloop_args" > /dev/zloop-control
>> Hmm, so the caller figures out its own /dev/zloopNNN number, passes NNN
>> into the zloop-control devices, and then maybe a new bdev is created?
>> Does NNN have to be one more than the current highest zloop device, or
>> can it be any number?
>>
>> Source code says that if NNN >= 0 then it tries to create a new
>> zloopNNN or fails with EEXIST; otherwise it gives you the lowest unused
>> id.  It'd be nice in the second case if there were a way for the driver
>> to tell you what the NNN is.
>>
>> The _create_zloop users seem to do an ls to select an NNN.  At a minimum
>> that code probably ought to get hoisted to here as a common function (or
>> maybe just put in _create_zloop itself).
>>
>> Or maybe turned into a loop like:
>>
>> 	while true; do
>> 		local id=$(_next_zloop_id)
>> 		err="$(echo "add id=$id$base_dir..." 2>&1 > /dev/zloop-control)"
>> 		if [ -z "$err" ]; then
>> 			echo "/dev/zloop$id"
>> 			return 0
>> 		fi
>> 		if echo "$err" | ! grep -q "File exists"; then
>> 			echo "$err" 1>&2
>> 			return 1;
>> 		fi
>> 	done
>>
>> That way test cases don't have to do all that setup themselves?
>>
> Unfortunately the user has to create the zloop directory (e.g. 
> BASE_DIR/0 for zloop0) beforehand (might be a bug though).

Not a bug. It is by design since the user can specify the ID of the zloop drive
to create. And there is no fixed association between device ID and directory
path to keep things flexible for the user/distro.

> What I could do is  encapsulate the find the next zloop and mkdir -p for 
> the user (and call in _create_zloop if no id is supplied?)

Yes. Do that. The zloop directory si not something that the tests should touch
anyway, so you should just define your own id <-> dir path mapping in the helpers.



-- 
Damien Le Moal
Western Digital Research