| Message ID | 20221014021653.1461512-2-Jason@zx2c4.com |
|---|---|
| State | New |
| Headers | show |
| Series | rerandomize RNG seeds on reboot and handle record&replay | expand |
(Cc'ing Markus for a QMP related question.) On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <Jason@zx2c4.com> 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 <Jason@zx2c4.com> > 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. (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
On 10/24/22, Peter Maydell <peter.maydell@linaro.org> wrote: > (Cc'ing Markus for a QMP related question.) > > On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <Jason@zx2c4.com> 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 <Jason@zx2c4.com> > >> 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 >
Peter Maydell <peter.maydell@linaro.org> writes: > (Cc'ing Markus for a QMP related question.) > > On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld <Jason@zx2c4.com> 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 <Jason@zx2c4.com> > >> 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 ? We do! Commonly like # @snapshot-load: A snapshot is being loaded by the record & replay # subsystem (since 7.2) >> ## >> # @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. > > (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.) Deleting enumeration values is a compatibility issue only if the value is usable in QMP input. "Purely internal" means it cannot occur in QMP output, and any attempt to use it in input fails. Aside: feels a bit fragile. Having an enumeration type where some values are like this is mildly ugly, because introspection still shows the value. If we care about fragile or mildly ugly, we can mark such values with a special feature flag, and have the QAPI generator keep them internal (input, output, introspection). Does this answer your question?
On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > 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.) > > Deleting enumeration values is a compatibility issue only if the value > is usable in QMP input. > > "Purely internal" means it cannot occur in QMP output, and any attempt > to use it in input fails. Aside: feels a bit fragile. In this case there are as far as I can see no QMP input commands which use the enum at all -- it's only used in events, which are always output, I think. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> > 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.) >> >> Deleting enumeration values is a compatibility issue only if the value >> is usable in QMP input. >> >> "Purely internal" means it cannot occur in QMP output, and any attempt >> to use it in input fails. Aside: feels a bit fragile. > > In this case there are as far as I can see no QMP input commands > which use the enum at all -- it's only used in events, which are > always output, I think. They are. Ascertaining "not used in QMP input" is pretty easy, and "not used in CLI" isn't hard. My point is that uses could creep in later. This is what makes "purely internal" fragile.
On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote: > >> > >> Peter Maydell <peter.maydell@linaro.org> writes: > >> > 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.) > >> > >> Deleting enumeration values is a compatibility issue only if the value > >> is usable in QMP input. > >> > >> "Purely internal" means it cannot occur in QMP output, and any attempt > >> to use it in input fails. Aside: feels a bit fragile. > > > > In this case there are as far as I can see no QMP input commands > > which use the enum at all -- it's only used in events, which are > > always output, I think. > > They are. > > Ascertaining "not used in QMP input" is pretty easy, and "not used in > CLI" isn't hard. My point is that uses could creep in later. This is > what makes "purely internal" fragile. True. But otoh if there's a meaningful use of the enum constant in input in future we'll want to keep it around anyway. I guess what I'm asking is: do you specifically want this patch done some other way, or to require that "mark some values as internal-only" feature in the QAPI generator, or are you OK with it as-is? QMP/QAPI is your area, so your call... -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote: >> >> >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > 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.) >> >> >> >> Deleting enumeration values is a compatibility issue only if the value >> >> is usable in QMP input. >> >> >> >> "Purely internal" means it cannot occur in QMP output, and any attempt >> >> to use it in input fails. Aside: feels a bit fragile. >> > >> > In this case there are as far as I can see no QMP input commands >> > which use the enum at all -- it's only used in events, which are >> > always output, I think. >> >> They are. >> >> Ascertaining "not used in QMP input" is pretty easy, and "not used in >> CLI" isn't hard. My point is that uses could creep in later. This is >> what makes "purely internal" fragile. > > True. But otoh if there's a meaningful use of the enum constant in > input in future we'll want to keep it around anyway. > > I guess what I'm asking is: do you specifically want this patch > done some other way, or to require that "mark some values as > internal-only" feature in the QAPI generator, or are you OK with > it as-is? QMP/QAPI is your area, so your call... QAPI was designed to specify QMP. We pretty soon discovered that QAPI types can be useful elsewhere, and added some to the schema without marking them. Introspection doesn't show these. Generated documentation does. Known shortcoming of the doc generator. This case differs in that we're adding an internal-only member to a type that is used by QMP. Both introspection and documentation will show it. Interface introspection and (especially!) documentation showing stuff that doesn't exist in the interface is certainly less than ideal. However, I don't want to hold this patch hostage to getting QAPI shortcomings addressed. Instead, I want QMP documentation to make clear that this value cannot actually occur. Fair?
On Mon, Oct 24, 2022 at 7:40 PM Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Mon, 24 Oct 2022 at 14:20, Markus Armbruster <armbru@redhat.com> wrote: > >> > >> Peter Maydell <peter.maydell@linaro.org> writes: > >> > >> > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster <armbru@redhat.com> wrote: > >> >> > >> >> Peter Maydell <peter.maydell@linaro.org> writes: > >> >> > 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.) > >> >> > >> >> Deleting enumeration values is a compatibility issue only if the value > >> >> is usable in QMP input. > >> >> > >> >> "Purely internal" means it cannot occur in QMP output, and any attempt > >> >> to use it in input fails. Aside: feels a bit fragile. > >> > > >> > In this case there are as far as I can see no QMP input commands > >> > which use the enum at all -- it's only used in events, which are > >> > always output, I think. > >> > >> They are. > >> > >> Ascertaining "not used in QMP input" is pretty easy, and "not used in > >> CLI" isn't hard. My point is that uses could creep in later. This is > >> what makes "purely internal" fragile. > > > > True. But otoh if there's a meaningful use of the enum constant in > > input in future we'll want to keep it around anyway. > > > > I guess what I'm asking is: do you specifically want this patch > > done some other way, or to require that "mark some values as > > internal-only" feature in the QAPI generator, or are you OK with > > it as-is? QMP/QAPI is your area, so your call... > > QAPI was designed to specify QMP. We pretty soon discovered that QAPI > types can be useful elsewhere, and added some to the schema without > marking them. Introspection doesn't show these. Generated > documentation does. Known shortcoming of the doc generator. > > This case differs in that we're adding an internal-only member to a type > that is used by QMP. Both introspection and documentation will show it. > > Interface introspection and (especially!) documentation showing stuff > that doesn't exist in the interface is certainly less than ideal. > However, I don't want to hold this patch hostage to getting QAPI > shortcomings addressed. > > Instead, I want QMP documentation to make clear that this value cannot > actually occur. > > Fair? Made mention of it in v4, just posted. https://lore.kernel.org/all/20221025004327.568476-2-Jason@zx2c4.com/#Z31qapi:run-state.json
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index bc3ecdb619..69cadb1c37 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -1349,12 +1349,12 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data) aspeed_soc_num_cpus(amc->soc_name); } -static void fby35_reset(MachineState *state) +static void fby35_reset(MachineState *state, ShutdownCause reason) { AspeedMachineState *bmc = ASPEED_MACHINE(state); AspeedGPIOState *gpio = &bmc->soc.gpio; - qemu_devices_reset(); + qemu_devices_reset(reason); /* Board ID: 7 (Class-1, 4 slots) */ object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal); diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index 394192b9b2..284c09c91d 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -1239,7 +1239,7 @@ static void mps2_set_remap(Object *obj, const char *value, Error **errp) } } -static void mps2_machine_reset(MachineState *machine) +static void mps2_machine_reset(MachineState *machine, ShutdownCause reason) { MPS2TZMachineState *mms = MPS2TZ_MACHINE(machine); @@ -1249,7 +1249,7 @@ static void mps2_machine_reset(MachineState *machine) * reset see the correct mapping. */ remap_memory(mms, mms->remap); - qemu_devices_reset(); + qemu_devices_reset(reason); } static void mps2tz_class_init(ObjectClass *oc, void *data) diff --git a/hw/core/reset.c b/hw/core/reset.c index 36be82c491..bcf323d6dd 100644 --- a/hw/core/reset.c +++ b/hw/core/reset.c @@ -33,6 +33,7 @@ typedef struct QEMUResetEntry { QTAILQ_ENTRY(QEMUResetEntry) entry; QEMUResetHandler *func; void *opaque; + bool skip_on_snapshot_load; } QEMUResetEntry; static QTAILQ_HEAD(, QEMUResetEntry) reset_handlers = @@ -47,6 +48,16 @@ void qemu_register_reset(QEMUResetHandler *func, void *opaque) QTAILQ_INSERT_TAIL(&reset_handlers, re, entry); } +void qemu_register_reset_nosnapshotload(QEMUResetHandler *func, void *opaque) +{ + QEMUResetEntry *re = g_new0(QEMUResetEntry, 1); + + re->func = func; + re->opaque = opaque; + re->skip_on_snapshot_load = true; + QTAILQ_INSERT_TAIL(&reset_handlers, re, entry); +} + void qemu_unregister_reset(QEMUResetHandler *func, void *opaque) { QEMUResetEntry *re; @@ -60,12 +71,14 @@ void qemu_unregister_reset(QEMUResetHandler *func, void *opaque) } } -void qemu_devices_reset(void) +void qemu_devices_reset(ShutdownCause reason) { QEMUResetEntry *re, *nre; /* reset all devices */ QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) { + if (reason == SHUTDOWN_CAUSE_SNAPSHOT_LOAD && re->skip_on_snapshot_load) + continue; re->func(re->opaque); } } diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c index e53d5f0fa7..19ea7c2c66 100644 --- a/hw/hppa/machine.c +++ b/hw/hppa/machine.c @@ -411,12 +411,12 @@ static void machine_hppa_init(MachineState *machine) cpu[0]->env.gr[19] = FW_CFG_IO_BASE; } -static void hppa_machine_reset(MachineState *ms) +static void hppa_machine_reset(MachineState *ms, ShutdownCause reason) { unsigned int smp_cpus = ms->smp.cpus; int i; - qemu_devices_reset(); + qemu_devices_reset(reason); /* Start all CPUs at the firmware entry point. * Monarch CPU will initialize firmware, secondary CPUs diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c index 7fe8cce03e..860cfa00f5 100644 --- a/hw/i386/microvm.c +++ b/hw/i386/microvm.c @@ -467,7 +467,7 @@ static void microvm_machine_state_init(MachineState *machine) microvm_devices_init(mms); } -static void microvm_machine_reset(MachineState *machine) +static void microvm_machine_reset(MachineState *machine, ShutdownCause reason) { MicrovmMachineState *mms = MICROVM_MACHINE(machine); CPUState *cs; @@ -480,7 +480,7 @@ static void microvm_machine_reset(MachineState *machine) mms->kernel_cmdline_fixed = true; } - qemu_devices_reset(); + qemu_devices_reset(reason); CPU_FOREACH(cs) { cpu = X86_CPU(cs); diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 566accf7e6..66a0245a65 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1846,12 +1846,12 @@ static void pc_machine_initfn(Object *obj) cxl_machine_init(obj, &pcms->cxl_devices_state); } -static void pc_machine_reset(MachineState *machine) +static void pc_machine_reset(MachineState *machine, ShutdownCause reason) { CPUState *cs; X86CPU *cpu; - qemu_devices_reset(); + qemu_devices_reset(reason); /* Reset APIC after devices have been reset to cancel * any changes that qemu_devices_reset() might have done. @@ -1868,7 +1868,7 @@ static void pc_machine_reset(MachineState *machine) static void pc_machine_wakeup(MachineState *machine) { cpu_synchronize_all_states(); - pc_machine_reset(machine); + pc_machine_reset(machine, SHUTDOWN_CAUSE_NONE); cpu_synchronize_all_post_reset(); } diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c index 61f4263953..919d5008ea 100644 --- a/hw/ppc/pegasos2.c +++ b/hw/ppc/pegasos2.c @@ -248,14 +248,14 @@ static void pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus, pegasos2_mv_reg_write(pm, pcicfg + 4, len, val); } -static void pegasos2_machine_reset(MachineState *machine) +static void pegasos2_machine_reset(MachineState *machine, ShutdownCause reason) { Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine); void *fdt; uint64_t d[2]; int sz; - qemu_devices_reset(); + qemu_devices_reset(reason); if (!pm->vof) { return; /* Firmware should set up machine so nothing to do */ } diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 78e00afb9b..358a64b397 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -643,13 +643,13 @@ static void pnv_powerdown_notify(Notifier *n, void *opaque) } } -static void pnv_reset(MachineState *machine) +static void pnv_reset(MachineState *machine, ShutdownCause reason) { PnvMachineState *pnv = PNV_MACHINE(machine); IPMIBmc *bmc; void *fdt; - qemu_devices_reset(); + qemu_devices_reset(reason); /* * The machine should provide by default an internal BMC simulator. diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 8bbaf4f8a4..6377e4cdb5 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1623,7 +1623,7 @@ void spapr_check_mmu_mode(bool guest_radix) } } -static void spapr_machine_reset(MachineState *machine) +static void spapr_machine_reset(MachineState *machine, ShutdownCause reason) { SpaprMachineState *spapr = SPAPR_MACHINE(machine); PowerPCCPU *first_ppc_cpu; @@ -1649,7 +1649,7 @@ static void spapr_machine_reset(MachineState *machine) spapr_setup_hpt(spapr); } - qemu_devices_reset(); + qemu_devices_reset(reason); spapr_ovec_cleanup(spapr->ov5_cas); spapr->ov5_cas = spapr_ovec_new(); diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 03855c7231..8017acb1d5 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -405,7 +405,7 @@ static void s390_pv_prepare_reset(S390CcwMachineState *ms) s390_pv_prep_reset(); } -static void s390_machine_reset(MachineState *machine) +static void s390_machine_reset(MachineState *machine, ShutdownCause reason) { S390CcwMachineState *ms = S390_CCW_MACHINE(machine); enum s390_reset reset_type; @@ -427,7 +427,7 @@ static void s390_machine_reset(MachineState *machine) s390_machine_unprotect(ms); } - qemu_devices_reset(); + qemu_devices_reset(reason); s390_crypto_reset(); /* configure and start the ipl CPU only */ diff --git a/include/hw/boards.h b/include/hw/boards.h index 311ed17e18..90f1dd3aeb 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -231,7 +231,7 @@ struct MachineClass { const char *deprecation_reason; void (*init)(MachineState *state); - void (*reset)(MachineState *state); + void (*reset)(MachineState *state, ShutdownCause reason); void (*wakeup)(MachineState *state); int (*kvm_type)(MachineState *machine, const char *arg); diff --git a/include/sysemu/reset.h b/include/sysemu/reset.h index 0b0d6d7598..609e4d50c2 100644 --- a/include/sysemu/reset.h +++ b/include/sysemu/reset.h @@ -1,10 +1,13 @@ #ifndef QEMU_SYSEMU_RESET_H #define QEMU_SYSEMU_RESET_H +#include "qapi/qapi-events-run-state.h" + typedef void QEMUResetHandler(void *opaque); void qemu_register_reset(QEMUResetHandler *func, void *opaque); +void qemu_register_reset_nosnapshotload(QEMUResetHandler *func, void *opaque); void qemu_unregister_reset(QEMUResetHandler *func, void *opaque); -void qemu_devices_reset(void); +void qemu_devices_reset(ShutdownCause reason); #endif 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'] } ## # @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);
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 <Jason@zx2c4.com> --- hw/arm/aspeed.c | 4 ++-- hw/arm/mps2-tz.c | 4 ++-- hw/core/reset.c | 15 ++++++++++++++- hw/hppa/machine.c | 4 ++-- hw/i386/microvm.c | 4 ++-- hw/i386/pc.c | 6 +++--- hw/ppc/pegasos2.c | 4 ++-- hw/ppc/pnv.c | 4 ++-- hw/ppc/spapr.c | 4 ++-- hw/s390x/s390-virtio-ccw.c | 4 ++-- include/hw/boards.h | 2 +- include/sysemu/reset.h | 5 ++++- migration/savevm.c | 2 +- qapi/run-state.json | 4 +++- softmmu/runstate.c | 4 ++-- 15 files changed, 44 insertions(+), 26 deletions(-)