Re: [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer

[Alberto Garcia]

On Tue 11 Oct 2016 06:50:27 PM CEST, Markus Armbruster wrote:

> * Is the extended command still a sane interface?  If writing clear
>   documentation for it is hard, it perhaps isn't.  Pay special
>   attention to failure modes.  Overloaded arguments are prone to
>   confusing errors.

This is what the current command looks like:

{ 'command': 'block-stream',
  'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
            '*backing-file': 'str', '*speed': 'int',
            '*on-error': 'BlockdevOnError' } }

If we decide to add a new command, this is what it could look like:

{ 'command': 'blockdev-stream',
  'data': { '*job-id': 'str', 'top': 'str', '*base': 'str',
            '*backing-file': 'str', '*speed': 'int',
            '*on-error': 'BlockdevOnError' } }

If we decide to extend the existing command, there's essentially two
changes that we have to do:

1) 'device' refers to a device name, it should refer to (or allow) a
   node name. This is trivial to do, the only problem is that the name
   of the parameter is not the best.

2) 'base' takes a file name, but we should have a way to pass a node
    name instead. Overloading here is not an option, we need a new
    parameter ('base-node' or something like that). 'base' and
    'base-node' would be optional but mutually exclusive.

{ 'command': 'block-stream',
  'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
            '*base-node': 'str', '*backing-file': 'str',
            '*speed': 'int', '*on-error': 'BlockdevOnError' } }

Considering that a new command would be very similar to the original one
(the only problems being an ill-named parameter and an obsolete one), I
actually don't think that extending the current command is such a bad
idea. But I don't have a strong opinion.

Berto