From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46674) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIZlc-0001C7-Og for qemu-devel@nongnu.org; Mon, 11 Jan 2016 05:32:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aIZlZ-0000dN-JF for qemu-devel@nongnu.org; Mon, 11 Jan 2016 05:32:04 -0500 Received: from mx2.parallels.com ([199.115.105.18]:60268) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIZlZ-0000cm-Dj for qemu-devel@nongnu.org; Mon, 11 Jan 2016 05:32:01 -0500 References: <1450256449-23779-1-git-send-email-den@openvz.org> <56712AF4.9030105@redhat.com> <20151216095029.GW1404639@andariel.pipo.sk> <567152C4.1010004@redhat.com> <56717988.2000207@openvz.org> From: "Denis V. Lunev" Message-ID: <56938474.3060401@openvz.org> Date: Mon, 11 Jan 2016 13:31:16 +0300 MIME-Version: 1.0 In-Reply-To: <56717988.2000207@openvz.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] qmp: process system-reset event in paused state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Krempa , qemu-devel@nongnu.org, Dmitry Andreev , Markus Armbruster On 12/16/2015 05:47 PM, Denis V. Lunev wrote: > On 12/16/2015 03:02 PM, Paolo Bonzini wrote: >> >> On 16/12/2015 10:50, Peter Krempa wrote: >>> We check that the state is "paused" and continue the vCPUs only in >>> that case. The panic devices will move the VM to 'crashed' state. >>> The code that is issuing 'system_reset' does not modify the state >>> in any way. >> Ok, thanks. >> >>> I'd say NACK here. This will break the possibility to reset a >>> system while the vCPUs are paused. The problem should be fixed in >>> libvirt. >> It is indeed a QEMU bug, and it was introduced in commit df39076 ("vl: >> allow "cont" from panicked state", 2013-11-04). >> >> Until that commit, a system_reset in panicked state would change the >> status to paused. The commit changed that as a side effect of >> removing VM_STATE_GUEST_PANICKED from runstate_needs_reset; see the >> call to runstate_needs_reset in main_loop_should_exit. >> >> IMO, after a reset, main_loop_should_exit should actually transition >> to VM_STATE_PRELAUNCH (*not* RUN_STATE_PAUSED) for *all* states except >> RUN_STATE_INMIGRATE, RUN_STATE_SAVE_VM (which I think cannot happen >> there) and (of course) RUN_STATE_RUNNING. Some changes will be required >> to the transition table as well. >> >> This will fix similar bugs for other runstates as well, though most of >> them probably cannot be triggered from libvirt. >> >> Thanks, >> >> Paolo > ok. Thank you for this input. I'll analyse this and come with > corrected patch :) > > Den What would be correct procedure to handle this state? Setting VM_STATE_PRELAUNCH in main_loop_should_exit does not move QEMU into VM_STATE_RUNNING and thus subsequent 'resume' command is necessary. In this case the processing of 'reset' command should be different in libvirt, i.e. libvirt should send two commands ('reset' and 'resume') in this state. Does I understand right? In the other case we could stick to shortcut proposed by me in the original patch. Den