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 <[email protected]>
> ---
> 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 <[email protected]>
> > ---
> > 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