Reset system before loadvm
PATCH 1/1 Reset system before loadvm
From: Jan Kiszka <[email protected]>
In case we load the vmstate during incoming migration, we start from a clean, default machine state as we went through system reset before. But if we load from a snapshot, the machine can be in any state. That can cause troubles if loading an older image which does not carry all state information the executing QEMU requires. Almost no device takes care of this scenario.
However, fixing this is trivial. We just need to issue a system reset during loadvm as well.
--- savevm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
savevm.c +1 -0
@@ -2074,6 +2074,7 @@ int load_vmstate(const char *name)return -EINVAL ; } +qemu_system_reset (); ret =qemu_loadvm_state (f);qemu_fclose (f);
Signature · 1 line
1.7.1
Quoted 22 lines · Jan Kiszka
From: Jan Kiszka<[email protected]>
In case we load the vmstate during incoming migration, we start from a clean, default machine state as we went through system reset before. But if we load from a snapshot, the machine can be in any state. That can cause troubles if loading an older image which does not carry all state information the executing QEMU requires. Almost no device takes care of this scenario.
However, fixing this is trivial. We just need to issue a system reset during loadvm as well.
Signed-off-by Jan Kiszka<[email protected]>--- savevm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)savevm.c +0
-0
savevm.c int load_vmstate(const char *name) · line 2080
Quoted 7 lines
savevm.c +1
-0@@ -2074,6 +2074,7 @@ int load_vmstate(const char *name)return -EINVAL ; } +qemu_system_reset (); ret =qemu_loadvm_state (f);qemu_fclose (f);
Should we suppress the reset event sent out on the monitor? After all, it's the result of an internal implementation choice, not something the user or the guest did.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 26 lines · Avi Kivity
Quoted 25 lines · Jan Kiszka
From: Jan Kiszka<[email protected]>
In case we load the vmstate during incoming migration, we start from a clean, default machine state as we went through system reset before. But if we load from a snapshot, the machine can be in any state. That can cause troubles if loading an older image which does not carry all state information the executing QEMU requires. Almost no device takes care of this scenario.
However, fixing this is trivial. We just need to issue a system reset during loadvm as well.
Signed-off-by Jan Kiszka<[email protected]>--- savevm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)savevm.c +0
-0@@ -2074,6 +2074,7 @@ int load_vmstate(const char *name)return -EINVAL ; }
Quoted 8 lines
savevm.c int load_vmstate(const char *name) · line 2080
Quoted 4 lines
savevm.c +1
-0+qemu_system_reset (); ret =qemu_loadvm_state (f);qemu_fclose (f);Should we suppress the reset event sent out on the monitor? After all, it's the result of an internal implementation choice, not something the user or the guest did.
We already issue this pattern during -loadvm or -incoming - or is the monitor not yet connected at this point?
Jan
Quoted 18 lines · Jan Kiszka
Quoted 17 lines · Avi Kivity
Quoted 16 lines · Jan Kiszka
From: Jan Kiszka<[email protected]>
In case we load the vmstate during incoming migration, we start from a clean, default machine state as we went through system reset before. But if we load from a snapshot, the machine can be in any state. That can cause troubles if loading an older image which does not carry all state information the executing QEMU requires. Almost no device takes care of this scenario.
However, fixing this is trivial. We just need to issue a system reset during loadvm as well.
+ qemu_system_reset(); ret = qemu_loadvm_state(f);
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
qemu_fclose(f);Should we suppress the reset event sent out on the monitor? After all, it's the result of an internal implementation choice, not something the user or the guest did.
We already issue this pattern during -loadvm or -incoming - or is the monitor not yet connected at this point?
I believe it is not. But regardless, we shouldn't add more incorrect behaviour.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 28 lines · Avi Kivity
Quoted 27 lines · Jan Kiszka
Quoted 4 lines · Avi Kivity
Quoted 3 lines · Jan Kiszka
From: Jan Kiszka<[email protected]>
In case we load the vmstate during incoming migration, we start
from a
Quoted 1 line
Quoted 1 line
clean, default machine state as we went through system reset
before. But
Quoted 2 lines
Quoted 2 lines
if we load from a snapshot, the machine can be in any state. That can cause troubles if loading an older image which does not carry all
state
Quoted 1 line
Quoted 1 line
information the executing QEMU requires. Almost no device takes
care of
Quoted 6 lines
Quoted 4 lines
this scenario.
However, fixing this is trivial. We just need to issue a system reset during loadvm as well.
Quoted 6 lines
Quoted 4 lines
+ qemu_system_reset(); ret = qemu_loadvm_state(f); qemu_fclose(f);Should we suppress the reset event sent out on the monitor? After
all,
Quoted 1 line
it's the result of an internal implementation choice, not something
Quoted 8 lines
Quoted 5 lines
the
Quoted 1 line
user or the guest did.
We already issue this pattern during -loadvm or -incoming - or is the monitor not yet connected at this point?
I believe it is not. But regardless, we shouldn't add more incorrect behaviour.
It depends on how the reset event is defined in QMP. As I see it, there is nothing stated about reset reasons or sources. So emitting information about the actually happening reset can't be incorrect. Just like emitting the information about the VM stop/start around loadvm.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Quoted 8 lines · Jan Kiszka
Quoted 3 lines
I believe it is not. But regardless, we shouldn't add more incorrect behaviour.
It depends on how the reset event is defined in QMP. As I see it, there is nothing stated about reset reasons or sources. So emitting information about the actually happening reset can't be incorrect. Just like emitting the information about the VM stop/start around loadvm.
I don't think so. Theoretically we could stop the vm, save a bit of state, reset it, and load the state back. Did a reset occur? Not from the user's point of view.
If a reset event is interesting (personally I don't think it is, so much, perhaps just for logging purposes), we should restrict it to user visible events (so it means either the user pressed the reset button or the guest reset itself).
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 10 lines · On Tue, 14 Jun 2011 14:14:58 +0300 Avi Kivity <[email protected]> wrote:
Quoted 8 lines · Jan Kiszka
Quoted 3 lines
I believe it is not. But regardless, we shouldn't add more incorrect behaviour.
It depends on how the reset event is defined in QMP. As I see it, there is nothing stated about reset reasons or sources. So emitting information about the actually happening reset can't be incorrect. Just like emitting the information about the VM stop/start around loadvm.
Quoted 8 lines
I don't think so. Theoretically we could stop the vm, save a bit of state, reset it, and load the state back. Did a reset occur? Not from the user's point of view.
If a reset event is interesting (personally I don't think it is, so much, perhaps just for logging purposes), we should restrict it to user visible events (so it means either the user pressed the reset button or the guest reset itself).
Yes, that's right. If the documentation (or even the code) is not precise enough, we have to fix it.
savevm.c int load_vmstate(const char *name) · line 2078
Quoted 6 lines · Jan Kiszka
savevm.c +1
-0@@ -2074,6 +2074,7 @@ int load_vmstate(const char *name) return -EINVAL; } + qemu_system_reset(); ret = qemu_loadvm_state(f);
This means that if we're doing a load because the user passed -loadvm on the command line we'll end up doing qemu_system_reset() twice (once in vl.c and then again here). Does that matter?
-- PMM
Quoted 5 lines · Peter Maydell
Quoted 4 lines · Jan Kiszka
savevm.c +0
-0@@ -2074,6 +2074,7 @@ int load_vmstate(const char *name)return -EINVAL ; }
Quoted 7 lines
savevm.c int load_vmstate(const char *name) · line 2078
Quoted 2 lines
savevm.c +1
-0+qemu_system_reset (); ret =qemu_loadvm_state (f);This means that if we're doing a load because the user passed -loadvm on the command line we'll end up doing
qemu_system_reset()twice (once invl.cand then again here). Does that matter?
I don't think it's worth optimizing. E.g. we already reset every PCI device twice during normal reset.
Jan
PATCH 1/2 Allow silent system resets
In reply to Jan KiszkaThis allows qemu_system_reset to be issued silently for internal purposes, ie. without sending out a monitor event. Convert the system reset after startup to the silent mode.
--- sysemu.h | 5 ++++- vl.c | 10 ++++++---- xen-all.c | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-)
sysemu.h +4 -1
@@ -34,6 +34,9 @@ void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);#define VMSTOP_LOADVM 7#define VMSTOP_MIGRATE 8 +#define VMRESET_SILENT false +#define VMRESET_REPORT true +void vm_start (void );void vm_stop (int reason);
@@ -50,7 +53,7 @@ int qemu_powerdown_requested(void);void qemu_system_killed (int signal,pid_t pid);void qemu_kill_report (void );extern qemu_irq qemu_system_powerdown; -void qemu_system_reset (void ); +void qemu_system_reset (bool report);void qemu_add_exit_notifier (Notifier *notify);void qemu_remove_exit_notifier (Notifier *notify);
vl.c +6 -4
@@ -1249,7 +1249,7 @@ void qemu_unregister_reset(QEMUResetHandler *func, void *opaque) } } -void qemu_system_reset (void ) +void qemu_system_reset (bool report) {QEMUResetEntry *re, *nre;
@@ -1257,7 +1257,9 @@ void qemu_system_reset(void) QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) { re->func (re->opaque); } -monitor_protocol_event (QEVENT_RESET ,NULL ); +if (report) { +monitor_protocol_event (QEVENT_RESET ,NULL ); + }cpu_synchronize_all_post_reset (); }
@@ -1399,7 +1401,7 @@ static void main_loop(void)if (qemu_reset_requested ()) {pause_all_vcpus ();cpu_synchronize_all_states (); -qemu_system_reset (); +qemu_system_reset (VMRESET_REPORT );resume_all_vcpus (); }if (qemu_powerdown_requested ()) {
@@ -3272,7 +3274,7 @@ int main(int argc, char **argv, char **envp)qemu_register_reset (qbus_reset_all_fn,sysbus_get_default ());qemu_run_machine_init_done_notifiers (); -qemu_system_reset (); +qemu_system_reset (VMRESET_SILENT );if (loadvm) {if (load_vmstate (loadvm) <0 ) { autostart =0 ;
xen-all.c +1 -1
@@ -452,7 +452,7 @@ static void cpu_handle_ioreq(void *opaque)destroy_hvm_domain (); }if (qemu_reset_requested_get ()) { -qemu_system_reset (); +qemu_system_reset (VMRESET_REPORT ); } }
Signature · 1 line
1.7.1
Re: [Qemu-devel] [PATCH 1/2] Allow silent system resets
Quoted 5 lines · On Tue, 14 Jun 2011 18:29:43 +0200 Jan Kiszka <[email protected]> wrote:
This allows
qemu_system_resetto be issued silently for internal purposes, ie. without sending out a monitor event. Convert the system reset after startup to the silent mode.Signed-off-by Jan Kiszka <[email protected]>
Only this patch is monitor related, but I've applied the two in the monitor branch.
Thanks.
Quoted 84 lines
--- sysemu.h | 5 ++++- vl.c | 10 ++++++---- xen-all.c | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-)sysemu.h +4
-1@@ -34,6 +34,9 @@ void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);#define VMSTOP_LOADVM 7#define VMSTOP_MIGRATE 8 +#define VMRESET_SILENT false +#define VMRESET_REPORT true +void vm_start (void );void vm_stop (int reason);@@ -50,7 +53,7 @@ int qemu_powerdown_requested(void);void qemu_system_killed (int signal,pid_t pid);void qemu_kill_report (void );extern qemu_irq qemu_system_powerdown; -void qemu_system_reset (void ); +void qemu_system_reset (bool report);void qemu_add_exit_notifier (Notifier *notify);void qemu_remove_exit_notifier (Notifier *notify);vl.c +6
-4@@ -1249,7 +1249,7 @@ void qemu_unregister_reset(QEMUResetHandler *func, void *opaque) } } -void qemu_system_reset (void ) +void qemu_system_reset (bool report) {QEMUResetEntry *re, *nre;@@ -1257,7 +1257,9 @@ void qemu_system_reset(void) QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) { re->func (re->opaque); } -monitor_protocol_event (QEVENT_RESET ,NULL ); +if (report) { +monitor_protocol_event (QEVENT_RESET ,NULL ); + }cpu_synchronize_all_post_reset (); }@@ -1399,7 +1401,7 @@ static void main_loop(void)if (qemu_reset_requested ()) {pause_all_vcpus ();cpu_synchronize_all_states (); -qemu_system_reset (); +qemu_system_reset (VMRESET_REPORT );resume_all_vcpus (); }if (qemu_powerdown_requested ()) {@@ -3272,7 +3274,7 @@ int main(int argc, char **argv, char **envp)qemu_register_reset (qbus_reset_all_fn,sysbus_get_default ());qemu_run_machine_init_done_notifiers (); -qemu_system_reset (); +qemu_system_reset (VMRESET_SILENT );if (loadvm) {if (load_vmstate (loadvm) <0 ) { autostart =0 ;xen-all.c +1
-1@@ -452,7 +452,7 @@ static void cpu_handle_ioreq(void *opaque)destroy_hvm_domain (); }if (qemu_reset_requested_get ()) { -qemu_system_reset (); +qemu_system_reset (VMRESET_REPORT ); } }
PATCH v2 2/2 Reset system before loadvm
In reply to Jan KiszkaIn case we load the vmstate during incoming migration, we start from a clean, default machine state as we went through system reset before. But if we load from a snapshot, the machine can be in any state. That can cause troubles if loading an older image which does not carry all state information the executing QEMU requires. Hardly any device takes care of this scenario.
However, fixing this is trivial. We just need to issue a system reset during loadvm as well.
--- savevm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
savevm.c +1 -0
@@ -2073,6 +2073,7 @@ int load_vmstate(const char *name)return -EINVAL ; } +qemu_system_reset (VMRESET_SILENT ); ret =qemu_loadvm_state (f);qemu_fclose (f);
Signature · 1 line
1.7.1