Re: [PATCH v5 4/6] block: Support multiple reopening with x-blockdev-reopen

[Vladimir Sementsov-Ogievskiy]

06.07.2021 14:23, Kevin Wolf wrote:
> From: Alberto Garcia <berto@igalia.com>
>
> [ kwolf: Fixed AioContext locking ]
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   qapi/block-core.json                          | 18 +++--
>   blockdev.c                                    | 81 ++++++++++---------
>   tests/qemu-iotests/155                        |  9 ++-
>   tests/qemu-iotests/165                        |  4 +-
>   tests/qemu-iotests/245                        | 27 ++++---
>   tests/qemu-iotests/248                        |  2 +-
>   tests/qemu-iotests/248.out                    |  2 +-
>   tests/qemu-iotests/296                        |  9 ++-

[..]

>   
>   ##
>   # @blockdev-del:
> diff --git a/blockdev.c b/blockdev.c
> index f657d090d3..1e8c946828 100644
> --- a/blockdev.c
> +++ b/blockdev.c

[..]

>   
> -    bs = bdrv_find_node(options->node_name);
> -    if (!bs) {
> -        error_setg(errp, "Failed to find node with node-name='%s'",
> +        bs = bdrv_find_node(options->node_name);
> +        if (!bs) {
> +            error_setg(errp, "Failed to find node with node-name='%s'",
>                      options->node_name);

indentation broken

> -        goto fail;
> -    }
> +            goto fail;
> +        }
>   
> -    /* Put all options in a QDict and flatten it */
> -    visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
> -    visit_complete(v, &obj);
> -    qdict = qobject_to(QDict, obj);
> +        /* Put all options in a QDict and flatten it */
> +        v = qobject_output_visitor_new(&obj);
> +        visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
> +        visit_complete(v, &obj);
> +        visit_free(v);

[..]

> +    # Run x-blockdev-reopen on a list of block devices
> +    def reopenMultiple(self, opts, errmsg = None):
> +        result = self.vm.qmp('x-blockdev-reopen', conv_keys = False, options = 
> opts)

I don't really care if this doesn't break iotest 297, but PEP8 doesn't like 
whitespaces around '=' for named arguments..

> +        if errmsg:
> +            self.assert_qmp(result, 'error/class', 'GenericError')
> +            self.assert_qmp(result, 'error/desc', errmsg)
> +        else:
> +            self.assert_qmp(result, 'return', {})



--
Best regards,
Vladimir