From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56381) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f3e8i-0004hW-AD for qemu-devel@nongnu.org; Wed, 04 Apr 2018 04:51:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f3e8f-0003u2-8g for qemu-devel@nongnu.org; Wed, 04 Apr 2018 04:51:32 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53768 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f3e8f-0003th-2L for qemu-devel@nongnu.org; Wed, 04 Apr 2018 04:51:29 -0400 Date: Wed, 4 Apr 2018 10:51:19 +0200 From: Igor Mammedov Message-ID: <20180404105119.1620b1b0@redhat.com> In-Reply-To: <20180403153129.GV5046@localhost.localdomain> References: <1520860275-101576-1-git-send-email-imammedo@redhat.com> <1520860275-101576-4-git-send-email-imammedo@redhat.com> <20180323212508.GF28161@localhost.localdomain> <20180327170541.295fc29a@redhat.com> <20180328134835.04f95535@redhat.com> <20180328192148.GR5046@localhost.localdomain> <20180329134303.548a864b@redhat.com> <20180329162409.GX5046@localhost.localdomain> <20180403163253.1d13633f@redhat.com> <20180403153129.GV5046@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 3/9] cli: add -preconfig option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: peter.maydell@linaro.org, pkrempa@redhat.com, cohuck@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com, david@gibson.dropbear.id.au On Tue, 3 Apr 2018 12:31:29 -0300 Eduardo Habkost wrote: > On Tue, Apr 03, 2018 at 04:32:53PM +0200, Igor Mammedov wrote: > > On Thu, 29 Mar 2018 13:24:09 -0300 > > Eduardo Habkost wrote: > > > > > On Thu, Mar 29, 2018 at 01:43:03PM +0200, Igor Mammedov wrote: > > > > On Wed, 28 Mar 2018 16:21:48 -0300 > > > > Eduardo Habkost wrote: > > > > > > > > > On Wed, Mar 28, 2018 at 01:48:35PM +0200, Igor Mammedov wrote: > > > > > > On Tue, 27 Mar 2018 17:05:41 +0200 > > > > > > Igor Mammedov wrote: > > > > > > > > > > > > > On Fri, 23 Mar 2018 18:25:08 -0300 > > > > > > > Eduardo Habkost wrote: > > [...] > > > > > This doesn't make sense to me. Why would we enter > > > > > RUN_STATE_PRECONFIG state if -preconfig is not used at all? > > > > because of RUN_STATE_PRECONFIG becomes new initial state of > > > > our state machine where we start of (used to be RUN_STATE_PRELAUNCH) > > > > > > Oh, I missed that part. > > > > > > > Lets call it variant 1: > > > > > > > > with this we have 2 possible transitions: > > > > RUN_STATE_PRECONFIG -> RUN_STATE_PRELAUNCH (machine_init) > > > > > > > > and > > > > > > > > RUN_STATE_PRECONFIG -> RUN_STATE_INMIGRATE > > > > ugly but it was the same with RUN_STATE_PRELAUNCH initial transition > > [...] > > > > > > Thanks, now variant 1 makes more sense to me. But I really miss > > > here are very clear and explicit descriptions of what each state > > > really mean, and what are the differences between them. > > > > > > It looks like the existing description for `prelaunch` isn't > > > accurate: > > > > > > # @prelaunch: QEMU was started with -S and guest has not started > > > > > > This is false, as QEMU can be in `prelaunch` state even if -S is > > > not used. > > > > > > > > > Also, this is the description you proposed for `preconfig`: > > > > > > # @preconfig: QEMU is paused before board specific init callback is executed. > > > # The state is reachable only if -preconfig CLI option is used. > > > # (Since 2.12) > > > > > > This seems wrong as well: the `prelaunch` state is reachable even > > > if `-preconfig` isn't used in the command-line (because it is the > > > initial state). > > I'm not sure we should describe transitions/initial state here > > (it's not the case now). > > > > I think descriptions 'almost' match 'end' result of what QMP client > > cloud see and the initial state is not something that QMP user could > > observe. > > That's my impression as well: `prelaunch` should be visible > externally only if `-S` was used, and `preconfig` should be > visible externally only if `-preconfig` was used. > > However, I miss documentation on what are the > expectations/requirements internally. e.g. there's no > explanation why a reset request moves QEMU to > RUN_STATE_PRELAUNCH. Ideally, each line in > runstate_transition_def would have a clear explanation for > when/why each transition happens. > > But this isn't a requirement for the new feature you are > implementing, anyway. I'll add TODO comment to inmigrate to transition I'm adding, to tag it for future work. + /* Early switch to inmigrate state to allow + * -incoming CLI option work as it used to. + * TODO: delay actual switching to inmigrate + * state to the point after machine is built + * and remove this hack. + */ + { RUN_STATE_PRECONFIG, RUN_STATE_INMIGRATE },