| Message ID | df20999140db716e8b1ccd57a7cbcb0fa343a78c.1362051582.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 02/28/2013 05:13 AM, Hu Tao wrote: > This patch enables preservation of cpu runstate during save/load vm. > So when a vm is restored from snapshot, the cpu runstate is restored, > too. What happens if a management app wants to override the runstate when restoring the domain? I can think of several useful scenarios: 1. management app pauses the guest, then saves domain state and other things (management state, or disk clones), then resumes the guest. Later, the management wants to revert to the saved state, but have the guest running right away. I guess here, knowing that the guest was saved in a paused state doesn't hurt, since the management app can resume it right away. 2. management app saves domain state of a live guest, then copies that state elsewhere. In its new location, the management app wants to investigate the state for forensic analysis - so even though the guest remembers that it was running, management wants to start it paused. Here, it is important that there must not be a window of time where the guest can run, otherwise, the results are not reproducible.
On Thu, Feb 28, 2013 at 02:12:37PM -0700, Eric Blake wrote: > On 02/28/2013 05:13 AM, Hu Tao wrote: > > This patch enables preservation of cpu runstate during save/load vm. > > So when a vm is restored from snapshot, the cpu runstate is restored, > > too. > > What happens if a management app wants to override the runstate when > restoring the domain? I can think of several useful scenarios: > > 1. management app pauses the guest, then saves domain state and other > things (management state, or disk clones), then resumes the guest. > Later, the management wants to revert to the saved state, but have the > guest running right away. I guess here, knowing that the guest was > saved in a paused state doesn't hurt, since the management app can > resume it right away. > > 2. management app saves domain state of a live guest, then copies that > state elsewhere. In its new location, the management app wants to > investigate the state for forensic analysis - so even though the guest > remembers that it was running, management wants to start it paused. > Here, it is important that there must not be a window of time where the > guest can run, otherwise, the results are not reproducible. -S takes precedence in the case. But for in-migration, runstate is loaded from src.
On 03/01/2013 12:36 AM, Hu Tao wrote: > On Thu, Feb 28, 2013 at 02:12:37PM -0700, Eric Blake wrote: >> On 02/28/2013 05:13 AM, Hu Tao wrote: >>> This patch enables preservation of cpu runstate during save/load vm. >>> So when a vm is restored from snapshot, the cpu runstate is restored, >>> too. >> >> What happens if a management app wants to override the runstate when >> restoring the domain? I can think of several useful scenarios: >> >> 1. management app pauses the guest, then saves domain state and other >> things (management state, or disk clones), then resumes the guest. >> Later, the management wants to revert to the saved state, but have the >> guest running right away. I guess here, knowing that the guest was >> saved in a paused state doesn't hurt, since the management app can >> resume it right away. >> >> 2. management app saves domain state of a live guest, then copies that >> state elsewhere. In its new location, the management app wants to >> investigate the state for forensic analysis - so even though the guest >> remembers that it was running, management wants to start it paused. >> Here, it is important that there must not be a window of time where the >> guest can run, otherwise, the results are not reproducible. > > -S takes precedence in the case. But for in-migration, runstate is > loaded from src. Given your answer, I think we're okay from the libvirt perspective. My biggest worry about a window where the guest runs unchecked is not a problem, given that libvirt always uses -S on incoming migration. In turn, libvirt has its own mechanisms for tracking whether the outgoing migration was started from a running state, along with API overrides to let a user override whether libvirt will resume the guest on the incoming migration side.
Il 28/02/2013 13:13, Hu Tao ha scritto: > This patch enables preservation of cpu runstate during save/load vm. > So when a vm is restored from snapshot, the cpu runstate is restored, > too. I don't think this feature is worth breaking backwards migration compatibility. It is usually handled at a higher-level (management, like libvirt). Please make this a separate patch. Paolo > See following example: > > # save two vms: one is running, the other is paused > (qemu) info status > VM status: running > (qemu) savevm running > (qemu) stop > (qemu) info status > VM status: paused > (qemu) savevm paused > > # restore the one running > (qemu) info status > VM status: paused > (qemu) loadvm running > (qemu) info status > VM status: running > > # restore the one paused > (qemu) loadvm paused > (qemu) info status > VM status: paused > (qemu) cont > (qemu)info status > VM status: running > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > include/sysemu/sysemu.h | 2 ++ > migration.c | 6 +----- > monitor.c | 5 ++--- > savevm.c | 1 + > vl.c | 34 ++++++++++++++++++++++++++++++++++ > 5 files changed, 40 insertions(+), 8 deletions(-) > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index b19ec95..f121213 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -19,6 +19,8 @@ extern uint8_t qemu_uuid[]; > int qemu_uuid_parse(const char *str, uint8_t *uuid); > #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx" > > +void save_run_state(void); > +void load_run_state(void); > bool runstate_check(RunState state); > void runstate_set(RunState new_state); > int runstate_is_running(void); > diff --git a/migration.c b/migration.c > index 11725ae..c29830e 100644 > --- a/migration.c > +++ b/migration.c > @@ -107,11 +107,7 @@ static void process_incoming_migration_co(void *opaque) > /* Make sure all file formats flush their mutable metadata */ > bdrv_invalidate_cache_all(); > > - if (autostart) { > - vm_start(); > - } else { > - runstate_set(RUN_STATE_PAUSED); > - } > + load_run_state(); > } > > void process_incoming_migration(QEMUFile *f) > diff --git a/monitor.c b/monitor.c > index 32a6e74..bf974b4 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2059,13 +2059,12 @@ void qmp_closefd(const char *fdname, Error **errp) > > static void do_loadvm(Monitor *mon, const QDict *qdict) > { > - int saved_vm_running = runstate_is_running(); > const char *name = qdict_get_str(qdict, "name"); > > vm_stop(RUN_STATE_RESTORE_VM); > > - if (load_vmstate(name) == 0 && saved_vm_running) { > - vm_start(); > + if (load_vmstate(name) == 0) { > + load_run_state(); > } > } > > diff --git a/savevm.c b/savevm.c > index a8a53ef..aa631eb 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -2143,6 +2143,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) > } > > saved_vm_running = runstate_is_running(); > + save_run_state(); > vm_stop(RUN_STATE_SAVE_VM); > > memset(sn, 0, sizeof(*sn)); > diff --git a/vl.c b/vl.c > index febd2ea..7991f2e 100644 > --- a/vl.c > +++ b/vl.c > @@ -523,6 +523,7 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > /* QEMU state */ > > static RunState current_run_state = RUN_STATE_PRELAUNCH; > +static RunState saved_run_state = RUN_STATE_RUNNING; > > typedef struct { > RunState from; > @@ -546,6 +547,7 @@ static const RunStateTransition runstate_transitions_def[] = { > { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE }, > > { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING }, > + { RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED }, > { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE }, > > { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING }, > @@ -556,6 +558,7 @@ static const RunStateTransition runstate_transitions_def[] = { > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE }, > > { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING }, > + { RUN_STATE_RESTORE_VM, RUN_STATE_PAUSED }, > > { RUN_STATE_RUNNING, RUN_STATE_DEBUG }, > { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR }, > @@ -585,11 +588,39 @@ static const RunStateTransition runstate_transitions_def[] = { > > static bool runstate_valid_transitions[RUN_STATE_MAX][RUN_STATE_MAX]; > > +void save_run_state(void) > +{ > + saved_run_state = current_run_state; > +} > + > +void load_run_state(void) > +{ > + if (saved_run_state == RUN_STATE_RUNNING) { > + vm_start(); > + } else if (!runstate_check(saved_run_state)) { > + runstate_set(saved_run_state); > + } else { > + ; /* leave unchanged */ > + } > +} > + > bool runstate_check(RunState state) > { > return current_run_state == state; > } > > +static void runstate_save(QEMUFile *f, void *opaque) > +{ > + qemu_put_byte(f, saved_run_state); > +} > + > +static int runstate_load(QEMUFile *f, void *opaque, int version_id) > +{ > + saved_run_state = qemu_get_byte(f); > + > + return 0; > +} > + > static void runstate_init(void) > { > const RunStateTransition *p; > @@ -599,6 +630,9 @@ static void runstate_init(void) > for (p = &runstate_transitions_def[0]; p->from != RUN_STATE_MAX; p++) { > runstate_valid_transitions[p->from][p->to] = true; > } > + > + register_savevm(NULL, "runstate", 0, 1, > + runstate_save, runstate_load, NULL); > } > > /* This function will abort() on invalid state transitions */ >
On Mon, Mar 04, 2013 at 10:30:48AM +0100, Paolo Bonzini wrote: > Il 28/02/2013 13:13, Hu Tao ha scritto: > > This patch enables preservation of cpu runstate during save/load vm. > > So when a vm is restored from snapshot, the cpu runstate is restored, > > too. > > I don't think this feature is worth breaking backwards migration > compatibility. It is usually handled at a higher-level (management, > like libvirt). If guest panic happens during migration, runstate will still be running on destination host without this patch. But, it does be a problem to break backwards migration compatibility. > > Please make this a separate patch. Sure. > > Paolo > > > See following example: > > > > # save two vms: one is running, the other is paused > > (qemu) info status > > VM status: running > > (qemu) savevm running > > (qemu) stop > > (qemu) info status > > VM status: paused > > (qemu) savevm paused > > > > # restore the one running > > (qemu) info status > > VM status: paused > > (qemu) loadvm running > > (qemu) info status > > VM status: running > > > > # restore the one paused > > (qemu) loadvm paused > > (qemu) info status > > VM status: paused > > (qemu) cont > > (qemu)info status > > VM status: running > > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > include/sysemu/sysemu.h | 2 ++ > > migration.c | 6 +----- > > monitor.c | 5 ++--- > > savevm.c | 1 + > > vl.c | 34 ++++++++++++++++++++++++++++++++++ > > 5 files changed, 40 insertions(+), 8 deletions(-) > > > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > > index b19ec95..f121213 100644 > > --- a/include/sysemu/sysemu.h > > +++ b/include/sysemu/sysemu.h > > @@ -19,6 +19,8 @@ extern uint8_t qemu_uuid[]; > > int qemu_uuid_parse(const char *str, uint8_t *uuid); > > #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx" > > > > +void save_run_state(void); > > +void load_run_state(void); > > bool runstate_check(RunState state); > > void runstate_set(RunState new_state); > > int runstate_is_running(void); > > diff --git a/migration.c b/migration.c > > index 11725ae..c29830e 100644 > > --- a/migration.c > > +++ b/migration.c > > @@ -107,11 +107,7 @@ static void process_incoming_migration_co(void *opaque) > > /* Make sure all file formats flush their mutable metadata */ > > bdrv_invalidate_cache_all(); > > > > - if (autostart) { > > - vm_start(); > > - } else { > > - runstate_set(RUN_STATE_PAUSED); > > - } > > + load_run_state(); > > } > > > > void process_incoming_migration(QEMUFile *f) > > diff --git a/monitor.c b/monitor.c > > index 32a6e74..bf974b4 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -2059,13 +2059,12 @@ void qmp_closefd(const char *fdname, Error **errp) > > > > static void do_loadvm(Monitor *mon, const QDict *qdict) > > { > > - int saved_vm_running = runstate_is_running(); > > const char *name = qdict_get_str(qdict, "name"); > > > > vm_stop(RUN_STATE_RESTORE_VM); > > > > - if (load_vmstate(name) == 0 && saved_vm_running) { > > - vm_start(); > > + if (load_vmstate(name) == 0) { > > + load_run_state(); > > } > > } > > > > diff --git a/savevm.c b/savevm.c > > index a8a53ef..aa631eb 100644 > > --- a/savevm.c > > +++ b/savevm.c > > @@ -2143,6 +2143,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) > > } > > > > saved_vm_running = runstate_is_running(); > > + save_run_state(); > > vm_stop(RUN_STATE_SAVE_VM); > > > > memset(sn, 0, sizeof(*sn)); > > diff --git a/vl.c b/vl.c > > index febd2ea..7991f2e 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -523,6 +523,7 @@ static int default_driver_check(QemuOpts *opts, void *opaque) > > /* QEMU state */ > > > > static RunState current_run_state = RUN_STATE_PRELAUNCH; > > +static RunState saved_run_state = RUN_STATE_RUNNING; > > > > typedef struct { > > RunState from; > > @@ -546,6 +547,7 @@ static const RunStateTransition runstate_transitions_def[] = { > > { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE }, > > > > { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING }, > > + { RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED }, > > { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE }, > > > > { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING }, > > @@ -556,6 +558,7 @@ static const RunStateTransition runstate_transitions_def[] = { > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE }, > > > > { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING }, > > + { RUN_STATE_RESTORE_VM, RUN_STATE_PAUSED }, > > > > { RUN_STATE_RUNNING, RUN_STATE_DEBUG }, > > { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR }, > > @@ -585,11 +588,39 @@ static const RunStateTransition runstate_transitions_def[] = { > > > > static bool runstate_valid_transitions[RUN_STATE_MAX][RUN_STATE_MAX]; > > > > +void save_run_state(void) > > +{ > > + saved_run_state = current_run_state; > > +} > > + > > +void load_run_state(void) > > +{ > > + if (saved_run_state == RUN_STATE_RUNNING) { > > + vm_start(); > > + } else if (!runstate_check(saved_run_state)) { > > + runstate_set(saved_run_state); > > + } else { > > + ; /* leave unchanged */ > > + } > > +} > > + > > bool runstate_check(RunState state) > > { > > return current_run_state == state; > > } > > > > +static void runstate_save(QEMUFile *f, void *opaque) > > +{ > > + qemu_put_byte(f, saved_run_state); > > +} > > + > > +static int runstate_load(QEMUFile *f, void *opaque, int version_id) > > +{ > > + saved_run_state = qemu_get_byte(f); > > + > > + return 0; > > +} > > + > > static void runstate_init(void) > > { > > const RunStateTransition *p; > > @@ -599,6 +630,9 @@ static void runstate_init(void) > > for (p = &runstate_transitions_def[0]; p->from != RUN_STATE_MAX; p++) { > > runstate_valid_transitions[p->from][p->to] = true; > > } > > + > > + register_savevm(NULL, "runstate", 0, 1, > > + runstate_save, runstate_load, NULL); > > } > > > > /* This function will abort() on invalid state transitions */ > >
Il 05/03/2013 03:33, Hu Tao ha scritto: > On Mon, Mar 04, 2013 at 10:30:48AM +0100, Paolo Bonzini wrote: >> Il 28/02/2013 13:13, Hu Tao ha scritto: >>> This patch enables preservation of cpu runstate during save/load vm. >>> So when a vm is restored from snapshot, the cpu runstate is restored, >>> too. >> >> I don't think this feature is worth breaking backwards migration >> compatibility. It is usually handled at a higher-level (management, >> like libvirt). > > If guest panic happens during migration, runstate will still be running > on destination host without this patch. But, it does be a problem to break > backwards migration compatibility. Yes, but the source management (libvirt for example) will have the occasion to observe the change and cancel the migration or forward the information on the wire. There are already similar reasons why libvirt always starts guests with -S. Paolo
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index b19ec95..f121213 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -19,6 +19,8 @@ extern uint8_t qemu_uuid[]; int qemu_uuid_parse(const char *str, uint8_t *uuid); #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx" +void save_run_state(void); +void load_run_state(void); bool runstate_check(RunState state); void runstate_set(RunState new_state); int runstate_is_running(void); diff --git a/migration.c b/migration.c index 11725ae..c29830e 100644 --- a/migration.c +++ b/migration.c @@ -107,11 +107,7 @@ static void process_incoming_migration_co(void *opaque) /* Make sure all file formats flush their mutable metadata */ bdrv_invalidate_cache_all(); - if (autostart) { - vm_start(); - } else { - runstate_set(RUN_STATE_PAUSED); - } + load_run_state(); } void process_incoming_migration(QEMUFile *f) diff --git a/monitor.c b/monitor.c index 32a6e74..bf974b4 100644 --- a/monitor.c +++ b/monitor.c @@ -2059,13 +2059,12 @@ void qmp_closefd(const char *fdname, Error **errp) static void do_loadvm(Monitor *mon, const QDict *qdict) { - int saved_vm_running = runstate_is_running(); const char *name = qdict_get_str(qdict, "name"); vm_stop(RUN_STATE_RESTORE_VM); - if (load_vmstate(name) == 0 && saved_vm_running) { - vm_start(); + if (load_vmstate(name) == 0) { + load_run_state(); } } diff --git a/savevm.c b/savevm.c index a8a53ef..aa631eb 100644 --- a/savevm.c +++ b/savevm.c @@ -2143,6 +2143,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) } saved_vm_running = runstate_is_running(); + save_run_state(); vm_stop(RUN_STATE_SAVE_VM); memset(sn, 0, sizeof(*sn)); diff --git a/vl.c b/vl.c index febd2ea..7991f2e 100644 --- a/vl.c +++ b/vl.c @@ -523,6 +523,7 @@ static int default_driver_check(QemuOpts *opts, void *opaque) /* QEMU state */ static RunState current_run_state = RUN_STATE_PRELAUNCH; +static RunState saved_run_state = RUN_STATE_RUNNING; typedef struct { RunState from; @@ -546,6 +547,7 @@ static const RunStateTransition runstate_transitions_def[] = { { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE }, { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING }, + { RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED }, { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE }, { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING }, @@ -556,6 +558,7 @@ static const RunStateTransition runstate_transitions_def[] = { { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE }, { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING }, + { RUN_STATE_RESTORE_VM, RUN_STATE_PAUSED }, { RUN_STATE_RUNNING, RUN_STATE_DEBUG }, { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR }, @@ -585,11 +588,39 @@ static const RunStateTransition runstate_transitions_def[] = { static bool runstate_valid_transitions[RUN_STATE_MAX][RUN_STATE_MAX]; +void save_run_state(void) +{ + saved_run_state = current_run_state; +} + +void load_run_state(void) +{ + if (saved_run_state == RUN_STATE_RUNNING) { + vm_start(); + } else if (!runstate_check(saved_run_state)) { + runstate_set(saved_run_state); + } else { + ; /* leave unchanged */ + } +} + bool runstate_check(RunState state) { return current_run_state == state; } +static void runstate_save(QEMUFile *f, void *opaque) +{ + qemu_put_byte(f, saved_run_state); +} + +static int runstate_load(QEMUFile *f, void *opaque, int version_id) +{ + saved_run_state = qemu_get_byte(f); + + return 0; +} + static void runstate_init(void) { const RunStateTransition *p; @@ -599,6 +630,9 @@ static void runstate_init(void) for (p = &runstate_transitions_def[0]; p->from != RUN_STATE_MAX; p++) { runstate_valid_transitions[p->from][p->to] = true; } + + register_savevm(NULL, "runstate", 0, 1, + runstate_save, runstate_load, NULL); } /* This function will abort() on invalid state transitions */
This patch enables preservation of cpu runstate during save/load vm. So when a vm is restored from snapshot, the cpu runstate is restored, too. See following example: # save two vms: one is running, the other is paused (qemu) info status VM status: running (qemu) savevm running (qemu) stop (qemu) info status VM status: paused (qemu) savevm paused # restore the one running (qemu) info status VM status: paused (qemu) loadvm running (qemu) info status VM status: running # restore the one paused (qemu) loadvm paused (qemu) info status VM status: paused (qemu) cont (qemu)info status VM status: running Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- include/sysemu/sysemu.h | 2 ++ migration.c | 6 +----- monitor.c | 5 ++--- savevm.c | 1 + vl.c | 34 ++++++++++++++++++++++++++++++++++ 5 files changed, 40 insertions(+), 8 deletions(-)