Re: [Qemu-devel] [PATCH 1/3] hmp: Allow options with parameters

2014-05-28 Thread Dr. David Alan Gilbert 
* Eric Blake (ebl...@redhat.com) wrote:
> On 05/28/2014 05:20 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > HMP currently allows optional string parameters, however where
> > there are multiple optional string parameters the order and
> > interdependence of them becomes complex.
> > 
> > Allow optional parameters of the form:
> > 
> >-x string
> > 
> > Also, add a hint to hmp-commands.hx as to where to find the
> > explanations of the flags.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  hmp-commands.hx |  2 ++
> >  monitor.c   | 19 +--
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> I like it. It can be used to solve one of the questions I raised to Jeff
> about his proposed HMP command to change the backing file name not
> actually being tractable as he had proposed it.
> 
> 
> > +++ b/monitor.c
> > @@ -110,6 +110,7 @@
> >   * 'b'  boolean
> >   *  user mode accepts "on" or "off"
> >   * '-'  optional parameter (eg. '-f')
> 
> I'd reword this line:
> 
> * '-'  optional boolean parameter, on if present (eg. '-f')
> 
> > + * '+'  optional parameter with value parameter
> 
> so that this line makes more sense as-is.

OK, I'll reword for the next cut.

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


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/3] hmp: Allow options with parameters

2014-05-28 Thread Eric Blake 
On 05/28/2014 05:20 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> HMP currently allows optional string parameters, however where
> there are multiple optional string parameters the order and
> interdependence of them becomes complex.
> 
> Allow optional parameters of the form:
> 
>-x string
> 
> Also, add a hint to hmp-commands.hx as to where to find the
> explanations of the flags.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  hmp-commands.hx |  2 ++
>  monitor.c   | 19 +--
>  2 files changed, 19 insertions(+), 2 deletions(-)

I like it. It can be used to solve one of the questions I raised to Jeff
about his proposed HMP command to change the backing file name not
actually being tractable as he had proposed it.


> +++ b/monitor.c
> @@ -110,6 +110,7 @@
>   * 'b'  boolean
>   *  user mode accepts "on" or "off"
>   * '-'  optional parameter (eg. '-f')

I'd reword this line:

* '-'  optional boolean parameter, on if present (eg. '-f')

> + * '+'  optional parameter with value parameter

so that this line makes more sense as-is.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 1/3] hmp: Allow options with parameters

2014-05-28 Thread Dr. David Alan Gilbert (git) 
From: "Dr. David Alan Gilbert" 

HMP currently allows optional string parameters, however where
there are multiple optional string parameters the order and
interdependence of them becomes complex.

Allow optional parameters of the form:

   -x string

Also, add a hint to hmp-commands.hx as to where to find the
explanations of the flags.

Signed-off-by: Dr. David Alan Gilbert 
---
 hmp-commands.hx |  2 ++
 monitor.c   | 19 +--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2e462c0..4cbceda 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -4,6 +4,8 @@ HXCOMM discarded from C version
 HXCOMM DEF(command, args, callback, arg_string, help) is used to construct
 HXCOMM monitor commands
 HXCOMM HXCOMM can be used for comments, discarded from both texi and C
+HXCOMM See the comment near the top of monitor.c for an explanation of the
+HXCOMM args_type options encoding.
 
 STEXI
 @table @option
diff --git a/monitor.c b/monitor.c
index 593679a..7343fab 100644
--- a/monitor.c
+++ b/monitor.c
@@ -110,6 +110,7 @@
  * 'b'  boolean
  *  user mode accepts "on" or "off"
  * '-'  optional parameter (eg. '-f')
+ * '+'  optional parameter with value parameter
  *
  */
 
@@ -4029,9 +4030,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 }
 break;
 case '-':
+case '+': /* Option with parameter */
 {
 const char *tmp = p;
 int skip_key = 0;
+bool wants_param = (c == '+');
 /* option */
 
 c = *typestr++;
@@ -4055,8 +4058,20 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 p = tmp;
 } else {
 /* has option */
-p++;
-qdict_put(qdict, key, qbool_from_int(1));
+p++; /* Now points to just after the option char */
+
+if (!wants_param) {
+qdict_put(qdict, key, qbool_from_int(1));
+} else {
+/* Get the parameter as a string */
+if (get_str(buf, sizeof(buf), &p) < 0) {
+monitor_printf(mon, "%s: Missing parameter for 
"
+"-%c\n", cmdname, c);
+goto fail;
+} else {
+qdict_put(qdict, key, qstring_from_str(buf));
+}
+}
 }
 }
 }
-- 
1.9.3