Re: [PATCH v2 04/17] iotests.py: rewrite default luks support in qemu_img

[Hanna Reitz <hreitz@redhat.com>]

On 14.09.21 09:44, Vladimir Sementsov-Ogievskiy wrote:
> 13.09.2021 14:16, Hanna Reitz wrote:
>> On 20.07.21 13:38, Vladimir Sementsov-Ogievskiy wrote:
>>> Move the logic to more generic qemu_img_pipe_and_status(). Also behave
>>> better when we have several -o options. And reuse argument parser of
>>> course.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/iotests.py | 36 
>>> +++++++++++++++++------------------
>>>   1 file changed, 17 insertions(+), 19 deletions(-)
>>
>> Reviewed-by: Hanna Reitz <hreitz@redhat.com>
>>
>>> diff --git a/tests/qemu-iotests/iotests.py 
>>> b/tests/qemu-iotests/iotests.py
>>> index def6ae2475..484f616270 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -128,9 +128,13 @@ def qemu_img_create_prepare_args(args: 
>>> List[str]) -> List[str]:
>>>       args = args[1:]
>>>       p = argparse.ArgumentParser(allow_abbrev=False)
>>> +    # -o option may be specified several times
>>> +    p.add_argument('-o', action='append', default=[])
>>>       p.add_argument('-f')
>>>       parsed, remaining = p.parse_known_args(args)
>>> +    opts_list = parsed.o
>>> +
>>>       result = ['create']
>>>       if parsed.f is not None:
>>>           result += ['-f', parsed.f]
>>> @@ -139,8 +143,18 @@ def qemu_img_create_prepare_args(args: 
>>> List[str]) -> List[str]:
>>>       # like extended_l2 or compression_type for qcow2. Test may 
>>> want to create
>>>       # additional images in other formats that doesn't support 
>>> these options.
>>>       # So, use IMGOPTS only for images created in imgfmt format.
>>> -    if parsed.f == imgfmt and 'IMGOPTS' in os.environ:
>>> -        result += ['-o', os.environ['IMGOPTS']]
>>> +    imgopts = os.environ.get('IMGOPTS')
>>> +    if imgopts and parsed.f == imgfmt:
>>> +        opts_list.insert(0, imgopts)
>>
>> Hm.  Yes, IMGOPTS should come first, so it has lower priority. That 
>> means that patch 2 should have inserted IMGOPTS at the beginning of 
>> the parameter list, though, right?
>
> Now looking at it closer: no, patch 2 is OK.
>
> It only put into result:
>
> 1. 'create'
> 2. -f <fmt>
> 3. -o <IMGOPTS>
> 4. remaining
>
> and that's correct

Ah, right!

Hanna