Re: [Qemu-devel] [PATCH v2 09/11] block: add ability for block-stream to use node-name

[Eric Blake]

On 05/27/2014 08:28 AM, Jeff Cody wrote:
> This adds the ability for block-stream to use node-name arguments
> for base, to specify the backing image to stream from.
> 
> Both 'base' and 'base-node-name' are optional, but mutually exclusive.
> Either can be specified, but not both together.
> 
> The argument for "device" is now optional as well, so long as
> base-node-name is specified.  From the node-name, we can determine
> the full device chain.
> 
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  blockdev.c       | 62 
> ++++++++++++++++++++++++++++++++++++++++++++++----------
>  hmp-commands.hx  |  2 +-
>  hmp.c            | 10 +++++----
>  qapi-schema.json | 15 ++++++++++----
>  qmp-commands.hx  |  2 +-
>  5 files changed, 70 insertions(+), 21 deletions(-)

Revisiting the HMP side of things:

> +++ b/hmp-commands.hx
> @@ -76,7 +76,7 @@ ETEXI
>  
>      {
>          .name       = "block_stream",
> -        .args_type  = "device:B,speed:o?,base:s?",
> +        .args_type  = "device:B?,speed:o?,base:s?,base-node-name:s?",

The HMP parser is trash.  It requires arguments to appear in declaration
order, and in order to specify an optional argument at the end of the
list, you must specify all earlier arguments.  Your proposed change is
unworkable, because the ONLY way HMP can supply base-node-name is if the
command line already supplied a base argument, but base and
base-node-name are incompatible.

I'm not quite sure if David's idea of allowing an optional named
argument would help:
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg05914.html

The idea being that you'd write
 .args_type = "device:B?,speed:o?,base-node-name:+n,base:s?"

where the command is then invoked as either:

block_stream dev 0 base    # resolve base as a file name

or as:

block_stream -n base       # resolve base as a node name, implicit dev
block_stream dev -n base   # resolve base as a node name within dev

But you can see why I suggested earlier that maybe it's better to just
forget about HMP entirely, and make this series focus on QMP (no need to
stall the feature addition on the HMP warts, when backports only care
about the QMP feature); save the HMP changes for another day (if ever).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature