Re: [PATCH] iotests: Drop deprecated 'props' from object-add

[Max Reitz <mreitz@redhat.com>]

On 16.02.21 18:16, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>   tests/qemu-iotests/087     |  8 ++------
>   tests/qemu-iotests/184     | 18 ++++++------------
>   tests/qemu-iotests/218     |  2 +-
>   tests/qemu-iotests/235     |  2 +-
>   tests/qemu-iotests/245     |  4 ++--
>   tests/qemu-iotests/258     |  7 +++----
>   tests/qemu-iotests/258.out |  4 ++--
>   tests/qemu-iotests/295     |  2 +-
>   tests/qemu-iotests/296     |  2 +-
>   9 files changed, 19 insertions(+), 30 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Two Python syntax nit picks below.

[...]

> diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
> index ae7c4fb187..cbb38923cf 100755
> --- a/tests/qemu-iotests/218
> +++ b/tests/qemu-iotests/218
> @@ -152,7 +152,7 @@ with iotests.VM() as vm, \
>       vm.launch()
>   
>       ret = vm.qmp('object-add', qom_type='throttle-group', id='tg',
> -                 props={'x-bps-read': 4096})
> +                 x_bps_read = 4096)

To stay consistent, I think there shouldn’t be spaces around '=' here.

(flake8 thinks so, too)

>       assert ret['return'] == {}
>   
>       ret = vm.qmp('blockdev-add',

[..]

> diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258
> index 9a2d33ae5e..65ce02501a 100755
> --- a/tests/qemu-iotests/258
> +++ b/tests/qemu-iotests/258
> @@ -103,10 +103,9 @@ def test_concurrent_finish(write_to_stream_node):
>           vm.qmp_log('object-add',
>                      qom_type='throttle-group',
>                      id='tg',
> -                   props={
> -                       'x-iops-write': 1,
> -                       'x-iops-write-max': 1
> -                   })
> +                   x_iops_write=1,
> +                   x_iops_write_max=1
> +                   )

This indentation looks weird to me now.  Unfortunately, flake8 finds 
this is the only correct indentation, so I have no reason to complain.

Perhaps putting it on the preceding line would be better?