Re: [Qemu-devel] [PATCH 6/7] Monitor: Don't allow cont on bad VM state

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 03 Aug 2011 17:32:03 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 2011-08-03 17:17, Luiz Capitulino wrote:
> > We have two states where issuing cont before system_reset can be
> > catastrophic: QSTATE_SHUTDOWN (when -no-shutdown is used) and
> > QSTATE_INTERROR (which only happen with kvm).
> > 
> > This commit fixes that by making system_reset mandatory before
> > issuing cont in those states.
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  cpus.c    |    4 ++++
> >  monitor.c |    8 ++++++++
> >  qerror.c  |    4 ++++
> >  qerror.h  |    3 +++
> >  sysemu.h  |    2 +-
> >  vl.c      |    1 +
> >  6 files changed, 21 insertions(+), 1 deletions(-)
> > 
> > diff --git a/cpus.c b/cpus.c
> > index 65ea503..a61e658 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -125,6 +125,10 @@ static void do_vm_stop(QemuState state)
> >          pause_all_vcpus();
> >          qemu_state_set(state);
> >          vm_state_notify(0, state);
> > +        if (state == QSTATE_INTERROR || state == QSTATE_SHUTDOWN) {
> > +            /* system_reset is required by 'cont' */
> > +            system_reset_required = 1;
> > +        }
> >          qemu_aio_flush();
> >          bdrv_flush_all();
> >          monitor_protocol_event(QEVENT_STOP, NULL);
> > diff --git a/monitor.c b/monitor.c
> > index 3fa2cf7..f1cb5af 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1312,7 +1312,14 @@ static int do_cont(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >      if (qemu_state_get() == QSTATE_INMIGRATE) {
> >          qerror_report(QERR_MIGRATION_EXPECTED);
> >          return -1;
> > +    } else if (qemu_state_get() == QSTATE_INTERROR ||
> > +               qemu_state_get() == QSTATE_SHUTDOWN) {
> > +        if (system_reset_required) {
> > +            qerror_report(QERR_RESET_REQUIRED);
> > +            return -1;
> > +        }
> 
> Why not just enter a proper state, likely QSTATE_PAUSED, when resetting
> over INTERROR or SHUTDOWN? Would save you system_reset_required and make
> the state machine simpler.

Yes, seems to be a good idea.