[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] qapi: move ShutdownCause to qapi/run-state.
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] qapi: move ShutdownCause to qapi/run-state.json |
Date: |
Fri, 30 Nov 2018 10:15:36 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
One more thing...
Markus Armbruster <address@hidden> writes:
> Dominik Csapak <address@hidden> writes:
>
>> this makes it possible to use the reason as return value
>> in a QMP event, such as SHUTDOWN
>
> Please start your sentences with a capital letter and end them with
> punctuation.
>
> The patch's purpose could perhaps be stated a bit more clearly. Let me
> try:
>
> qapi: Turn ShutdownCause into a QAPI enum
>
> Needed so the patch after next can add ShutdownCause to QMP events
> SHUTDOWN and RESET.
>
>> Signed-off-by: Dominik Csapak <address@hidden>
>> ---
>> include/sysemu/sysemu.h | 20 --------------------
>> qapi/run-state.json | 33 +++++++++++++++++++++++++++++++++
>> 2 files changed, 33 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 8d6095d98b..f83522c7e7 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -31,26 +31,6 @@ VMChangeStateEntry
>> *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>> void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
>> void vm_state_notify(int running, RunState state);
>>
>> -/* Enumeration of various causes for shutdown. */
>> -typedef enum ShutdownCause {
>> - SHUTDOWN_CAUSE_NONE, /* No shutdown request pending */
>> - SHUTDOWN_CAUSE_HOST_ERROR, /* An error prevents further use of guest
>> */
>> - SHUTDOWN_CAUSE_HOST_QMP, /* Reaction to a QMP command, like 'quit'
>> */
>> - SHUTDOWN_CAUSE_HOST_SIGNAL, /* Reaction to a signal, such as SIGINT */
>> - SHUTDOWN_CAUSE_HOST_UI, /* Reaction to UI event, like window
>> close */
>> - SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest shutdown/suspend request, via
>> - ACPI or other hardware-specific means
>> */
>> - SHUTDOWN_CAUSE_GUEST_RESET, /* Guest reset request, and command line
>> - turns that into a shutdown */
>> - SHUTDOWN_CAUSE_GUEST_PANIC, /* Guest panicked, and command line turns
>> - that into a shutdown */
>> - SHUTDOWN_CAUSE_SUBSYSTEM_RESET,/* Partial guest reset that does not
>> trigger
>> - QMP events and ignores --no-reboot.
>> This
>> - is useful for sanitize hypercalls on
>> s390
>> - that are used during kexec/kdump/boot
>> */
>> - SHUTDOWN_CAUSE__MAX,
>> -} ShutdownCause;
>> -
>> static inline bool shutdown_caused_by_guest(ShutdownCause cause)
>> {
>> return cause >= SHUTDOWN_CAUSE_GUEST_SHUTDOWN;
Because of this, order matters. Your patch moves the enumeration away,
so readers get even less of a chance to catch this.
>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>> index 332e44897b..883bed167c 100644
>> --- a/qapi/run-state.json
>> +++ b/qapi/run-state.json
>> @@ -60,6 +60,39 @@
>> 'guest-panicked', 'colo', 'preconfig' ] }
>>
>> ##
>> +# @ShutdownCause:
>> +#
>> +# An enumeration of reasons for a Shutdown.
>> +#
>> +# @none: No shutdown request pending
>> +#
>> +# @host-error: An error prevented further use of guest
>
> Any particular reason for changing the tense from "prevents" to
> "prevented"?
>
>> +#
>> +# @host-qmp: Reaction to a QMP command, like 'quit'
>> +#
>> +# @host-signal: Reaction to a signal, such as SIGINT
>> +#
>> +# @host-ui: Reaction to a UI event, like window close
>> +#
>> +# @guest-shutdown: Guest shutdown/suspend request, via ACPI or other
>> +# hardware-specific means
>> +#
>> +# @guest-reset: Guest reset request, and command line turns that into
>> +# a shutdown
>> +#
>> +# @guest-panic: Guest panicked, and command line turns that into a shutdown
>> +#
>> +# @subsystem-reset: Partial guest reset that does not trigger QMP events and
>> +# ignores --no-reboot. This is useful for sanitizing
>> +# hypercalls on s390 that are used during kexec/kdump/boot
>> +#
>> +##
>> +{ 'enum': 'ShutdownCause',
Let's add
# Beware, shutdown_caused_by_guest() depends on enumeration order
here. Adding it to the doc comment above would be no good, because
that's external documentation (it would end up in the QEMU QMP reference
manual).
>> + 'data': [ 'none', 'host-error', 'host-qmp', 'host-signal', 'host-ui',
>> + 'guest-shutdown', 'guest-reset', 'guest-panic',
>> + 'subsystem-reset'] }
>> +
>> +##
>> # @StatusInfo:
>> #
>> # Information about VCPU run state