On 10/24/22, Peter Maydell <[email protected]> wrote:
> (Cc'ing Markus for a QMP related question.)
>
> On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <[email protected]> wrote:
>>
>> Snapshot loading only expects to call deterministic handlers, not
>> non-deterministic ones. So introduce a way of registering handlers that
>> won't be called when reseting for snapshots.
>>
>> Signed-off-by: Jason A. Donenfeld <[email protected]>
>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 48e85c052c..a0cdb714f7 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char
>> *vmstate,
>>          goto err_drain;
>>      }
>>
>> -    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
>> +    qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD);
>>      mis->from_src_file = f;
>>
>>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
>> diff --git a/qapi/run-state.json b/qapi/run-state.json
>> index 9273ea6516..74ed0ac93c 100644
>> --- a/qapi/run-state.json
>> +++ b/qapi/run-state.json
>> @@ -86,12 +86,14 @@
>>  #                   ignores --no-reboot. This is useful for sanitizing
>>  #                   hypercalls on s390 that are used during
>> kexec/kdump/boot
>>  #
>> +# @snapshot-load: A snapshot is being loaded by the record & replay
>> subsystem.
>> +#
>>  ##
>>  { 'enum': 'ShutdownCause',
>>    # Beware, shutdown_caused_by_guest() depends on enumeration order
>>    'data': [ 'none', 'host-error', 'host-qmp-quit',
>> 'host-qmp-system-reset',
>>              'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset',
>> -            'guest-panic', 'subsystem-reset'] }
>> +            'guest-panic', 'subsystem-reset', 'snapshot-load'] }
>
> Markus: do we need to mark new enum values with some kind of "since n.n"
> version info ?
>
>>  ##
>>  # @StatusInfo:
>> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
>> index 1e68680b9d..03e1ee3a8a 100644
>> --- a/softmmu/runstate.c
>> +++ b/softmmu/runstate.c
>> @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason)
>>      cpu_synchronize_all_states();
>>
>>      if (mc && mc->reset) {
>> -        mc->reset(current_machine);
>> +        mc->reset(current_machine, reason);
>>      } else {
>> -        qemu_devices_reset();
>> +        qemu_devices_reset(reason);
>>      }
>>      if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
>>          qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
>
> This change means that resets on snapshot-load, which previously
> did not result in sending a QMP event, will now start to do so
> with this new ShutdownCause type. I don't think we want that
> change in behaviour.

Good point. I'll exclude that case and send a v+1.

Jason

>
> (Also, as per the 'Beware' comment in run-state.json, we will
> claim this to be a 'caused by guest' reset, which doesn't seem
> right. If we suppress the sending the event that is moot, though.)
>
> Markus: if we add a new value to the ShutdownCause enumeration,
> how annoying is it if we decide we don't want it later? I guess
> we can just leave it in the enum unused... (In this case we're
> using it for purely internal purposes and it won't ever actually
> wind up in any QMP events.)
>
> thanks
> -- PMM
>


-- 
Jason A. Donenfeld
Deep Space Explorer
fr: +33 6 51 90 82 66
us: +1 513 476 1200
www.jasondonenfeld.com
www.zx2c4.com
zx2c4.com/keys/AB9942E6D4A4CFC3412620A749FC7012A5DE03AE.asc

Reply via email to