| Message ID | 20191024123458.13505-20-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Configurable policy for handling deprecated interfaces | expand |
On Thu, Oct 24, 2019 at 02:34:58PM +0200, Markus Armbruster wrote: > This policy suppresses deprecated events, and thus permits "testing > the future". One thing that occurs to me is that this is a fairly passive impact on libvirt. eg it may well be not at all obvious if libvirt is behaving in a broken way due to an event not being emitted, as the code in question simply won't be triggered. With the current QMP this situation is unavoidable since QEMU doesn't know which events the client (libvirt) is actually using. QEMU just unconditionally emits all events. I've often wondered if we should have the client explicitly tell QEMU which events it wants to receive as part of the QMP greeting handshake. ie, libvirt knows which events it can handle. QEMU knows which events it can emit, and reports this via capabilities which libvirt probes. So on connecting libvirt can tell QEMU exactly which evnets it wants to get back. QEMU is now able to explicitly tell libvirt it has asked for a deprecated event, and so the logic from the "deprecated-input" option can take effect. We'd not need "deprecated-output" at that point. Regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Thu, Oct 24, 2019 at 02:34:58PM +0200, Markus Armbruster wrote: >> This policy suppresses deprecated events, and thus permits "testing >> the future". > > One thing that occurs to me is that this is a fairly passive impact > on libvirt. eg it may well be not at all obvious if libvirt is behaving > in a broken way due to an event not being emitted, as the code in > question simply won't be triggered. Intented use of -compat deprecated-input=error,deprecated-output=hide is "testing the future": make QEMU behave as if the deprecated features were already gone. Can be useful when you want to test code that deals with the anticipated future *now*. It can also be used to ferret out unknown uses of deprecated interfaces: run test suite with it, see what fails. But as you note, the deprecated-output=hide part is somewhat problematic in that role. > With the current QMP this situation is unavoidable since QEMU doesn't > know which events the client (libvirt) is actually using. QEMU just > unconditionally emits all events. > > I've often wondered if we should have the client explicitly tell > QEMU which events it wants to receive as part of the QMP greeting > handshake. > > ie, libvirt knows which events it can handle. QEMU knows which > events it can emit, and reports this via capabilities which > libvirt probes. > > So on connecting libvirt can tell QEMU exactly which evnets it > wants to get back. QEMU is now able to explicitly tell libvirt > it has asked for a deprecated event, and so the logic from the > "deprecated-input" option can take effect. QEMU already reports its events via introspection. What's missing is an event subscription mechanism. Should be feasible. Additional benefit: can reduce I/O. > We'd not need "deprecated-output" at that point. If deprecated-input=error makes subscribing to a deprecated event fail, we don't need deprecated-output=hide for events. But events are not the only output: there's also command returns. Consider query-cpus-fast. Returns list of CpuInfoFast. CpuInfoFast member @arch is deprecated. deprecated-output=hide should hide it, except it's not implemented in this series. This is also "a fairly passive impact on libvirt", I'm afraid. We have some 40 events, and having libvirt subscribe to the ones it actually uses is obviously practical. I doubt the subscription idea scales up to return values.
diff --git a/qapi/common.json b/qapi/common.json index 06e54642bb..4e3da4beee 100644 --- a/qapi/common.json +++ b/qapi/common.json @@ -167,14 +167,14 @@ # Policy for handling "funny" output. # # @accept: Pass on unchanged -# TODO @hide: Filter out +# @hide: Filter out # # FIXME Guidance on intended use. # # Since: 4.2 ## { 'enum': 'CompatPolicyOutput', - 'data': [ 'accept' ] } + 'data': [ 'accept', 'hide' ] } ## # @CompatPolicy: diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c index 7dd0053190..303c8d6382 100644 --- a/tests/test-qmp-event.c +++ b/tests/test-qmp-event.c @@ -14,6 +14,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" +#include "qapi/compat-policy.h" #include "qapi/error.h" #include "qapi/qmp/qbool.h" #include "qapi/qmp/qdict.h" @@ -140,6 +141,21 @@ static void test_event_d(TestEventData *data, qobject_unref(data->expect); } +static void test_event_deprecated(TestEventData *data, const void *unused) +{ + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); + + qapi_event_send_test_event_features1(); + g_assert(data->emitted); + + qapi_compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; + data->emitted = false; + qapi_event_send_test_event_features1(); + g_assert(!data->emitted); + + qobject_unref(data->expect); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -148,6 +164,7 @@ int main(int argc, char **argv) event_test_add("/event/event_b", test_event_b); event_test_add("/event/event_c", test_event_c); event_test_add("/event/event_d", test_event_d); + event_test_add("/event/deprecated", test_event_deprecated); g_test_run(); return 0; diff --git a/qemu-options.hx b/qemu-options.hx index 645629457a..c0128813c6 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3320,7 +3320,7 @@ STEXI ETEXI DEF("compat", HAS_ARG, QEMU_OPTION_compat, - "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept]\n" + "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n" " Policy for handling deprecated management interfaces\n", QEMU_ARCH_ALL) STEXI @@ -3337,6 +3337,8 @@ Reject deprecated commands Crash on deprecated command @item deprecated-output=accept (default) Emit deprecated events +@item deprecated-output=hide +Suppress deprecated events @end table FIXME Guidance on intended use ETEXI diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index f64e61076e..5778fa1a0d 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -61,7 +61,8 @@ def gen_param_var(typ): return ret -def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit): +def gen_event_send(name, arg_type, features, boxed, + event_enum_name, event_emit): # FIXME: Our declaration of local variables (and of 'errp' in the # parameter list) can collide with exploded members of the event's # data type passed in as parameters. If this collision ever hits in @@ -86,6 +87,14 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit): if not boxed: ret += gen_param_var(arg_type) + if 'deprecated' in [f.name for f in features]: + ret += mcgen(''' + + if (qapi_compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) { + return; + } +''') + ret += mcgen(''' qmp = qmp_event_build_dict("%(name)s"); @@ -154,6 +163,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor): #include "%(prefix)sqapi-emit-events.h" #include "%(events)s.h" #include "%(visit)s.h" +#include "qapi/compat-policy.h" #include "qapi/error.h" #include "qapi/qmp/qdict.h" #include "qapi/qobject-output-visitor.h" @@ -192,7 +202,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict); def visit_event(self, name, info, ifcond, features, arg_type, boxed): with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_event_send_decl(name, arg_type, boxed)) - self._genc.add(gen_event_send(name, arg_type, boxed, + self._genc.add(gen_event_send(name, arg_type, features, boxed, self._event_enum_name, self._event_emit_name)) # Note: we generate the enum member regardless of @ifcond, to
This policy suppresses deprecated events, and thus permits "testing the future". No QMP event is deprecated right now. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qapi/common.json | 4 ++-- tests/test-qmp-event.c | 17 +++++++++++++++++ qemu-options.hx | 4 +++- scripts/qapi/events.py | 14 ++++++++++++-- 4 files changed, 34 insertions(+), 5 deletions(-)