Libguestfs - Sep 2020 - Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

New option -compat lets you configure what to do when deprecated
interfaces get used.  This is intended for testing users of the
management interfaces.  It is experimental.

-compat deprecated-input=<in-policy> configures what to do when
deprecated input is received.  Available policies:

* accept: Accept deprecated commands and arguments (default)
* reject: Reject them
* crash: Crash

-compat deprecated-output=<out-policy> configures what to do when
deprecated output is sent.  Available output policies:

* accept: Emit deprecated command results and events (default)
* hide: Suppress them

For now, -compat covers only deprecated syntactic aspects of QMP.  We
may want to extend it to cover semantic aspects, CLI, and experimental
features.

v5:
* Old PATCH 01-26 merged in commit f57587c7d47.
* Rebased, non-trivial conflicts in PATCH 1 due to Meson, and in PATCH
  7 due to visitor changes
* PATCH 1: Comments updated for 5.2 [Eric]
* PATCH 2: Harmless missing initialization fixed [Eric]
* PATCH 3+4: Harmless missing has_FOO = true fixed [Eric]
* PATCH 6+7: Commit message tweaked

v4:
* PATCH 05+07: Temporary memory leak plugged [Marc-André]
* PATCH 23: Rewritten [Marc-André]
* PATCH 24: Comment typo [Marc-André]
* PATCH 30: Memory leaks plugged

v3:
* Rebased, non-trivial conflicts in PATCH 01+26+27+34 due to RST
  conversion and code motion
* PATCH 28-29: Old PATCH 28 split up to ease review
* PATCH 30-31: New
* PATCH 32-33: Old PATCH 29 split up to ease review

Comparison to RFC (24 Oct 2019):
* Cover arguments and results in addition to commands and events
* Half-baked "[RFC PATCH 18/19] qapi: Include a warning in the
  response to a deprecated command" dropped

See also last item of
    Subject: Minutes of KVM Forum BoF on deprecating stuff
    Date: Fri, 26 Oct 2018 16:03:51 +0200
    Message-ID: <87mur0ls8o.fsf@dusky.pond.sub.org>
    https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html

Cc: Lukáš Doktor <ldoktor@redhat.com>
Cc: libguestfs@redhat.com
Cc: libvir-list@redhat.com
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Peter Krempa <pkrempa@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>

Markus Armbruster (8):
  qemu-options: New -compat to set policy for deprecated interfaces
  qapi: Implement deprecated-output=hide for QMP command results
  qapi: Implement deprecated-output=hide for QMP events
  qapi: Implement deprecated-output=hide for QMP event data
  qapi: Implement deprecated-output=hide for QMP introspection
  qapi: Implement deprecated-input=reject for QMP commands
  qapi: Implement deprecated-input=reject for QMP command arguments
  qapi: New -compat deprecated-input=crash

 qapi/compat.json                        |  52 ++++++++++++
 qapi/introspect.json                    |   2 +-
 qapi/qapi-schema.json                   |   1 +
 include/qapi/compat-policy.h            |  20 +++++
 include/qapi/qmp/dispatch.h             |   1 +
 include/qapi/qobject-input-visitor.h    |   9 +++
 include/qapi/qobject-output-visitor.h   |   9 +++
 include/qapi/visitor-impl.h             |   6 ++
 include/qapi/visitor.h                  |  18 +++++
 monitor/monitor-internal.h              |   3 -
 monitor/misc.c                          |   2 -
 monitor/qmp-cmds-control.c              | 100 +++++++++++++++++++++---
 qapi/qapi-visit-core.c                  |  18 +++++
 qapi/qmp-dispatch.c                     |  17 ++++
 qapi/qobject-input-visitor.c            |  29 +++++++
 qapi/qobject-output-visitor.c           |  19 +++++
 softmmu/vl.c                            |  17 ++++
 storage-daemon/qemu-storage-daemon.c    |   2 -
 tests/test-qmp-cmds.c                   |  91 +++++++++++++++++++--
 tests/test-qmp-event.c                  |  41 ++++++++++
 qapi/meson.build                        |   1 +
 qapi/trace-events                       |   2 +
 qemu-options.hx                         |  22 ++++++
 scripts/qapi/commands.py                |  14 ++--
 scripts/qapi/events.py                  |  22 +++++-
 scripts/qapi/visit.py                   |  15 ++++
 tests/qapi-schema/qapi-schema-test.json |  20 +++--
 tests/qapi-schema/qapi-schema-test.out  |  20 ++---
 28 files changed, 522 insertions(+), 51 deletions(-)
 create mode 100644 qapi/compat.json
 create mode 100644 include/qapi/compat-policy.h

-- 
2.26.2

Some general comments on using the patch:

