From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42548)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1aMCaN-0004Y3-Dz
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 05:35:28 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1aMCaI-0002iG-DN
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 05:35:27 -0500
Received: from mx1.redhat.com ([209.132.183.28]:54178)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1aMCaI-0002hy-7S
	for qemu-devel@nongnu.org; Thu, 21 Jan 2016 05:35:22 -0500
References: <1453190362-2127-1-git-send-email-den@openvz.org>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <56A0B465.9040109@redhat.com>
Date: Thu, 21 Jan 2016 11:35:17 +0100
MIME-Version: 1.0
In-Reply-To: <1453190362-2127-1-git-send-email-den@openvz.org>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/1] vl: change QEMU state machine for
	system reset
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Denis V. Lunev" <den@openvz.org>
Cc: qemu-devel@nongnu.org, Dmitry Andreev <dandreev@virtuozzo.com>



On 19/01/2016 08:59, Denis V. Lunev wrote:
> @@ -612,8 +617,10 @@ static const RunStateTransition runstate_transitions_def[] = {
>  
>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> +    { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
>  
>      { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
> +    { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
>  
>      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
>      { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
> @@ -627,20 +634,25 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>  
>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
> +    { RUN_STATE_SAVE_VM, RUN_STATE_PRELAUNCH },

This should not happen; thus I would remove this line.

>      { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
>      { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
>  
>      { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
>      { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
>      { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
>      { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_SUSPENDED, RUN_STATE_PRELAUNCH },
>  
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_WATCHDOG, RUN_STATE_PRELAUNCH },
>  
>      { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
>      { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PRELAUNCH },
>  
>      { RUN_STATE__MAX, RUN_STATE__MAX },
>  };
> @@ -1886,8 +1899,10 @@ static bool main_loop_should_exit(void)
>          cpu_synchronize_all_states();
>          qemu_system_reset(VMRESET_REPORT);
>          resume_all_vcpus();
> -        if (runstate_needs_reset()) {
> -            runstate_set(RUN_STATE_PAUSED);
> +        if (!runstate_check(RUN_STATE_RUNNING) &&
> +                !runstate_check(RUN_STATE_INMIGRATE) &&
> +                !runstate_check(RUN_STATE_SAVE_VM)) {

Since SAVE_VM should not happen here, I would leave this check out too.
 If it happens, the lack of a SAVE_VM->PRELAUNCH transition will cause
an assertion failure.

Thanks for the patch!

Paolo

> +            runstate_set(RUN_STATE_PRELAUNCH);
>          }
>      }
>      if (qemu_wakeup_requested()) {
>