qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: libvir-list@redhat.com, qemu-devel@nongnu.org
Subject: Re: [libvirt] [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command
Date: Thu, 24 Oct 2019 15:01:46 +0100	[thread overview]
Message-ID: <20191024140146.GC8381@redhat.com> (raw)
In-Reply-To: <20191024123458.13505-19-armbru@redhat.com>

On Thu, Oct 24, 2019 at 02:34:57PM +0200, Markus Armbruster wrote:
> Looks like this
> 
>     ---> {"execute": "query-cpus"}
>     <--- {"return": [...], "warnings": [{"class": "CommandNotFound", "desc": "command is deprecated"}]}
> 
> Management applications may want to log such warnings.
> 
> This commit is not for merging as is, because
> 
> * docs/interop/qmp-spec.txt needs an update for the new success
>   response member "warnings".
> 
> * I'd like to see a prospective user before I extend the QMP protocol.
>   If you have specific plans to put them to use, let me know.

Thinking about libvirt's usage of QMP

 - A public API call may result in many QMP commands being run.
 - Public APIs don't have any convenient way to report deprecated
   usage synchronously at runtime.
 - The set of QMP comamnds used by libvirt is a private impl
   detail that a mgmt app shouldn't know about

Some (most) deprecations will be things targetted at libvirt
developers, where libvirt just needs fixing to use some new
alternative instead.

Other deprecations where there's no replacement provided by QEMU
are things where an application might need to be told to stop
using the feature. From libvirt's public API POV the feature
likely won't be deprecated, only the specific usage of that
feature with the QEMU driver. eg consider QEMU decides to
stop POSTCOPY migration for some reason. Its deprecated from
POV of QEMU & QMP commands. If Xen or ESX support POSTCOPY
though, its not deprecated from libvirt's API POV. In many
ways this becomes a capabilities reporting problem between
libvirt & the application. Libvirt needs to tell the app which
features they can use, given their curent open libvirt connection
and VM instance(s).

So, either way, I don't think the QMP deprecations are something
we would want to expose to applications 'as is', since they're
either not something an app dev can fix, or they need rephrasing
in terms of the libvirt API/config feature the app is using, or
translating into a way for libvirt to expose capabilities to apps.

Libvirt could potentially log the deprecation warning in the per
QEMU VM log file. If end users see such log messages they'll
probably file support tickets / bug reports against libvirt and/or
the mgmt app, which will alert their maintainers to the fact. THis
could be useful if the maintainers missed the QEMU documentation
update listing the deprecation. It could be annoying if libvirt
knows it is deprecated though, and intentionally is still using
it in this particular version, with plans already present to fix
it in future.   So if libvirt does log the deprecations to the
VM log file, we'll probably want to /not/ log certain deprecations
that we're intentionally ignoring (temporarily).

In theory libvirt could see the deprecation reply and take
different action, but I don't much like that idea. It is too
late becasue we've already run the command, and its providing
a second way to deal with capabilities. We should be able to
query/probe the right way to invoke commands upfront, so that
we avoid using deprecated stuff in the first place.

> * The same warning should be included in a deprecated event.
> 
> * Emitting the same warning over and over again might be annoying or
>   slow.  Perhaps warning just once would be better.

If written to a log file, any single deprecation definitely
needs to be limited to once only per QEMU process lifetime.
Once a libvirt/qemu pair is deployed on a host it may be a
long time before an upgrade is done that pulls in the new
libvirt to avoid the deprecation. So we don't want to be
spamming logs of an otherwise fully functional VM.

In summary, it is probably reasonable to include this info in the QMP
command reply, but don't expect much to be done with it beyond possibly
writing it to a log file.

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



  reply	other threads:[~2019-10-24 15:04 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 01/19] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 02/19] tests/test-qmp-cmds: Check responses more thoroughly Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 03/19] tests/test-qmp-cmds: Simplify test data setup Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 04/19] tests/test-qmp-event: " Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 05/19] tests/test-qmp-event: Use qobject_is_equal() Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 06/19] tests/test-qmp-event: Check event is actually emitted Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 07/19] qapi: Add feature flags to remaining definitions Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 08/19] qapi: Consistently put @features parameter right after @ifcond Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 09/19] qapi: Inline do_qmp_dispatch() into qmp_dispatch() Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 10/19] qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 11/19] qapi: Simplify how qmp_dispatch() gets the request ID Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 12/19] qapi: Replace qmp_dispatch()'s TODO comment by an explanation Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 13/19] qapi: New special feature flag "deprecated" Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 14/19] qemu-options: New -compat to set policy for "funny" interfaces Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 15/19] qapi: Mark deprecated QMP commands with feature 'deprecated' Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 16/19] qapi: Implement -compat deprecated-input=reject for commands Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 17/19] qapi: Implement -compat deprecated-input=crash " Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command Markus Armbruster
2019-10-24 14:01   ` Daniel P. Berrangé [this message]
2019-10-24 19:35     ` [libvirt] " Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events Markus Armbruster
2019-10-24 14:16   ` [libvirt] " Daniel P. Berrangé
2019-10-24 19:56     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191024140146.GC8381@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).