From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH 04/15] Improve vm_stop reason declarations
Date: Wed, 9 Feb 2011 12:17:00 -0200
Message-ID: <20110209141700.GB32689@amt.cnet>
References: <cover.1297077506.git.jan.kiszka@siemens.com>
 <35fe72889a43ad4465c298a70069e9a6fc1eab66.1297077506.git.jan.kiszka@siemens.com>
 <20110208185932.GB13617@amt.cnet>
 <4D524B25.80907@web.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org,
	qemu-devel@nongnu.org
To: Jan Kiszka <jan.kiszka@web.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:8407 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755652Ab1BIOoz (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 9 Feb 2011 09:44:55 -0500
Content-Disposition: inline
In-Reply-To: <4D524B25.80907@web.de>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Feb 09, 2011 at 09:07:01AM +0100, Jan Kiszka wrote:
> On 2011-02-08 19:59, Marcelo Tosatti wrote:
> > On Mon, Feb 07, 2011 at 12:19:15PM +0100, Jan Kiszka wrote:
> >> index d6556c9..3397566 100644
> >> --- a/gdbstub.c
> >> +++ b/gdbstub.c
> >> @@ -2194,14 +2194,14 @@ static void gdb_vm_state_change(void *opaque, int running, int reason)
> >>      const char *type;
> >>      int ret;
> >>  
> >> -    if (running || (reason != EXCP_DEBUG && reason != EXCP_INTERRUPT) ||
> >> -        s->state == RS_INACTIVE || s->state == RS_SYSCALL)
> >> +    if (running || (reason != VMSTOP_DEBUG && reason != VMSTOP_INTERRUPT) ||
> >> +        s->state == RS_INACTIVE || s->state == RS_SYSCALL) {
> >>          return;
> > 
> > What about VMSTOP_USER ?
> > 
> > VMSTOP_INTERRUPT -> "the VM is stopped by an interrupt".
> > 
> > VMSTOP_USER -> "the VM is stopped by the user".
> 
> Makes a lot of sense, will change.
> 
> > 
> >> diff --git a/migration.c b/migration.c
> >> index 3612572..20ea113 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -378,7 +378,7 @@ void migrate_fd_put_ready(void *opaque)
> >>          int old_vm_running = vm_running;
> >>  
> >>          DPRINTF("done iterating\n");
> >> -        vm_stop(0);
> >> +        vm_stop(VMSTOP_RWSTATE);
> > 
> > VMSTOP_RWSTATE is cryptic. What about VMSTOP_SAVEVM, MIGRATE, etc.
> 
> Both alternatives are not completely accurate as we also stop of vmload.
> What about VMSTOP_SYNC_STATE or UPDATE_STATE?

IMO it would be better to state the specific reason why the vm stopped,
not some generic action. VMSTOP_SAVEVM, VMSTOP_VMLOAD and
VMSTOP_MIGRATE.