On 06/04/2018 12:35 PM, Daniel P. Berrangé wrote: > 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 <imamm...@redhat.com> >>> 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é <berra...@redhat.com> >>> --- >>> 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.
Yes, looks like that. Except when your patch is merged then: + runstate_set(RUN_STATE_PRELAUNCH); from my patch becomes unnecessary. So I'll wait for you to merge your patch and then I can resend v2 of mine. Michal