Re: [Qemu-devel] [PATCH] vl: don't use RUN_STATE_PRECONFIG unless --preconfig is given

[Igor Mammedov]

On Mon, 4 Jun 2018 12:33:04 +0200
Max Reitz <address@hidden> wrote:

> On 2018-06-04 12:27, Daniel P. Berrangé wrote:
> > The RUN_STATE_PRECONFIG state is not supposed to be reachable unless the
> > --preconfig argument is given to QEMU, but when it was introduced in:
> > 
> >   commit 047f7038f586d2150f16c6d9ba9cfd0479f0f6ac
> >   Author: Igor Mammedov <address@hidden>
> >   Date:   Fri May 11 19:24:43 2018 +0200
> > 
> >     cli: add --preconfig option
> > 
> > The global 'current_run_state' variable was changed to have an initial
> > value of RUN_STATE_PRECONFIG regardless of whether --preconfig is given.
That was intentional, see 8a36283e12 commit message which has it right.
I should fix doc comment qapi/run-state.json though as it's misleading
(I've meant it reachable for user, while doc comment applies to
not only to user but to qemu internals as well).
I'll post a patch to clarify doc comment.

Idea behind PRECONFIG runstate is to start spitting PRELAUNCH
runstate (which is historically ended up as a dump for almost everything)
into smaller more manageable part.

Michal's patch should fix issue short term and
I'll look more at the issues Max found out, maybe there is more to fix.

> > It then relies on the main loop to toggle it back to RUN_STATE_PRELAUNCH
> > when --preconfig is not given. This is racy because it means that there
> > is a window where QEMU is in RUN_STATE_PRECONFIG despite --preconfig not
> > being given. This can be seen with the failure:
> > 
> >   $ echo | x86_64-softmmu/qemu-system-x86_64 -monitor stdio
> >   QEMU 2.12.50 monitor - type 'help' for more information
> >   (qemu)
> >   HMP not available in preconfig state, use QMP instead
> > 
> > Signed-off-by: Daniel P. Berrangé <address@hidden>
> > ---
> >  vl.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)  
> 
> This indeed fixes the issue that the preconfig state is reachable
> without --preconfig, but it still keeps the main loop being invoked
> twice (which means that e.g. HMP will process a single character before
> the main loop is actually really invoked:
> 
> $ echo quit | x86_64-softmmu/qemu-system-x86_64 \
>     -drive file=/dev/null,if=ide,readonly=on -monitor stdio
> QEMU 2.12.50 monitor - type 'help' for more information
> (qemu) qqemu-system-x86_64: Initialization of device ide-hd failed:
> Block node is read-only
> 
> (Note the "q" before "qemu-system-x86_64"))
> 
> (Naively,) I agree with Michal that the main loop should only be invoked
> twice if --preconfig has been given, which is implemented by his patch:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00367.html
> 
> Max
>