[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] vl: don't use RUN_STATE_PRECONFIG as ini
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] vl: don't use RUN_STATE_PRECONFIG as initial state |
Date: |
Tue, 5 Jun 2018 11:59:32 -0300 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Tue, Jun 05, 2018 at 04:25:17PM +0200, Igor Mammedov wrote:
> On Tue, 5 Jun 2018 09:01:09 -0300
> Eduardo Habkost <address@hidden> wrote:
>
> > On Tue, Jun 05, 2018 at 10:37:55AM +0200, Igor Mammedov wrote:
> > > On Mon, 4 Jun 2018 21:56:47 -0300
> > > Eduardo Habkost <address@hidden> wrote:
> > > > On Mon, Jun 04, 2018 at 04:21:47PM +0200, Michal Privoznik wrote:
> > > > [...]
> > > > > > @@ -3572,7 +3570,12 @@ int main(int argc, char **argv, char **envp)
> > > > > > }
> > > > > > break;
> > > > > > case QEMU_OPTION_preconfig:
> > > > > > - preconfig_exit_requested = false;
> > > > > > + if (!runstate_check(RUN_STATE_NONE)) {
> > > > > > + error_report("'--preconfig' and '--incoming'
> > > > > > options are "
> > > > > > + "mutually exclusive");
> > > > > > + exit(EXIT_FAILURE);
> > > > > > + }
> > > > > > + runstate_set(RUN_STATE_PRECONFIG);
> > > > >
> > > > > Specifying --preconfig twice on the command line now fails with a very
> > > > > cryptic message (there's no --incoming).
> > > > >
> > > > > > break;
> > > > > > case QEMU_OPTION_enable_kvm:
> > > > > > olist = qemu_find_opts("machine");
> > > > > > @@ -3768,9 +3771,12 @@ int main(int argc, char **argv, char **envp)
> > > > > > }
> > > > > > break;
> > > > > > case QEMU_OPTION_incoming:
> > > > > > - if (!incoming) {
> > > > > > - runstate_set(RUN_STATE_INMIGRATE);
> > > > > > + if (!runstate_check(RUN_STATE_NONE)) {
> > > > > > + error_report("'--preconfig' and '--incoming'
> > > > > > options are "
> > > > > > + "mutually exclusive");
> > > > > > + exit(EXIT_FAILURE);
> > > > > > }
> > > > > > + runstate_set(RUN_STATE_INMIGRATE);
> > > > >
> > > > > Same here. Specifying --incoming twice fails with cryptic message. But
> > > > > one can argue that specifying --incoming twice is wrong anyway.
> > > > >
> > > >
> > > > Initially I was going to suggest simply not changing runstate
> > > > during option parsing to avoid this kind of problem, but maybe
> > > > this would be a nice way to implement the command-line parsing
> > > > rules:
> > > Is there a big reason to try making early incoming transition hack nicer?
> > > I'd rather leave it as it is for now and fix it properly later
> > > (i.e. postponing the transition after machine_done point, which is on my
> > > todo list).
> >
> > Yeah, I'm not sure we want go towards encoding more knowledge in
> > the state machine, or keeping the system simple and encoding the
> > rules in more straightforward code+variables inside main().
> I've tried before to use code+variables for preconfig checks without
> introducing a new runstate and it turned out difficult to manage
> and the later changes often broke it. With new runstate end result
> was much cleaner.
Right. I wasn't talking specifically about the new runstates
like PRECONFIG and NONE (which both make sense, IMO), but about
my suggestion below to also use runstate transition rules to
encode command-line validation and error messages.
>
> >
> > > >
> > > > case QEMU_OPTION_preconfig:
> > > > /*
> > > > * A INCOMING -> PRECONFIG transition would call:
> > > > * error_setg("--preconfig and --incoming options are
> > > > mutually exclusive");
> > > > */
> > > > try_runstate_set(RUN_STATE_PRECONFIG, &error_fatal);
> > > > break;
> > > > case QEMU_OPTION_incoming:
> > > > /*
> > > > * A PRECONFIG -> INCOMING transition would also call:
> > > > * error_setg("--preconfig and --incoming options are
> > > > mutually exclusive");
> > > > *
> > > > * Maybe a INCOMING -> INCOMING transition could
> > > > * result in:
> > > > * error_setg("--incoming can't be specified twice");
> > > > */
> > > > try_runstate_set(RUN_STATE_INMIGRATE, &error_fatal);
> > > > break;
> > > >
> > > >
> > > > > > incoming = optarg;
> > > > > > break;
> > > > > > case QEMU_OPTION_only_migratable:
> > > > >
> > > > > Michal
> > > > >
> > > >
> > >
> >
>
--
Eduardo
Re: [Qemu-devel] [PATCH v2 1/2] vl: don't use RUN_STATE_PRECONFIG as initial state, Igor Mammedov, 2018/06/04
Re: [Qemu-devel] [PATCH v2 1/2] vl: don't use RUN_STATE_PRECONFIG as initial state, Daniel P . Berrangé, 2018/06/05
[Qemu-devel] [PATCH v2 2/2] vl: fix use of --daemonize with --preconfig, Daniel P . Berrangé, 2018/06/04