Daniel P. Berrangé <berra...@redhat.com> writes:
> This removes the TARGET_I386 condition from the rtc-reset-reinjection
> command. This requires providing a QMP command stub for non-i386 target.
> This in turn requires moving the command out of misc-target.json, since
> that will trigger symbol poisoning errors when built from target
> independent code.
>
> Rather than putting the command into misc.json, it is proposed to create
> misc-$TARGET.json files to hold commands whose impl is conceptually
> only applicable to a single target. This gives an obvious docs hint to
> consumers that the command is only useful in relation a specific target,
> while misc.json is for commands applicable to 2 or more targets.
Starting with this patch, the series structures the manual like this:
= Machines
... contents of machine.json ...
== Specific to S390
... contents of machine-s390.json ...
and
= Miscellanea
... contents of misc.json ...
== Specific to ARM
... contents of misc-arm.json ...
== Specific to i386
... contents of misc-i386.json ...
Except it doesn't add == subsection headers, but that's detail. The
text I show for them here is crap.
Possible alternative: collect the target-specific stuff in one place
rather than two:
= Targets
== ARM
== i386
== S390
Again the header text is crap.
Is separating the current contents of misc-<target>.json from
machine-<target>.json useful?
> The current impl of qmp_rtc_reset_reinject() is a no-op if the i386
> RTC is disabled in Kconfig, or if the running machine type lack any
> RTC device. Thus the stub impl for non-i386 targets retains this
> no-op behaviour, instead of reporting a Error which is the more usual
> choice for commands invoked against unsupported configurations.
>
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
> hw/i386/monitor.c | 2 +-
> qapi/meson.build | 1 +
> qapi/misc-i386.json | 24 ++++++++++++++++++++++++
> qapi/misc-target.json | 17 -----------------
> qapi/qapi-schema.json | 1 +
> stubs/meson.build | 1 +
> stubs/monitor-i386-rtc.c | 10 ++++++++++
> 7 files changed, 38 insertions(+), 18 deletions(-)
> create mode 100644 qapi/misc-i386.json
> create mode 100644 stubs/monitor-i386-rtc.c
>
> diff --git a/hw/i386/monitor.c b/hw/i386/monitor.c
> index 1921e4d52e..79df96562f 100644
> --- a/hw/i386/monitor.c
> +++ b/hw/i386/monitor.c
> @@ -26,7 +26,7 @@
> #include "monitor/monitor.h"
> #include "qobject/qdict.h"
> #include "qapi/error.h"
> -#include "qapi/qapi-commands-misc-target.h"
> +#include "qapi/qapi-commands-misc-i386.h"
> #include "hw/i386/x86.h"
> #include "hw/rtc/mc146818rtc.h"
>
> diff --git a/qapi/meson.build b/qapi/meson.build
> index eadde4db30..3a9bd06104 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -64,6 +64,7 @@ if have_system
> 'qdev',
> 'pci',
> 'rocker',
> + 'misc-i386',
> 'tpm',
> 'uefi',
> ]
> diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
> new file mode 100644
> index 0000000000..d5bfd91405
> --- /dev/null
> +++ b/qapi/misc-i386.json
> @@ -0,0 +1,24 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
Might be cleaner to add this to all qapi/*.json first, and in a separate
patch.
> +
> +##
> +# @rtc-reset-reinjection:
> +#
> +# This command will reset the RTC interrupt reinjection backlog. Can
> +# be used if another mechanism to synchronize guest time is in effect,
> +# for example QEMU guest agent's guest-set-time command.
> +#
> +# Use of this command is only applicable for x86 machines with an RTC,
> +# and on other machines will silently return without performing any
> +# action.
This paragraph replaces ...
> +#
> +# Since: 2.1
> +#
> +# .. qmp-example::
> +#
> +# -> { "execute": "rtc-reset-reinjection" }
> +# <- { "return": {} }
> +##
> +{ 'command': 'rtc-reset-reinjection' }
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 42e4a7417d..5d0ffb0164 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -2,23 +2,6 @@
> # vim: filetype=python
> #
>
> -##
> -# @rtc-reset-reinjection:
> -#
> -# This command will reset the RTC interrupt reinjection backlog. Can
> -# be used if another mechanism to synchronize guest time is in effect,
> -# for example QEMU guest agent's guest-set-time command.
> -#
> -# Since: 2.1
> -#
> -# .. qmp-example::
> -#
> -# -> { "execute": "rtc-reset-reinjection" }
> -# <- { "return": {} }
> -##
> -{ 'command': 'rtc-reset-reinjection',
> - 'if': 'TARGET_I386' }
... the conditional.
Before, attempting to execute the command fails with CommandNotFound.
Afterwards it succeeds without doing anything. I think it should fail
instead. CommandNotFound would be a lie, so change it to GenericError.
"Specific to target" is no longer machine-readable. Should
machine-readability be (or become) desirable, we could expose it via
suitable QAPI features, e.g. 'features': ['target-i386'].
> -
> ##
> # @SevState:
> #
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 7bc600bb76..96f6aa4413 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -61,6 +61,7 @@
> { 'include': 'replay.json' }
> { 'include': 'yank.json' }
> { 'include': 'misc.json' }
> +{ 'include': 'misc-i386.json' }
> { 'include': 'misc-target.json' }
> { 'include': 'audio.json' }
> { 'include': 'acpi.json' }
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 63392f5e78..9907b54c1e 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -77,6 +77,7 @@ if have_system
> stub_ss.add(files('target-monitor-defs.c'))
> stub_ss.add(files('win32-kbd-hook.c'))
> stub_ss.add(files('xen-hw-stub.c'))
> + stub_ss.add(files('monitor-i386-rtc.c'))
> endif
>
> if have_system or have_user
> diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
> new file mode 100644
> index 0000000000..ee2e60d95b
> --- /dev/null
> +++ b/stubs/monitor-i386-rtc.c
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-misc-i386.h"
> +
> +void qmp_rtc_reset_reinjection(Error **errp)
> +{
> + /* Nothing to do since non-x86 machines lack an RTC */
> +}
I think I'd create one stub file per qapi/<foo>-<target>.json.
