From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fPmvu-00074C-I1 for qemu-devel@nongnu.org; Mon, 04 Jun 2018 06:41:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fPmvp-0006QQ-8w for qemu-devel@nongnu.org; Mon, 04 Jun 2018 06:41:50 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50110 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fPmvp-0006Pk-3H for qemu-devel@nongnu.org; Mon, 04 Jun 2018 06:41:45 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DA5BD4022414 for ; Mon, 4 Jun 2018 10:41:43 +0000 (UTC) References: <20180604102752.32260-1-berrange@redhat.com> <8297e5b4-dca4-f080-fb87-4cf29a2bb857@redhat.com> <20180604103515.GE19749@redhat.com> From: Michal Privoznik Message-ID: <748b2e3a-194b-c769-f93b-04572cbf16d7@redhat.com> Date: Mon, 4 Jun 2018 12:41:41 +0200 MIME-Version: 1.0 In-Reply-To: <20180604103515.GE19749@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] vl: don't use RUN_STATE_PRECONFIG unless --preconfig is given List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "=?UTF-8?Q?Daniel_P._Berrang=c3=a9?=" , Max Reitz Cc: Igor Mammedov , qemu-devel@nongnu.org, Paolo Bonzini On 06/04/2018 12:35 PM, Daniel P. Berrang=C3=A9 wrote: > On Mon, Jun 04, 2018 at 12:33:04PM +0200, Max Reitz wrote: >> On 2018-06-04 12:27, Daniel P. Berrang=C3=A9 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 >>> Date: Fri May 11 19:24:43 2018 +0200 >>> >>> cli: add --preconfig option >>> >>> The global 'current_run_state' variable was changed to have an initia= l >>> value of RUN_STATE_PRECONFIG regardless of whether --preconfig is giv= en. >>> >>> It then relies on the main loop to toggle it back to RUN_STATE_PRELAU= NCH >>> when --preconfig is not given. This is racy because it means that the= re >>> 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=C3=A9 >>> --- >>> 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 befor= e >> the main loop is actually really invoked: >> >> $ echo quit | x86_64-softmmu/qemu-system-x86_64 \ >> -drive file=3D/dev/null,if=3Dide,readonly=3Don -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 invok= ed >> twice if --preconfig has been given, which is implemented by his patch= : >> >> http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg00367.html >=20 > I think we probably need a combination of both patches for maximum safe= ty. 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