Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-04 Thread Markus Armbruster
Daniel P. Berrangé  writes:

> On Wed, Nov 04, 2020 at 12:31:40PM +0100, Paolo Bonzini wrote:
>> On 04/11/20 09:29, Markus Armbruster wrote:
>> > >   It only hurts in the odd case of a boolean option becoming on/off/auto
>> > > or on/off/split.
>> > Another argument for deprecating values other than "on" and "off".
>> 
>> Unfortunately I'm fairly sure that I've seen yes/no in use.

If it wasn't, we'd remove rather than deprecate :)

>>  I can buy
>> insta-removal (not deprecation) of case-insensitivity.

Since case-insensitivity is undocumented, and we're not aware of actual
use, no objection.

> Seems a couple of example usages are my fault as I documented them :-(
>
> docs/system/vnc-security.rst: -object 
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
> docs/system/vnc-security.rst: -object 
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
> docs/system/vnc-security.rst: -object 
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
> include/authz/listfile.h: *   
> filename=/etc/qemu/myvm-vnc.acl,refresh=yes
> qemu-options.hx:``-object 
> authz-listfile,id=id,filename=path,refresh=yes|no``
> qemu-options.hx: -object 
> authz-simple,id=auth0,filename=/etc/qemu/vnc-sasl.acl,refresh=yes \\
>
> We should fix thos docs in QEMU at least, and unfortunately it seems I
> missed that libvirt did use  verify-peer=yes

Yes, please.




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-04 Thread Daniel P . Berrangé
On Wed, Nov 04, 2020 at 12:31:40PM +0100, Paolo Bonzini wrote:
> On 04/11/20 09:29, Markus Armbruster wrote:
> > >   It only hurts in the odd case of a boolean option becoming on/off/auto
> > > or on/off/split.
> > Another argument for deprecating values other than "on" and "off".
> 
> Unfortunately I'm fairly sure that I've seen yes/no in use.  I can buy
> insta-removal (not deprecation) of case-insensitivity.

Seems a couple of example usages are my fault as I documented them :-(

docs/system/vnc-security.rst: -object 
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
docs/system/vnc-security.rst: -object 
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
docs/system/vnc-security.rst: -object 
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
include/authz/listfile.h: *   
filename=/etc/qemu/myvm-vnc.acl,refresh=yes
qemu-options.hx:``-object 
authz-listfile,id=id,filename=path,refresh=yes|no``
qemu-options.hx: -object 
authz-simple,id=auth0,filename=/etc/qemu/vnc-sasl.acl,refresh=yes \\

We should fix thos docs in QEMU at least, and unfortunately it seems I
missed that libvirt did use  verify-peer=yes


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-04 Thread Paolo Bonzini

On 04/11/20 08:35, Markus Armbruster wrote:

+   "boolean (on/off, yes/no, true/false, y/n)");

Recommend to have the error message only mention the preferred form.  I
like the laconic "'on' or 'off'".  It's really all the user needs to
know.



I went for "boolean (on/off) value".



Avoiding the full_name() on success isn't hard:

   if (!qapi_bool_parse(name, str, obj, NULL)) {
   error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
  full_name(qiv, name), "'on' or 'off'");
   return false;
   }
   return true;


Can't refuse.



 case QEMU_OPT_BOOL:
-return parse_option_bool(opt->name, opt->str, &opt->value.boolean,
- errp);
+return qapi_bool_parse(opt->name, opt->str, &opt->value.boolean, errp);


Please break the line the same way as before.


Why? It's not even 80 characters.

Paolo




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-04 Thread Paolo Bonzini

On 04/11/20 09:29, Markus Armbruster wrote:

  It only hurts in the odd case of a boolean option becoming on/off/auto
or on/off/split.

Another argument for deprecating values other than "on" and "off".


Unfortunately I'm fairly sure that I've seen yes/no in use.  I can buy 
insta-removal (not deprecation) of case-insensitivity.


Paolo




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-04 Thread Markus Armbruster
Paolo Bonzini  writes:

