[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 3/9] cli: add -preconfig option
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH v4 3/9] cli: add -preconfig option |
Date: |
Wed, 4 Apr 2018 10:51:19 +0200 |
On Tue, 3 Apr 2018 12:31:29 -0300
Eduardo Habkost <address@hidden> 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 <address@hidden> 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 <address@hidden> 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 <address@hidden> wrote:
> > > > > >
> > > > > > > On Fri, 23 Mar 2018 18:25:08 -0300
> > > > > > > Eduardo Habkost <address@hidden> 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 },