Hi On Tue, Dec 18, 2018 at 10:26 PM Markus Armbruster <arm...@redhat.com> wrote: > > Marc-André posted v1 that relies on a QAPI schema language extension > 'top-unit' to permit splitting the generated code. Here is his cover > letter: > > The thrid and last part (of "[PATCH v2 00/54] qapi: add #if > pre-processor conditions to generated code") is about adding schema > conditions based on the target. > > For now, the qapi code is compiled in common objects (common to all > targets). This makes it impossible to add #if TARGET_ARM for example. > > The patch "RFC: qapi: learn to split the schema by 'top-unit'" > proposes to split the schema by "top-unit", so that generated code can > be built either in common objects or per-target. That patch is a bit > rough, I would like to get some feedback about the approach before > trying to improve it. The following patches demonstrate usage of > target-based #if conditions, and getting rid of the > qmp_unregister_command() hack. > > We already have a way to split generated code: QAPI modules. I took > the liberty to rework Marc-André's series to use a target module > instead. Less code, no change to the QAPI language. > > One of the patches (marked WIP) is a total hack. Fixable, but I want > to get this out for discussion first.
The approach you propose includes -target.h header in the top headers, effectively making all the qapi code target-dependent, no? I am actually a bit surprised there are no poisoined define errors. Possibly because the top-level header is rarely included. By contrast, my approach has the advantage of a clean split between target and non-target dependent code, which I would feel more confident about. That's the reason why I promptly discarded the QAPI modules approach without having second thoughts at least. Now you force me to reconsider it though. > > Marc-André Lureau (9): > build-sys: move qmp-introspect per target > qapi: make rtc-reset-reinjection and SEV depend on TARGET_I386 > qapi: make s390 commands depend on TARGET_S390X > target.json: add a note about query-cpu* not being s390x-specific > qapi: make query-gic-capabilities depend on TARGET_ARM > qapi: make query-cpu-model-expansion depend on s390 or x86 > qapi: make query-cpu-definitions depend on specific targets > qapi: remove qmp_unregister_command() > qapi: move RTC_CHANGE to the target schema > > Markus Armbruster (6): > qapi: Belatedly update docs for commit 9c2f56e9f9d > qapi: Eliminate indirection through qmp_event_get_func_emit() > qapi: Generate QAPIEvent stuff into separate files WIP > qapi: New module target.json > Revert "qapi-events: add 'if' condition to implicit event enum" > qmp: Deprecate query-events in favor of query-qmp-schema > > Makefile | 1 + > Makefile.objs | 22 +- > Makefile.target | 12 + > docs/devel/qapi-code-gen.txt | 12 +- > hw/ppc/spapr_rtc.c | 2 +- > hw/s390x/s390-skeys.c | 2 +- > hw/timer/mc146818rtc.c | 4 +- > include/qapi/qmp-event.h | 6 - > include/qapi/qmp/dispatch.h | 1 - > include/sysemu/arch_init.h | 11 - > monitor.c | 92 +---- > qapi/misc.json | 485 +--------------------- > qapi/qapi-schema.json | 1 + > qapi/qmp-event.c | 12 - > qapi/qmp-registry.c | 8 - > qapi/target.json | 514 ++++++++++++++++++++++++ > qemu-deprecated.texi | 5 + > qmp.c | 26 -- > scripts/qapi/events.py | 51 ++- > stubs/Makefile.objs | 4 - > stubs/arch-query-cpu-def.c | 11 - > stubs/arch-query-cpu-model-baseline.c | 13 - > stubs/arch-query-cpu-model-comparison.c | 13 - > stubs/arch-query-cpu-model-expansion.c | 13 - > stubs/monitor.c | 5 + > target/arm/helper.c | 3 +- > target/arm/monitor.c | 2 +- > target/i386/cpu.c | 7 +- > target/i386/sev_i386.h | 2 +- > target/ppc/translate_init.inc.c | 3 +- > target/s390x/cpu_models.c | 9 +- > tests/Makefile.include | 4 +- > tests/test-qmp-event.c | 7 +- > tests/test-qobject-input-visitor.c | 1 - > ui/vnc.c | 3 +- > 35 files changed, 623 insertions(+), 744 deletions(-) > create mode 100644 qapi/target.json > delete mode 100644 stubs/arch-query-cpu-def.c > delete mode 100644 stubs/arch-query-cpu-model-baseline.c > delete mode 100644 stubs/arch-query-cpu-model-comparison.c > delete mode 100644 stubs/arch-query-cpu-model-expansion.c > > -- > 2.17.2 > > -- Marc-André Lureau