lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 04 Mar 2013 10:40:15 +0100
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Hu Tao <hutao@...fujitsu.com>
CC:	kvm list <kvm@...r.kernel.org>, qemu-devel <qemu-devel@...gnu.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Daniel P. Berrange" <berrange@...hat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Jan Kiszka <jan.kiszka@...mens.com>,
	Gleb Natapov <gleb@...hat.com>,
	Blue Swirl <blauwirbel@...il.com>,
	Eric Blake <eblake@...hat.com>,
	Andrew Jones <drjones@...hat.com>,
	Marcelo Tosatti <mtosatti@...hat.com>,
	Sasha Levin <levinsasha928@...il.com>,
	Luiz Capitulino <lcapitulino@...hat.com>,
	Anthony Liguori <aliguori@...ibm.com>,
	Markus Armbruster <armbru@...hat.com>,
	Stefan Hajnoczi <stefanha@...hat.com>,
	Juan Quintela <quintela@...hat.com>,
	Orit Wasserman <owasserm@...hat.com>,
	Kevin Wolf <kwolf@...hat.com>,
	Wen Congyang <wency@...fujitsu.com>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Alexander Graf <agraf@...e.de>,
	Alex Williamson <alex.williamson@...hat.com>,
	Peter Maydell <peter.maydell@...aro.org>
Subject: Re: [PATCH v13 4/8] add a new runstate: RUN_STATE_GUEST_PANICKED

Il 28/02/2013 13:13, Hu Tao ha scritto:
> The guest will be in this state when it is panicked.
> 
> Signed-off-by: Wen Congyang <wency@...fujitsu.com>
> Signed-off-by: Hu Tao <hutao@...fujitsu.com>
> ---
>  migration.c      |  1 +
>  qapi-schema.json |  6 +++++-
>  qmp.c            |  3 ++-
>  vl.c             | 11 ++++++++++-
>  4 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index c29830e..fa17b82 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -698,6 +698,7 @@ static void *buffered_file_thread(void *opaque)
>                  int64_t start_time, end_time;
>  
>                  DPRINTF("done iterating\n");
> +                save_run_state();
>                  start_time = qemu_get_clock_ms(rt_clock);
>                  qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>                  if (old_vm_running) {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 28b070f..8f1d138 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -174,11 +174,15 @@
>  # @suspended: guest is suspended (ACPI S3)
>  #
>  # @watchdog: the watchdog action is configured to pause and has been triggered
> +#
> +# @guest-panicked: the panicked action is configured to pause and has been
> +# triggered.
>  ##
>  { 'enum': 'RunState',
>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
> -            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
> +            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> +            'guest-panicked' ] }
>  
>  ##
>  # @SnapshotInfo
> diff --git a/qmp.c b/qmp.c
> index 5f1bed1..f5027f6 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -150,7 +150,8 @@ void qmp_cont(Error **errp)
>      Error *local_err = NULL;
>  
>      if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> -               runstate_check(RUN_STATE_SHUTDOWN)) {
> +            runstate_check(RUN_STATE_SHUTDOWN) ||
> +            runstate_check(RUN_STATE_GUEST_PANICKED)) {
>          error_set(errp, QERR_RESET_REQUIRED);
>          return;
>      } else if (runstate_check(RUN_STATE_SUSPENDED)) {
> diff --git a/vl.c b/vl.c
> index 3d08e1a..51d4922 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -536,6 +536,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>  
>      { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
> +    { RUN_STATE_INMIGRATE, RUN_STATE_GUEST_PANICKED },

Is this a consequence of the first patch?

>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_RUNNING },
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
> @@ -549,6 +550,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_PAUSED },
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_POSTMIGRATE, RUN_STATE_GUEST_PANICKED },

Impossible.  GUEST_PANICKED requires an instruction to be executed in
the guest, so it should first go to RUNNING.

>      { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
>      { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
> @@ -559,6 +561,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>  
>      { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
>      { RUN_STATE_RESTORE_VM, RUN_STATE_PAUSED },
> +    { RUN_STATE_RESTORE_VM, RUN_STATE_GUEST_PANICKED },

Is it also for the first patch?

>      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
>      { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
> @@ -569,6 +572,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM },
>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
> +    { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },

This one is obviously ok.

>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>  
> @@ -583,6 +587,10 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>  
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },

Like SHUTDOWN, it should go first to PAUSED and then to RUNNING.  A
GUEST_PANICKED -> RUNNING transition is not possible.  You're seeing it
because you lack the addition of GUEST_PANICKED here:

        if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
            runstate_check(RUN_STATE_SHUTDOWN)) {
            runstate_set(RUN_STATE_PAUSED);
        }

I think you should first move the INTERNAL_ERROR || SHUTDOWN checks to a
separate function, so that you can then add GUEST_PANICKED.

Paolo

>      { RUN_STATE_MAX, RUN_STATE_MAX },
>  };
>  
> @@ -2001,7 +2009,8 @@ static bool main_loop_should_exit(void)
>          qemu_system_reset(VMRESET_REPORT);
>          resume_all_vcpus();
>          if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> -            runstate_check(RUN_STATE_SHUTDOWN)) {
> +            runstate_check(RUN_STATE_SHUTDOWN) ||
> +            runstate_check(RUN_STATE_GUEST_PANICKED)) {
>              bdrv_iterate(iostatus_bdrv_it, NULL);
>              vm_start();
>          }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