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

[Daniel P . Berrangé]

On Mon, Jun 04, 2018 at 12:33:04PM +0200, Max Reitz 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.
> > 
> > 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

I think we probably need a combination of both patches for maximum safety.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|