On Wed, May 28, 2014 at 06:42:24AM -0600, Eric Blake wrote:
> 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 <jc...@redhat.com>
> > ---
> > 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).
>
I agree with not worrying about HMP in this series; the new
functionality is targeted towards a management software layer (e.g.
libvirt). As you said, if it is desired for some reason, it can be
added later.
