Re: [Qemu-devel] [PATCH 3/7] QemuState: Add additional states

[Luiz Capitulino]

On Thu, 04 Aug 2011 11:02:06 +0200
Markus Armbruster <address@hidden> wrote:

> Luiz Capitulino <address@hidden> writes:
> 
> > Currently, only vm_start() and vm_stop() change the VM state. That's,
> > the state is only changed when starting or stopping the VM.
> >
> > This commit adds the qemu_state_set() function, making it possible
> > to also do state transitions when qemu is stopped or running.
> >
> > Additional states are also added and the current state is stored.
> > This is going to be used by the next commits.
> >
> > Signed-off-by: Luiz Capitulino <address@hidden>
> > ---
> >  cpus.c      |    1 +
> >  migration.c |    8 +++++++-
> >  sysemu.h    |   10 +++++++++-
> >  vl.c        |   20 ++++++++++++++++++++
> >  4 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/cpus.c b/cpus.c
> > index ebbb8b9..48e6ca1 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -124,6 +124,7 @@ static void do_vm_stop(QemuState state)
> >          cpu_disable_ticks();
> >          vm_running = 0;
> >          pause_all_vcpus();
> > +        qemu_state_set(state);
> >          vm_state_notify(0, state);
> >          qemu_aio_flush();
> >          bdrv_flush_all();
> > diff --git a/migration.c b/migration.c
> > index 9724ce0..8aacf64 100644
> > --- a/migration.c
> > +++ b/migration.c
> > @@ -72,8 +72,11 @@ void process_incoming_migration(QEMUFile *f)
> >  
> >      incoming_expected = false;
> >  
> > -    if (autostart)
> > +    if (autostart) {
> >          vm_start();
> > +    } else {
> > +        qemu_state_set(QSTATE_PRELAUNCH);
> > +    }
> >  }
> >  
> >  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > @@ -394,6 +397,9 @@ void migrate_fd_put_ready(void *opaque)
> >              }
> >              state = MIG_STATE_ERROR;
> >          }
> > +        if (state == MIG_STATE_COMPLETED) {
> > +            qemu_state_set(QSTATE_POSTMIGRATE);
> > +        }
> >          s->state = state;
> >          notifier_list_notify(&migration_state_notifiers, NULL);
> >      }
> > diff --git a/sysemu.h b/sysemu.h
> > index 2c3ea3d..32c9abb 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -11,16 +11,22 @@
> >  /* vl.c */
> >  
> >  typedef enum {
> > +    QSTATE_NOSTATE,
> 
> QSTATE_NO_STATE?
> 
> >      QSTATE_DEBUG,         /* qemu is running under gdb */
> > +    QSTATE_INMIGRATE,     /* paused waiting for an incoming migration */
> 
> QSTATE_IN_MIGRATE?
> 
> >      QSTATE_INTERROR,      /* paused due to an internal error */
> 
> QSTATE_PANICKED?
> 
> >      QSTATE_IOERROR,       /* paused due to an I/O error */
> 
> QSTATE_IO_ERROR?
> 
> >      QSTATE_PAUSED,        /* paused by the user (ie. the 'stop' command) */
> > +    QSTATE_POSTMIGRATE,   /* paused following a successful migration */
> 
> QSTATE_POST_MIGRATE?
> 
> > +    QSTATE_PRELAUNCH,     /* qemu was started with -S and haven't started 
> > */
> 
> QSTATE_PRE_LAUNCH?
> 
> >      QSTATE_PREMIGRATE,    /* paused preparing to finish migrate */
> 
> QSTATE_PRE_MIGRATE?
> 
> >      QSTATE_RESTVM,        /* paused restoring the VM state */
> > +    QSTATE_RESTVMFAILED,  /* paused due to a failed attempt to load state 
> > */
> 
> QSTATE_RESTVM_FAILED?
> 
> Consistently separating words by spaces became a general custom about
> the tenth century A.D., and lasted until about 1957, when FORTRAN
> abandoned the practice.
>       -- Sun FORTRAN Reference Manual
> 
> >      QSTATE_RUNNING,       /* qemu is running */
> >      QSTATE_SAVEVM,        /* paused saving VM state */
> >      QSTATE_SHUTDOWN,      /* guest shut down and -no-shutdown is in use */
> > -    QSTATE_WATCHDOG       /* watchdog fired and qemu is configured to 
> > pause */
> > +    QSTATE_WATCHDOG,      /* watchdog fired and qemu is configured to 
> > pause */
> > +    QSTATE_MAX
> >  } QemuState;
> >  
> >  extern const char *bios_name;
> > @@ -31,6 +37,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"
> >  
> > +QemuState qemu_state_get(void);
> > +void qemu_state_set(QemuState state);
> >  typedef struct vm_change_state_entry VMChangeStateEntry;
> >  typedef void VMChangeStateHandler(void *opaque, int running, QemuState 
> > state);
> >  
> > diff --git a/vl.c b/vl.c
> > index faa7c5f..2619c8e 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -320,6 +320,22 @@ static int default_driver_check(QemuOpts *opts, void 
> > *opaque)
> >  }
> >  
> >  /***********************************************************/
> > +/* QEMU state */
> > +
> > +static QemuState qemu_current_state = QSTATE_NOSTATE;
> > +
> > +QemuState qemu_state_get(void)
> > +{
> > +    return qemu_current_state;
> > +}
> > +
> > +void qemu_state_set(QemuState state)
> > +{
> > +    assert(state < QSTATE_MAX);
> 
> Beware, comparison is signed if QemuState is signed (implementation
> defined; QSTATE_MAX is int).

It's unsigned here and I got the expected warning when I did:

 assert(state >= 0);

Don't how to address that (besides dropping the check).

> 
> > +    qemu_current_state = state;
> > +}
> > +
> > +/***********************************************************/
> [...]
>