qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH RFC] block: add block-insert-node Q


From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH RFC] block: add block-insert-node QMP command
Date: Tue, 15 Aug 2017 17:12:42 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

On 08/15/2017 02:45 AM, Manos Pitsidianakis wrote:
> block-insert-node and its pair command block-remove-node provide runtime
> insertion and removal of filter nodes.
> 
> block-insert-node takes a (parent, child) and (node, child) pair of
> edges and unrefs the (parent, child) BdrvChild relationship and creates
> a new (parent, node) child with the same BdrvChildRole.
> 
> This is a different approach than x-blockdev-change which uses the driver
> methods bdrv_add_child() and bdrv_del_child(),
> 
> Signed-off-by: Manos Pitsidianakis <address@hidden>
> ---
>  block.c                    |  192 ++++++++
>  blockdev.c                 |   44 ++
>  include/block/block.h      |    6 +
>  qapi/block-core.json       |   60 +++
>  tests/qemu-iotests/193     |  241 ++++++++++
>  tests/qemu-iotests/193.out | 1116 
> ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |    1 +
>  7 files changed, 1660 insertions(+)
>  create mode 100755 tests/qemu-iotests/193
>  create mode 100644 tests/qemu-iotests/193.out

You may want to look at using scripts/git.orderfile, to rearrange your
patch so that interface changes (.json, .h) occur before implementation
(.c).  For now, I'm just focusing on the interface:


> +++ b/qapi/block-core.json
> @@ -3947,3 +3947,63 @@
>    'data' : { 'parent': 'str',
>               '*child': 'str',
>               '*node': 'str' } }
> +
> +##
> +# @block-insert-node:
> +#
> +# Insert a filter node between a specific edge in the block driver state 
> graph.
> +# @parent:  the name of the parent node or device
> +# @node:    the name of the node to insert under parent
> +# @child:   the name of the child of both node and parent

Is this always going to be between two existing nodes, or can this
command also be used to insert at the end of the chain (for example, if
parent or child is omitted)?


> +#    }
> +# <- { 'return': {} }
> +#
> +##

Missing 'Since: 2.11'.

> +{ 'command': 'block-insert-node',
> +  'data': { 'parent': 'str',
> +             'child': 'str',
> +             'node': 'str'} }

For now, it looks like you require all arguments, and therefore this is
always insertion in the middle.

> +##
> +# @block-remove-node:
> +#
> +# Remove a filter node between two other nodes in the block driver state 
> graph.
> +# @parent:  the name of the parent node or device
> +# @node:    the name of the node to remove from parent
> +# @child:   the name of the child of node which will go under parent
> +##
> +{ 'command': 'block-remove-node',
> +  'data': { 'parent': 'str',
> +             'child': 'str',
> +             'node': 'str'} }

Likewise missing 2.11.

Overall I'm not seeing problems with the interface from the UI
perspective, but I have not been paying close attention to your larger
efforts on throttling nodes, so I hope other reviewers will chime in.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]