[PATCH v6 1/5] monitor/hmp: add support for flag argument with value

Stefan Reiter posted 5 patches 4 years, 3 months ago
There is a newer version of this series
[PATCH v6 1/5] monitor/hmp: add support for flag argument with value
Posted by Stefan Reiter 4 years, 3 months ago
Adds support for the "-xV" parameter type, where "-x" denotes a flag
name and the "V" suffix indicates that this flag is supposed to take an
arbitrary string parameter.

These parameters are always optional, the entry in the qdict will be
omitted if the flag is not given.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

v6:
It wasn't possible to pass the 'connected' parameter to set_password, since the
code to handle optional parameters couldn't live with a different param (not
starting with '-') coming up instead - fix that by advancing over the 'value
flag' modifier in case `*p != '-'`.

Also change the modifier to 'V' instead of 'S' so it can be distinguished from
an actual trailing 'S' type param.

Discovered in testing. I dropped Eric's R-b due to the code change.

 monitor/hmp.c              | 19 ++++++++++++++++++-
 monitor/monitor-internal.h |  3 ++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/monitor/hmp.c b/monitor/hmp.c
index d50c3124e1..899e0c990f 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -980,6 +980,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
             {
                 const char *tmp = p;
                 int skip_key = 0;
+                int ret;
                 /* option */
 
                 c = *typestr++;
@@ -1002,11 +1003,27 @@ static QDict *monitor_parse_arguments(Monitor *mon,
                     }
                     if (skip_key) {
                         p = tmp;
+                    } else if (*typestr == 'V') {
+                        /* has option with string value */
+                        typestr++;
+                        tmp = p++;
+                        while (qemu_isspace(*p)) {
+                            p++;
+                        }
+                        ret = get_str(buf, sizeof(buf), &p);
+                        if (ret < 0) {
+                            monitor_printf(mon, "%s: value expected for -%c\n",
+                                           cmd->name, *tmp);
+                            goto fail;
+                        }
+                        qdict_put_str(qdict, key, buf);
                     } else {
-                        /* has option */
+                        /* has boolean option */
                         p++;
                         qdict_put_bool(qdict, key, true);
                     }
+                } else if (*typestr == 'V') {
+                    typestr++;
                 }
             }
             break;
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 9c3a09cb01..9e708b329d 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -63,7 +63,8 @@
  * '.'          other form of optional type (for 'i' and 'l')
  * 'b'          boolean
  *              user mode accepts "on" or "off"
- * '-'          optional parameter (eg. '-f')
+ * '-'          optional parameter (eg. '-f'); if followed by an 'V', it
+ *              specifies an optional string param (e.g. '-fV' allows '-f foo')
  *
  */
 
-- 
2.30.2



Re: [PATCH v6 1/5] monitor/hmp: add support for flag argument with value
Posted by Eric Blake 4 years, 3 months ago
On Wed, Oct 20, 2021 at 03:54:56PM +0200, Stefan Reiter wrote:
> Adds support for the "-xV" parameter type, where "-x" denotes a flag
> name and the "V" suffix indicates that this flag is supposed to take an
> arbitrary string parameter.
> 
> These parameters are always optional, the entry in the qdict will be
> omitted if the flag is not given.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> v6:
> It wasn't possible to pass the 'connected' parameter to set_password, since the
> code to handle optional parameters couldn't live with a different param (not
> starting with '-') coming up instead - fix that by advancing over the 'value
> flag' modifier in case `*p != '-'`.
> 
> Also change the modifier to 'V' instead of 'S' so it can be distinguished from
> an actual trailing 'S' type param.
> 
> Discovered in testing. I dropped Eric's R-b due to the code change.

Good that you caught it, and correct in dropping my earlier R-b.  But
now that I've read this version:

> +++ b/monitor/monitor-internal.h
> @@ -63,7 +63,8 @@
>   * '.'          other form of optional type (for 'i' and 'l')
>   * 'b'          boolean
>   *              user mode accepts "on" or "off"
> - * '-'          optional parameter (eg. '-f')
> + * '-'          optional parameter (eg. '-f'); if followed by an 'V', it

s/an/a/

> + *              specifies an optional string param (e.g. '-fV' allows '-f foo')
>   *

With the typo fix,
Reviewed-by: Eric Blake <eblake@redhat.com>

[Hmm, I see that you posted v7 in the meantime, and it still has the
typo, but I already typed this mail]

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org