From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39907) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBTee-0000YN-H8 for qemu-devel@nongnu.org; Wed, 05 Oct 2011 11:45:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RBTeT-0003s1-KJ for qemu-devel@nongnu.org; Wed, 05 Oct 2011 11:45:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34760) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBTeT-0003ro-8M for qemu-devel@nongnu.org; Wed, 05 Oct 2011 11:44:57 -0400 Message-ID: <4E8C7B73.60300@redhat.com> Date: Wed, 05 Oct 2011 17:44:51 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1317729885-17534-1-git-send-email-pbonzini@redhat.com> <20111005113707.5312f98b@doriath> <4E8C7B1C.2020808@redhat.com> In-Reply-To: <4E8C7B1C.2020808@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Paolo Bonzini , Jan Kiszka , qemu-devel@nongnu.org On 10/05/2011 05:43 PM, Avi Kivity wrote: > On 10/05/2011 04:37 PM, Luiz Capitulino wrote: >> Now, we have three options to fix this but I don't know which one to >> choose: >> >> 1. We could just add the transition RSTATE_PAUSED -> >> RSTATE_POST_MIGRATE >> as valid. Not sure this is a good thing to do though, as it >> seems a silly >> workaround for the fact that the transition to >> RSTATE_PRE_MIGRATE has >> never occurred >> >> 2. This patch makes vm_stop() do the state transition even if the VM >> is already stopped. Seems good enough, except that I fear two >> things. >> First, today we know that vm_stop() is a no-op if the VM is already >> stopped, so there's a semantic change that could turn out to be >> trap. >> Second, I also fear people using vm_stop() as a way to change the >> VM status, just like runstate_set() (which can also become an >> horrible >> trap) >> >> 3. Avi suggested we should keep a reference count, so that states are >> not discarded: >> >> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00595.html >> >> That solution seemed to be the perfect one, except for one >> important >> detail: how should we implement vm_start() (and thus 'cont')? >> >> In order to maintain how we behave with the external world, the >> only >> option is that vm_start() will set the stop count to 0. Ie, doesn't >> matter if we have stopped the VM 500 times at some point, a >> vm_start() >> call will discard all stored states. >> >> Not sure if that's what you expected, but the first time I read >> Avi's >> idea I had the impression that it would be a good idea that >> vm_start() >> decremented the ref count only once, ie. vm_stop() and >> vm_start() calls >> have to match. >> > > vm_start() should be symmetric with vm_stop(). That is, if a piece of > code wants to execute with vcpus stopped, it should just run inside a > stop/start pair. > > The only confusion can come from the user, if he sees multiple stop > events and expects that just one cont will continue the vm. For the > machine monitor, we should just document that the you have to issue > one cont for every stop event you see (plus any stops you issue). > It's not unnatural - the code that handles a stop_due_to_enospace can > work to fix the error and issue a cont, disregarding any other stops > in progress (due to a user pressing the stop button, or migration, or > cpu hotplug, or whatever). For the human monitor, it's not so > intuitive, but the situation is so rare we can just rely on the user > to issue cont again. > btw, another way to look at it is like a lock: vm_lock() / vm_unlock(). It's obvious that vm_unlock() can't release all the locks. -- error compiling committee.c: too many arguments to function