Re: [Qemu-block] [RFC for-2.7 1/1] block/qapi: Add query-block-node-tree

[Max Reitz]

On 25.03.2016 03:50, Wen Congyang wrote:
> On 03/25/2016 03:07 AM, Max Reitz wrote:
>> This command returns the tree of BlockDriverStates under a given root
>> node.
>>
>> Every tree node is described by its node name and the connection of a
>> parent node to its children additionally contains the role the child
>> assumes.
>>
>> A node's name can then be used e.g. in conjunction with
>> query-named-block-nodes to get more information about the node.
> 
> I test this patch, and it works.
> {'execute': 'query-block-node-tree', 'arguments': {'root-node': 'disk1' } }
> {"return": {"children": [{"role": "children.0", "node": {"children": 
> [{"role": "file", "node": {"children": [], "node-name": "#block175"}}], 
> "node-name": "#block267"}}], "node-name": "#block040"}}
> 
> Shoule we hide the node name like "#blockxxx"?

No, I don't think so. In fact I was thinking of making the node name
non-optional because every BDS should have one due to these
auto-generated ones.

> If the bs doesn't have any child, should we output: '"children": [], '?

Omitting an empty array (thus making the @children key optional)
occurred to me, too. I didn't find any strong reason to do so, however.
It makes generating the output a bit more complicated and may also make
parsing it just a tiny bit more complicated.

I think that omitting it would make more sense to a human reader, but
QMP is a machine protocol, so this is not a very strong argument.

So all in all I saw neither strong arguments to omit an empty array nor
to include it. Thus I included it because that makes the code simpler.

> Can we add a new parameter: depth? For example, If I only want to know the 
> quorum's
> child name, we can limit the depth, and the output may be very clear.

Good idea. I can do so in the next version, but first I'd like to hear
more opinions on what other people think of this command. If they think
it's fine, I'll include a depth parameter.

Thank you for you comments!

Max

signature.asc
Description: OpenPGP digital signature