Re: [Qemu-devel] [PATCH v13 1/8] save/load cpu runstate

[Hu Tao]

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 <address@hidden>
> > ---
> >  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 */
> >