Re: [Qemu-devel] [PATCH 11/12] qemu: add crash_occurred flag into CPUState

["Andreas Färber" <afaerber@suse.de>]

Hi,

This patch is clearly against QEMU, please name it "cpu: Add crash_...".
(You may want to take a second look at the non-CPU patches, too.)

Am 02.07.2015 um 18:07 schrieb Denis V. Lunev:
> From: Andrey Smetanin <asmetanin@virtuozzo.com>
> 
> CPUState->crash_occurred value inside CPUState marks

"CPUState::crash_occurred field inside ..."

> that guest crash occurred. This value added into cpu common

"value is added"

> migration subsection.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Andreas Färber <afaerber@suse.de>
> ---
>  exec.c            | 19 +++++++++++++++++++
>  include/qom/cpu.h |  1 +
>  vl.c              |  3 +++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index f7883d2..adf49e8 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -465,6 +465,24 @@ static const VMStateDescription vmstate_cpu_common_exception_index = {
>      }
>  };
>  
> +static bool cpu_common_crash_occurred_needed(void *opaque)
> +{
> +    CPUState *cpu = opaque;
> +
> +    return cpu->crash_occurred != 0;
> +}
> +
> +static const VMStateDescription vmstate_cpu_common_crash_occurred = {
> +    .name = "cpu_common/crash_occurred",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = cpu_common_crash_occurred_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(crash_occurred, CPUState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_cpu_common = {
>      .name = "cpu_common",
>      .version_id = 1,
> @@ -478,6 +496,7 @@ const VMStateDescription vmstate_cpu_common = {
>      },
>      .subsections = (const VMStateDescription*[]) {
>          &vmstate_cpu_common_exception_index,
> +        &vmstate_cpu_common_crash_occurred,
>          NULL
>      }
>  };
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 39f0f19..f559a69 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -263,6 +263,7 @@ struct CPUState {
>      bool created;
>      bool stop;
>      bool stopped;
> +    uint32_t crash_occurred;
>      volatile sig_atomic_t exit_request;
>      uint32_t interrupt_request;
>      int singlestep_enabled;

If you add this field to CPUState, you'll also need to reset it in
qom/cpu.c. Or is it intentionally persistent?

Looks good otherwise.

Regards,
Andreas

> diff --git a/vl.c b/vl.c
> index 38eee1f..9e0aee5 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1723,6 +1723,9 @@ void qemu_system_reset(bool report)
>  
>  void qemu_system_guest_panicked(void)
>  {
> +    if (current_cpu) {
> +        current_cpu->crash_occurred = 1;
> +    }
>      qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE, &error_abort);
>      vm_stop(RUN_STATE_GUEST_PANICKED);
>  }

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)