diff mbox

[1/3] hmp: Allow options with parameters

Message ID 1401276034-30486-2-git-send-email-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert May 28, 2014, 11:20 a.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

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 <dgilbert@redhat.com>
---
 hmp-commands.hx |  2 ++
 monitor.c       | 19 +++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

Comments

Eric Blake May 28, 2014, 12:31 p.m. UTC | #1
On 05/28/2014 05:20 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> 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 <dgilbert@redhat.com>
> ---
>  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 <eblake@redhat.com>
Dr. David Alan Gilbert May 28, 2014, 1:34 p.m. UTC | #2
* Eric Blake (eblake@redhat.com) wrote:
> On 05/28/2014 05:20 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > 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 <dgilbert@redhat.com>
> > ---
> >  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 <eblake@redhat.com>
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

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));
+                            }
+                        }
                     }
                 }
             }