> On 03/11/20 17:25, Daniel P. Berrangé wrote:
>>> OptsVisitor, StringInputVisitor and the keyval visitor have
>>> three different ideas of how a human could write the value of
>>> a boolean option.  Pay homage to the backwards-compatibility
>>> gods and make the new common helper accept all four sets (on/off,
>>> true/false, y/n and yes/no), and case-insensitive at that.
>>>
>>> Since OptsVisitor is supposed to match qemu-options, adjust
>>> it as well.
>> FWIW, libvirt does not appear to use true/false or y/n, nor
>> ever use uppercase / mixed case.
>> 
>> IOW this level of back compat may well be overkill.
>> 
>> I'd particular suggest deprecating case-insensitivity, as
>> Yes, YES, yEs feel unlikely to be important or widely used.
>
> True; at least it's type-safe code unlike the short-form boolean option.
>  It only hurts in the odd case of a boolean option becoming on/off/auto
> or on/off/split.

Another argument for deprecating values other than "on" and "off".

> I didn't want to introduce deprecation at this point, because

Not wanting to rush deprecation this close to the release is a valid
point.  So is not wanting to rush in additional sugar :)

> consistency is better anyway even if we plan to later deprecate
> something.  For example, since there is a common parser now, introducing
> deprecation would be much easier.  It also lets us switch parsers even
> during the deprecation period (which is how I got into this mess).

On the other hand, we'll have more to deprecate.  Status quo:


  on/off  yes/no y/n  true/false  case-sensitive?
qemu_opt_get_bool() X no
keyval visitor  X no
string visitor  XXX   yes
opts visitorXXX   no

For once, there is no way to blame QemuOpts ;)

qemu_opt_get_bool() is everywhere in CLI and HMP.

The keyval visitor is used for QAPIfied human-friendly interfaces: block
stuff, -display, -audio.

The string input visitor is used for -object, -global, HMP object_add,
qom-set, migrate_set_parameter.

The opts visitor is used for -mon and its sugared forms (new in 5.2!),
-net, -netdev, -acpitable, -numa.

I'd very much prefer to deprecate the odd ones in string and opts
visitor, and not copy them elsewhere.

We can still factor out a common parsing function if we like: pass a
flag that makes it recognize deprecated forms.  A single flag should
suffice; it lets odd ones spread from string to opts visitor and vice
versa, which I find tolerable.




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-03 Thread Markus Armbruster
Paolo Bonzini  writes:

> OptsVisitor, StringInputVisitor and the keyval visitor have
> three different ideas of how a human could write the value of
> a boolean option.  Pay homage to the backwards-compatibility
> gods and make the new common helper accept all four sets (on/off,
> true/false, y/n and yes/no), and case-insensitive at that.
>
> Since OptsVisitor is supposed to match qemu-options, adjust
> it as well.

For clarity: s/it/the latter/

> Supersedes: <20201103142344.402353-1-pbonz...@redhat.com>
> Signed-off-by: Paolo Bonzini 
> ---
>  include/qapi/util.h  |  2 ++
>  qapi/opts-visitor.c  | 14 +-
>  qapi/qapi-util.c | 23 +++
>  qapi/qobject-input-visitor.c | 15 +--
>  qapi/string-input-visitor.c  | 17 +
>  util/qemu-option.c   | 20 ++--
>  6 files changed, 34 insertions(+), 57 deletions(-)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index bc312e90aa..6178e98e97 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -19,6 +19,8 @@ typedef struct QEnumLookup {
>  const char *qapi_enum_lookup(const QEnumLookup *lookup, int val);
>  int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
>  int def, Error **errp);
> +bool qapi_bool_parse(const char *name, const char *value, bool *obj,
> + Error **errp);
>  
>  int parse_qapi_name(const char *name, bool complete);
>  
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 7781c23a42..587f31baf6 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -368,7 +368,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, 
> Error **errp)
>  }
>  
>  
> -/* mimics qemu-option.c::parse_option_bool() */
>  static bool
>  opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
>  {
> @@ -379,19 +378,8 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, 
> Error **errp)
>  if (!opt) {
>  return false;
>  }
> -
>  if (opt->str) {
> -if (strcmp(opt->str, "on") == 0 ||
> -strcmp(opt->str, "yes") == 0 ||
> -strcmp(opt->str, "y") == 0) {
> -*obj = true;
> -} else if (strcmp(opt->str, "off") == 0 ||
> -strcmp(opt->str, "no") == 0 ||
> -strcmp(opt->str, "n") == 0) {
> -*obj = false;
> -} else {
> -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
> -   "on|yes|y|off|no|n");
> +if (!qapi_bool_parse(opt->name, opt->str, obj, errp)) {
>  return false;
>  }
>  } else {
> diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
> index 29a6c98b53..4dd8f6c313 100644
> --- a/qapi/qapi-util.c
> +++ b/qapi/qapi-util.c
> @@ -13,6 +13,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/ctype.h"
> +#include "qapi/qmp/qerror.h"
>  
>  const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
>  {
> @@ -40,6 +41,28 @@ int qapi_enum_parse(const QEnumLookup *lookup, const char 
> *buf,
>  return def;
>  }
>  
> +bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error 
> **errp)
> +{
> +if (!strcasecmp(value, "on") ||
> +!strcasecmp(value, "yes") ||
> +!strcasecmp(value, "true") ||
> +!strcasecmp(value, "y")) {
> +*obj = true;
> +return true;
> +}
> +if (!strcasecmp(value, "off") ||
> +!strcasecmp(value, "no") ||
> +!strcasecmp(value, "false") ||
> +!strcasecmp(value, "n")) {
> +*obj = false;
> +return true;
> +}
> +
> +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +   "boolean (on/off, yes/no, true/false, y/n)");

Recommend to have the error message only mention the preferred form.  I
like the laconic "'on' or 'off'".  It's really all the user needs to
know.

> +return false;
> +}
> +
>  /*
>   * Parse a valid QAPI name from @str.
>   * A valid name consists of letters, digits, hyphen and underscore.
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 7b184b50a7..f4541a4fdd 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -512,16 +512,11 @@ static bool qobject_input_type_bool_keyval(Visitor *v, 
> const char *name,
>  return false;
>  }
>  
> -if (!strcmp(str, "on")) {
> -*obj = true;
> -} else if (!strcmp(str, "off")) {
> -*obj = false;
> -} else {
> -error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> -   full_name(qiv, name), "'on' or 'off'");
> -return false;
> -}
> -return true;
> +/*
> + * Calling full_name is a bit slow, but keyval (human/CLI) parsing
> + * is not a hot path.
> + */
> +return qapi_bool_parse(full_name(qiv, name), str, obj, errp);

