From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:33844)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1QofIi-0002E6-WA
	for qemu-devel@nongnu.org; Wed, 03 Aug 2011 13:32:13 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1QofIh-0005iB-QE
	for qemu-devel@nongnu.org; Wed, 03 Aug 2011 13:32:12 -0400
Received: from mx1.redhat.com ([209.132.183.28]:52454)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1QofIh-0005i7-Hi
	for qemu-devel@nongnu.org; Wed, 03 Aug 2011 13:32:11 -0400
Date: Wed, 3 Aug 2011 14:32:06 -0300
From: Luiz Capitulino <lcapitulino@redhat.com>
Message-ID: <20110803143206.420c2418@doriath>
In-Reply-To: <4E3969F3.4080703@siemens.com>
References: <1312384643-581-1-git-send-email-lcapitulino@redhat.com>
	<1312384643-581-7-git-send-email-lcapitulino@redhat.com>
	<4E3969F3.4080703@siemens.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 6/7] Monitor: Don't allow cont on bad VM
	state
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: blauwirbel@gmail.com, amit.shah@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@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.