Re: [PATCH for-5.2 4/4] qemu-option: warn for short-form boolean options

[Daniel P . Berrangé]

On Tue, Nov 03, 2020 at 10:14:52AM -0500, Paolo Bonzini wrote:
> Options such as "server" or "nowait", that are commonly found in -chardev,
> are sugar for "server=on" and "wait=off".  This is quite surprising and
> also does not have any notion of typing attached.  It is even possible to
> do "-device e1000,noid" and get a device with "id=off".
> 
> Deprecate all this, except for -chardev and -spice where it is in
> wide use.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  chardev/char.c             |  1 +
>  docs/system/deprecated.rst |  7 +++++++
>  include/qemu/option.h      |  1 +
>  tests/test-qemu-opts.c     |  1 +
>  ui/spice-core.c            |  1 +
>  util/qemu-option.c         | 22 +++++++++++++++-------
>  6 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/chardev/char.c b/chardev/char.c
> index 78553125d3..108da615df 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -829,6 +829,7 @@ Chardev *qemu_chr_find(const char *name)
>  
>  QemuOptsList qemu_chardev_opts = {
>      .name = "chardev",
> +    .allow_flag_options = true, /* server, nowait, etc. */
>      .implied_opt_name = "backend",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_chardev_opts.head),
>      .desc = {
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 32a0e620db..0e7edf7e56 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -146,6 +146,13 @@ Drives with interface types other than ``if=none`` are 
> for onboard
>  devices.  It is possible to use drives the board doesn't pick up with
>  -device.  This usage is now deprecated.  Use ``if=none`` instead.
>  
> +Short-form boolean options (since 5.2)
> +''''''''''''''''''''''''''''''''''''''
> +
> +Boolean options such as ``share=on``/``share=off`` can be written
> +in short form as ``share`` and ``noshare``.  This is deprecated
> +for all command-line options except ``-chardev` and ``-spice``, for
> +which the short form was in wide use.

So IIUC, the short form was possible to use for absolutely /any/
boolean property ?

IMHO if we're going to deprecate short forms, we should do it
universally including chardev and spice. Arguably spice/chardev
are the most important ones to give an explicit warning about
precisely because their widespread usage means a heads up is
important to users.  For chardev in particular it is possible
we might end up wanting to wait longer than the usual 2 cycles
before removal. So if we're serious about removing the short
forms long term, the sooner we deprecate & warn the better
for chardev.


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 :|