From: Stefan Reiter <s.reiter@proxmox.com>
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.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
[FE: fixed typo pointed out by Eric Blake]
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
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 b20737e63c..fd4f5866c7 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -981,6 +981,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
{
const char *tmp = p;
int skip_key = 0;
+ int ret;
/* option */
c = *typestr++;
@@ -1003,11 +1004,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 3da3f86c6a..a4cb307c8a 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 a 'V', it
+ * specifies an optional string param (e.g. '-fV' allows '-f foo')
*
*/
--
2.30.2
Fabian Ebner <f.ebner@proxmox.com> writes:
> From: Stefan Reiter <s.reiter@proxmox.com>
>
> 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.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> [FE: fixed typo pointed out by Eric Blake]
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 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 b20737e63c..fd4f5866c7 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -981,6 +981,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
> {
> const char *tmp = p;
> int skip_key = 0;
> + int ret;
> /* option */
>
> c = *typestr++;
> @@ -1003,11 +1004,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 3da3f86c6a..a4cb307c8a 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 a 'V', it
> + * specifies an optional string param (e.g. '-fV' allows '-f foo')
> *
> */
For what it's worth, getopt() uses ':' after the option character for
"takes an argument".
Happy to make that tweak in my tree. But see my review of PATCH 3
first.
Am 09.02.22 um 15:12 schrieb Markus Armbruster:
> Fabian Ebner <f.ebner@proxmox.com> writes:
----8<----
>> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
>> index 3da3f86c6a..a4cb307c8a 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 a 'V', it
>> + * specifies an optional string param (e.g. '-fV' allows '-f foo')
>> *
>> */
>
> For what it's worth, getopt() uses ':' after the option character for
> "takes an argument".
>
Doing that leads to e.g.
.args_type = "protocol:s,password:s,display:-d:,connected:s?",
so there's two different kinds of colons now. It's not a problem
functionality-wise AFAICT, but it might not be ideal. Should I still go
for it?
Also, wouldn't future non-string flag parameters need their own letter
too? What about re-using 's' here instead?
> Happy to make that tweak in my tree. But see my review of PATCH 3
> first.
>
>
Fabian Ebner <f.ebner@proxmox.com> writes: > Am 09.02.22 um 15:12 schrieb Markus Armbruster: >> Fabian Ebner <f.ebner@proxmox.com> writes: > > ----8<---- > >>> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h >>> index 3da3f86c6a..a4cb307c8a 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 a 'V', it >>> + * specifies an optional string param (e.g. '-fV' allows '-f foo') >>> * >>> */ >> >> For what it's worth, getopt() uses ':' after the option character for >> "takes an argument". >> > > Doing that leads to e.g. > .args_type = "protocol:s,password:s,display:-d:,connected:s?", > so there's two different kinds of colons now. Point. > It's not a problem > functionality-wise AFAICT, but it might not be ideal. Should I still go > for it? > > Also, wouldn't future non-string flag parameters need their own letter > too? What about re-using 's' here instead? Another good point. Dave, what do you think? >> Happy to make that tweak in my tree. But see my review of PATCH 3 >> first.
* Markus Armbruster (armbru@redhat.com) wrote: > Fabian Ebner <f.ebner@proxmox.com> writes: > > > Am 09.02.22 um 15:12 schrieb Markus Armbruster: > >> Fabian Ebner <f.ebner@proxmox.com> writes: > > > > ----8<---- > > > >>> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h > >>> index 3da3f86c6a..a4cb307c8a 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 a 'V', it > >>> + * specifies an optional string param (e.g. '-fV' allows '-f foo') > >>> * > >>> */ > >> > >> For what it's worth, getopt() uses ':' after the option character for > >> "takes an argument". > >> > > > > Doing that leads to e.g. > > .args_type = "protocol:s,password:s,display:-d:,connected:s?", > > so there's two different kinds of colons now. > > Point. Yeh, : is probably a bad idea. > > It's not a problem > > functionality-wise AFAICT, but it might not be ideal. Should I still go > > for it? > > > > Also, wouldn't future non-string flag parameters need their own letter > > too? What about re-using 's' here instead? > > Another good point. > > Dave, what do you think? Yeh, I'd be happy reusing 's' here. Dave > >> Happy to make that tweak in my tree. But see my review of PATCH 3 > >> first. > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2026 Red Hat, Inc.