Avoiding the full_name() on success isn't hard:

   if (!qapi_bool_parse(name,

Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-03 Thread Daniel P . Berrangé
On Tue, Nov 03, 2020 at 05:33:47PM +0100, Paolo Bonzini wrote:
> On 03/11/20 17:25, Daniel P. Berrangé wrote:
> >> OptsVisitor, StringInputVisitor and the keyval visitor have
> >> three different ideas of how a human could write the value of
> >> a boolean option.  Pay homage to the backwards-compatibility
> >> gods and make the new common helper accept all four sets (on/off,
> >> true/false, y/n and yes/no), and case-insensitive at that.
> >>
> >> Since OptsVisitor is supposed to match qemu-options, adjust
> >> it as well.
> > FWIW, libvirt does not appear to use true/false or y/n, nor
> > ever use uppercase / mixed case.
> > 
> > IOW this level of back compat may well be overkill.
> > 
> > I'd particular suggest deprecating case-insensitivity, as
> > Yes, YES, yEs feel unlikely to be important or widely used.
> 
> True; at least it's type-safe code unlike the short-form boolean option.
>  It only hurts in the odd case of a boolean option becoming on/off/auto
> or on/off/split.
> 
> I didn't want to introduce deprecation at this point, because
> consistency is better anyway even if we plan to later deprecate
> something.  For example, since there is a common parser now, introducing
> deprecation would be much easier.  It also lets us switch parsers even
> during the deprecation period (which is how I got into this mess).

Ok,

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-03 Thread Paolo Bonzini
On 03/11/20 17:25, Daniel P. Berrangé wrote:
>> OptsVisitor, StringInputVisitor and the keyval visitor have
>> three different ideas of how a human could write the value of
>> a boolean option.  Pay homage to the backwards-compatibility
>> gods and make the new common helper accept all four sets (on/off,
>> true/false, y/n and yes/no), and case-insensitive at that.
>>
>> Since OptsVisitor is supposed to match qemu-options, adjust
>> it as well.
> FWIW, libvirt does not appear to use true/false or y/n, nor
> ever use uppercase / mixed case.
> 
> IOW this level of back compat may well be overkill.
> 
> I'd particular suggest deprecating case-insensitivity, as
> Yes, YES, yEs feel unlikely to be important or widely used.

True; at least it's type-safe code unlike the short-form boolean option.
 It only hurts in the odd case of a boolean option becoming on/off/auto
or on/off/split.

I didn't want to introduce deprecation at this point, because
consistency is better anyway even if we plan to later deprecate
something.  For example, since there is a common parser now, introducing
deprecation would be much easier.  It also lets us switch parsers even
during the deprecation period (which is how I got into this mess).

Paolo




Re: [PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-03 Thread Daniel P . Berrangé
On Tue, Nov 03, 2020 at 11:13:39AM -0500, Paolo Bonzini wrote:
> OptsVisitor, StringInputVisitor and the keyval visitor have
> three different ideas of how a human could write the value of
> a boolean option.  Pay homage to the backwards-compatibility
> gods and make the new common helper accept all four sets (on/off,
> true/false, y/n and yes/no), and case-insensitive at that.
> 
> Since OptsVisitor is supposed to match qemu-options, adjust
> it as well.

FWIW, libvirt does not appear to use true/false or y/n, nor
ever use uppercase / mixed case.

IOW this level of back compat may well be overkill.

I'd particular suggest deprecating case-insensitivity, as
Yes, YES, yEs feel unlikely to be important or widely used.

Based on libvirt test suite QEMU CLI args, I see primarily on|off,
with a small amount of yes|no:

3dnowext=on
3dnow=on
3dnowprefetch=off
aarch64=off
acpi=on
adjust=on
adjvtime=on
aefsi=on
aen=on
apft=on
ap=on
append=on
apqci=on
apqi=on
assist=on
ats=off
ats=on
avx2=off
avx=off
bpb=on
cache=off
cache=on
clflushopt=on
clwb=on
cmmnt=on
cmm=on
cmp_legacy=on
compression=off
compression=on
core=off
core=on
cpl=on
csske=on
csum=off
cte=on
data=yes
dedicated=on
deflate=on
dfppc=on
display=on
ds_cpl=on
edat2=on
edat=on
eim=on
eoi=off
eoi=on
erms=on
esop=on
esort=on
etoken=on
event_idx=off
event_idx=on
f16c=off
failover=on
fma=off
fxsr_opt=off
fxsr_opt=on
gl=on
gs=on
gso=off
guest_csum=off
guest_ecn=off
guest_tso4=off
guest_tso6=off
guest_ufo=off
hmat=on
host_ecn=off
host_tso4=off
host_tso6=off
host_ufo=off
hotplug=off
hotplug=on
htm=on
hugetlb=yes
hv=off
hypervisor=on
iep=on
info=off
info=on
intremap=on
invtsc=on
ioeventfd=off
ioeventfd=on
iommu=on
iommu_platform=off
iommu_platform=on
iotlb=on
ipter=on
kvmclock=off
kvm=off
kvm_pv_eoi=off
kvm_pv_eoi=on
kvm_pv_unhalt=off
kvm_pv_unhalt=on
la57=on
lahf_lm=on
legacy=off
legacy=on
lm=on
lock=off
lock=on
logappend=off
logappend=on
master=off
master=on
menu=off
menu=on
mepoch=on
mepochptff=on
merge=off
migration=on
misalignsse=off
mlock=off
mlock=on
mode=on
modern=off
mouse=off
mpx=on
mq=on
mrg_rxbuf=off
msa1=on
msa2=on
msa3=on
msa4=on
msa5=on
msa6=on
msa7=on
msa8=on
msa9=on
msa9_pckmo=on
msi=on
msr=on
multifunction=off
multifunction=on
nb=on
ni=off
nodeid_msr=on
npt=on
nvdimm=on
oom=off
oom=on
opt=on
osvw=off
packed=off
packed=on
pclmuldq=on
pclmulqdq=on
pcommit=on
pconfig=off
pdpe1gb=on
peer=yes
perfctr_core=on
perfctr_nb=on
pku=on
pmem=yes
pmu=off
pmu=on
ppa15=on
prealloc=yes
ramfb=on
rdrand=off
rdseed=off
read=on
readonly=on
readonly=on'
removable=off
removable=on
reporting=on
ri=on
rw=on
save=off
scsi=off
scsi=on
sea_esop2=on
share=no
share=yes
smm=on
ssbd=on
ss=on
stckf=off
sthyi=on
strict=on
sve128=on
sve256=on
sve384=off
sve512=on
sve=on
te=on
threads=on
timestamp=on
topoext=off
tsc_adjust=on
umip=on
unarmed=on
unhalt=off
unhalt=on
usb=off
value=on
vhost=on
virgl=on
vme=off
vmport=off
vmx=on
vxeh2=on
vxeh=on
vx=on
vxpdeh=on
vxpd=on
wrap=off
wrap=on
xsavec=off
xsaves=on
zeroes=on
zpci=on



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




[PATCH v2] qapi, qemu-options: make all parsing visitors parse boolean options the same

2020-11-03 Thread Paolo Bonzini
OptsVisitor, StringInputVisitor and the keyval visitor have
three different ideas of how a human could write the value of
a boolean option.  Pay homage to the backwards-compatibility
gods and make the new common helper accept all four sets (on/off,
true/false, y/n and yes/no), and case-insensitive at that.

Since OptsVisitor is supposed to match qemu-options, adjust
it as well.

Supersedes: <20201103142344.402353-1-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 include/qapi/util.h  |  2 ++
 qapi/opts-visitor.c  | 14 +-
 qapi/qapi-util.c | 23 +++
 qapi/qobject-input-visitor.c | 15 +--
 qapi/string-input-visitor.c  | 17 +
 util/qemu-option.c   | 20 ++--
 6 files changed, 34 insertions(+), 57 deletions(-)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index bc312e90aa..6178e98e97 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -19,6 +19,8 @@ typedef struct QEnumLookup {
 const char *qapi_enum_lookup(const QEnumLookup *lookup, int val);
 int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
 int def, Error **errp);
+bool qapi_bool_parse(const char *name, const char *value, bool *obj,
+ Error **errp);
 
 int parse_qapi_name(const char *name, bool complete);
 
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7781c23a42..587f31baf6 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -368,7 +368,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, 
Error **errp)
 }
 
 
-/* mimics qemu-option.c::parse_option_bool() */
 static bool
 opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
 {
@@ -379,19 +378,8 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, 
Error **errp)
 if (!opt) {
 return false;
 }
-
 if (opt->str) {
-if (strcmp(opt->str, "on") == 0 ||
-strcmp(opt->str, "yes") == 0 ||
-strcmp(opt->str, "y") == 0) {
-*obj = true;
-} else if (strcmp(opt->str, "off") == 0 ||
-strcmp(opt->str, "no") == 0 ||
-strcmp(opt->str, "n") == 0) {
-*obj = false;
-} else {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
-   "on|yes|y|off|no|n");
+if (!qapi_bool_parse(opt->name, opt->str, obj, errp)) {
 return false;
 }
 } else {
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index 29a6c98b53..4dd8f6c313 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/ctype.h"
+#include "qapi/qmp/qerror.h"
 
 const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
 {
@@ -40,6 +41,28 @@ int qapi_enum_parse(const QEnumLookup *lookup, const char 
*buf,
 return def;
 }
 
+bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error 
**errp)
+{
+if (!strcasecmp(value, "on") ||
+!strcasecmp(value, "yes") ||
+!strcasecmp(value, "true") ||
+!strcasecmp(value, "y")) {
+*obj = true;
+return true;
+}
+if (!strcasecmp(value, "off") ||
+!strcasecmp(value, "no") ||
+!strcasecmp(value, "false") ||
+!strcasecmp(value, "n")) {
+*obj = false;
+return true;
+}
+
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+   "boolean (on/off, yes/no, true/false, y/n)");
+return false;
+}
+
 /*
  * Parse a valid QAPI name from @str.
  * A valid name consists of letters, digits, hyphen and underscore.
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 7b184b50a7..f4541a4fdd 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -512,16 +512,11 @@ static bool qobject_input_type_bool_keyval(Visitor *v, 
const char *name,
 return false;
 }
 
-if (!strcmp(str, "on")) {
-*obj = true;
-} else if (!strcmp(str, "off")) {
-*obj = false;
-} else {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-   full_name(qiv, name), "'on' or 'off'");
-return false;
-}
-return true;
+/*
+ * Calling full_name is a bit slow, but keyval (human/CLI) parsing
+ * is not a hot path.
+ */
+return qapi_bool_parse(full_name(qiv, name), str, obj, errp);
 }
 
 static bool qobject_input_type_str(Visitor *v, const char *name, char **obj,
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 6e53396ea3..197139c1c0 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -332,22 +332,7 @@ static bool parse_type_bool(Visitor *v, const char *name, 
bool *obj,
 StringInputVisitor *siv = to_siv(v);
 
 assert(siv->lm == LM_NONE);
-if (!strcasecmp(siv->string, "on") ||
-!strcasecmp(siv->string, "yes") ||
-!strcasecmp(siv->string, "true"