Markus Armbruster <arm...@redhat.com> writes:
> Hu Tao <hu...@cn.fujitsu.com> writes:
>
>> The guest will be in this state when it is panicked.
>>
>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
>> Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
>> ---
>> include/sysemu/sysemu.h | 1 +
>> qapi-schema.json | 5 ++++-
>> qmp.c | 3 +--
>> vl.c | 13 +++++++++++--
>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index b19ec95..0412a8a 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -22,6 +22,7 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid);
>> bool runstate_check(RunState state);
>> void runstate_set(RunState new_state);
>> int runstate_is_running(void);
>> +bool runstate_needs_reset(void);
>> typedef struct vm_change_state_entry VMChangeStateEntry;
>> typedef void VMChangeStateHandler(void *opaque, int running, RunState
>> state);
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 28b070f..003cbf2 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -174,11 +174,14 @@
>> # @suspended: guest is suspended (ACPI S3)
>> #
>> # @watchdog: the watchdog action is configured to pause and has been
>> triggered
>> +#
>> +# @guest-panicked: guest has been panicked as a result of guest OS panic
>> ##
>> { 'enum': 'RunState',
>> 'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>> 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
>> - 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
>> + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
>> + 'guest-panicked' ] }
>>
>> ##
>> # @SnapshotInfo
>
> RUN_STATE_GUEST_PANICKED is similar to RUN_STATE_INTERNAL_ERROR and
> RUN_STATE_SHUTDOWN: the only way for the guest to continue is reset.
> Correct?
>
>> diff --git a/qmp.c b/qmp.c
>> index 55b056b..a1ebb5d 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -149,8 +149,7 @@ void qmp_cont(Error **errp)
>> {
>> Error *local_err = NULL;
>>
>> - if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
>> - runstate_check(RUN_STATE_SHUTDOWN)) {
>> + if (runstate_needs_reset()) {
>> error_set(errp, QERR_RESET_REQUIRED);
>> return;
>> } else if (runstate_check(RUN_STATE_SUSPENDED)) {
>> diff --git a/vl.c b/vl.c
>> index c03edf1..926822b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -566,6 +566,7 @@ static const RunStateTransition
>> runstate_transitions_def[] = {
>> { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
>> { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>> { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>> + { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>>
>> { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>>
>
> This permits runstate_set(RUN_STATE_GUEST_PANICKED) in
> RUN_STATE_RUNNING. Used in PATCH 3/4. Good.
>
>> @@ -580,6 +581,8 @@ static const RunStateTransition
>> runstate_transitions_def[] = {
>> { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>> { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>>
>> + { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
>> +
>> { RUN_STATE_MAX, RUN_STATE_MAX },
>> };
>
> This permits runstate_set(RUN_STATE_PAUSED) in RUN_STATE_GUEST_PANICKED.
> An example for such a transition is in the last patch hunk: main loop
> resetting the guest.
>
> Let's examine the other transitions to RUN_STATE_PAUSED, and whether
> they can now come from RUN_STATE_GUEST_PANICKED:
>
> * gdb_read_byte()
>
> No, because vm_stop() is protected by runstate_is_running() here.
>
> * gdb_chr_event() case CHR_EVENT_OPENED
>
> Can this happen in RUN_STATE_GUEST_PANICKED? Jan?
>
> What about RUN_STATE_INTERNAL_ERROR and RUN_STATE_SHUTDOWN?
Nevermind this one. Like for qmp_stop() below, the actual state
transition is protected by runstate_is_running().
> * gdb_sigterm_handler()
>
> No, because vm_stop() is protected by runstate_is_running() here.
>
> Aside: despite its name, this function handles SIGINT. Ugh.
>
> * process_incoming_migration_co()
>
> No, because we're in RUN_STATE_INMIGRATE here, aren't we? Juan?
>
> * qmp_stop()
>
> No, because vm_stop() calls do_vm_stop() to do the actual state
> transition, which protects it by runstate_is_running().
>
> We can ignore the conditional, it merely punts the vm_stop() to the
> main loop.
[...]
