From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56638) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBUNw-0004iN-TS for qemu-devel@nongnu.org; Wed, 05 Oct 2011 12:31:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RBUNv-0005Uc-Jg for qemu-devel@nongnu.org; Wed, 05 Oct 2011 12:31:56 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:34935) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBUNv-0005UN-6N for qemu-devel@nongnu.org; Wed, 05 Oct 2011 12:31:55 -0400 Message-ID: <4E8C8656.3040706@web.de> Date: Wed, 05 Oct 2011 18:31:18 +0200 From: Jan Kiszka 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: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig7A8ED6B6D386BE4CD1859932" Sender: jan.kiszka@web.de 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: Avi Kivity Cc: Paolo Bonzini , qemu-devel@nongnu.org, Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig7A8ED6B6D386BE4CD1859932 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2011-10-05 17:43, 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 ->=20 >> 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 alrea= dy >> 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 ar= e >> 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 import= ant >> detail: how should we implement vm_start() (and thus 'cont')? >> >> In order to maintain how we behave with the external world, the o= nly >> 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. >> >=20 > 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. >=20 > 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 fi= x > 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. Making this kind of user-visible change would be a bad idea. We are talking about multiple stop states here, but only a single function (vm_stop) to enter them - maybe that's not optimal. But the point is that we were missing one stop-to-stop transition. And that needs to be fixed, either inside vm_stop or when it is called. If you want to lock the VM into paused state, add a new symmetric API that does precisely this. That API would send the VM into RSTATE_LOCKED if it is not yet stopped on lock or is still locked on resume. That would avoid redefining stop states that have no use for lock-counting semantics. Jan --------------enig7A8ED6B6D386BE4CD1859932 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk6MhlsACgkQitSsb3rl5xRyxwCgtKdWGSqJ9txXC2ypQGuGNxft QeUAoJuXnOyIdpShGOSvMkUmipQWohCK =kOz9 -----END PGP SIGNATURE----- --------------enig7A8ED6B6D386BE4CD1859932--