* For libguestfs I chose to add

  -compat deprecated-input=reject,deprecated-output=hide

  This is only enabled in developer builds of libguestfs when we
  are running qemu directly (not via libvirt).  The patch for
  this is attached.

* What's the point/difference in having reject vs crash?

* I hope that by hiding deprecated QAPI fields we may detect
  errors in libguestfs, but I suspect what'll happen is it'll
  cause fall-back behaviour which might be harder to detect.

* Be *really* good to have this for command line parameters!

Notes on the attached patch:

* https://libguestfs.org/guestfs-building.1.html

* Simple test:

LIBGUESTFS_BACKEND=direct \
LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
./run libguestfs-test-tool

* Run the full test suite:

LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
make -k check-direct

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Mon, Sep 21, 2020 at 13:45:14 +0100, Richard W.M. Jones
wrote:
> Some general comments on using the patch:
> 
> * For libguestfs I chose to add
> 
>   -compat deprecated-input=reject,deprecated-output=hide
> 
>   This is only enabled in developer builds of libguestfs when we
>   are running qemu directly (not via libvirt).  The patch for
>   this is attached.
> 
> * What's the point/difference in having reject vs crash?
I'll be adding the following documentation for the qemu.conf entry in
libvirt controling the feature:

+# The "reject" option is less harsh towards the VMs but some code
paths ignore
+# errors reported by qemu and thus it may not be obvious that a deprecated
+# command/field was used, thus it's suggested to use the "crash"
option instead.

On Mon, 14 Sep 2020 at 09:55, Markus Armbruster <armbru@redhat.com>
wrote:
>
> New option -compat lets you configure what to do when deprecated
> interfaces get used.  This is intended for testing users of the
> management interfaces.  It is experimental.
>
> -compat deprecated-input=<in-policy> configures what to do when
> deprecated input is received.  Available policies:
>
> * accept: Accept deprecated commands and arguments (default)
> * reject: Reject them
> * crash: Crash
>
> -compat deprecated-output=<out-policy> configures what to do when
> deprecated output is sent.  Available output policies:
>
> * accept: Emit deprecated command results and events (default)
> * hide: Suppress them
>
> For now, -compat covers only deprecated syntactic aspects of QMP.  We
> may want to extend it to cover semantic aspects, CLI, and experimental
> features.
Some bikeshedding on option naming...

If this only covers QMP, should we make the argument to -compat
have a name that expresses that? eg deprecated-qmp-input,
deprecated-qmp-output ?

Also, it seems a bit repetitive to say 'deprecated' here all
the time -- do you have a future use of -compat in mind which
would be to adjust something that is *not* deprecated ? If
not, maybe the 'deprecated' part should be in the option name
rather than in every argument, eg

  -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject

?

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:
> On Mon, 14 Sep 2020 at 09:55, Markus Armbruster <armbru@redhat.com>
wrote:
>>
>> New option -compat lets you configure what to do when deprecated
>> interfaces get used.  This is intended for testing users of the
>> management interfaces.  It is experimental.
>>
>> -compat deprecated-input=<in-policy> configures what to do when
>> deprecated input is received.  Available policies:
>>
>> * accept: Accept deprecated commands and arguments (default)
>> * reject: Reject them
>> * crash: Crash
>>
>> -compat deprecated-output=<out-policy> configures what to do when
>> deprecated output is sent.  Available output policies:
>>
>> * accept: Emit deprecated command results and events (default)
>> * hide: Suppress them
>>
>> For now, -compat covers only deprecated syntactic aspects of QMP.  We
>> may want to extend it to cover semantic aspects, CLI, and experimental
>> features.
>
> Some bikeshedding on option naming...
>
> If this only covers QMP, should we make the argument to -compat
> have a name that expresses that? eg deprecated-qmp-input,
> deprecated-qmp-output ?
It's only implemented for QMP so far.  But we really want it for all
external interfaces for use by machines.  Today, that's QMP and CLI.

Naming the parameters deprecated-qmp-{input,output} leads to separate
settings for QMP and CLI.

Naming them just deprecated-{input,output} leads to a single set of
settings common to all externeal interfaces, or to sugar for setting all
the deprecated-*-{input,output} we may have.

I don't think getting it wrong now would be a big deal.  No excuse for
getting it wrong unthinkingly :)
> Also, it seems a bit repetitive to say 'deprecated' here all
> the time -- do you have a future use of -compat in mind which
> would be to adjust something that is *not* deprecated ? If
> not, maybe the 'deprecated' part should be in the option name
> rather than in every argument, eg
>
>   -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject
>
> ?
My cover letter hints at such future uses: "We may want to extend it to
cover [...] experimental features."  Something like

    -compat experimental-input=reject,experimental-output=hide