From: Anton Nefedov <anton.nefedov@virtuozzo.com>

Do not make some foreign function do it.

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 kvm-all.c |  5 ++++-
 vl.c      | 22 +++++++++-------------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 0c94637..ac07902 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1909,6 +1909,7 @@ int kvm_cpu_exec(CPUState *cpu)
 
     do {
         MemTxAttrs attrs;
+        GuestPanicInformation *crash_info;
 
         if (cpu->kvm_vcpu_dirty) {
             kvm_arch_put_registers(cpu, KVM_PUT_RUNTIME_STATE);
@@ -2001,9 +2002,11 @@ int kvm_cpu_exec(CPUState *cpu)
                 break;
             case KVM_SYSTEM_EVENT_CRASH:
                 kvm_cpu_synchronize_state(cpu);
+                crash_info = cpu_get_crash_info(cpu);
                 qemu_mutex_lock_iothread();
-                qemu_system_guest_panicked(cpu_get_crash_info(cpu));
+                qemu_system_guest_panicked(crash_info);
                 qemu_mutex_unlock_iothread();
+                qapi_free_GuestPanicInformation(crash_info);
                 ret = 0;
                 break;
             default:
diff --git a/vl.c b/vl.c
index 27d9829..81382b6 100644
--- a/vl.c
+++ b/vl.c
@@ -1682,6 +1682,15 @@ void qemu_system_reset(bool report)
 void qemu_system_guest_panicked(GuestPanicInformation *info)
 {
     qemu_log_mask(LOG_GUEST_ERROR, "Guest crashed\n");
+    if (info && info->type == GUEST_PANIC_INFORMATION_KIND_HYPER_V) {
+        qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
+                      " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
+                      info->u.hyper_v.data->arg1,
+                      info->u.hyper_v.data->arg2,
+                      info->u.hyper_v.data->arg3,
+                      info->u.hyper_v.data->arg4,
+                      info->u.hyper_v.data->arg5);
+    }
 
     if (current_cpu) {
         current_cpu->crash_occurred = true;
@@ -1694,19 +1703,6 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
                                        !!info, info, &error_abort);
         qemu_system_shutdown_request();
     }
-
-    if (info) {
-        if (info->type == GUEST_PANIC_INFORMATION_KIND_HYPER_V) {
-            qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
-                          " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
-                          info->u.hyper_v.data->arg1,
-                          info->u.hyper_v.data->arg2,
-                          info->u.hyper_v.data->arg3,
-                          info->u.hyper_v.data->arg4,
-                          info->u.hyper_v.data->arg5);
-        }
-        qapi_free_GuestPanicInformation(info);
-    }
 }
 
 void qemu_system_reset_request(void)
-- 
2.7.4

On 02/20/2017 12:21 PM, Denis V. Lunev wrote:
> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> Do not make some foreign function do it.
> 
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> ---
>  kvm-all.c |  5 ++++-
>  vl.c      | 22 +++++++++-------------
>  2 files changed, 13 insertions(+), 14 deletions(-)
> 

> +++ b/vl.c
> @@ -1682,6 +1682,15 @@ void qemu_system_reset(bool report)
>  void qemu_system_guest_panicked(GuestPanicInformation *info)
>  {
>      qemu_log_mask(LOG_GUEST_ERROR, "Guest crashed\n");
> +    if (info && info->type == GUEST_PANIC_INFORMATION_KIND_HYPER_V) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
> +                      " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
> +                      info->u.hyper_v.data->arg1,
> +                      info->u.hyper_v.data->arg2,
> +                      info->u.hyper_v.data->arg3,
> +                      info->u.hyper_v.data->arg4,
> +                      info->u.hyper_v.data->arg5);
> +    }

Why are we hoisting this code earlier in the function (which changes the
log order, if I'm reading correctly)...

>  
>      if (current_cpu) {
>          current_cpu->crash_occurred = true;
> @@ -1694,19 +1703,6 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>                                         !!info, info, &error_abort);
>          qemu_system_shutdown_request();
>      }
> -
> -    if (info) {
> -        if (info->type == GUEST_PANIC_INFORMATION_KIND_HYPER_V) {
> -            qemu_log_mask(LOG_GUEST_ERROR, "HV crash parameters: (%#"PRIx64
> -                          " %#"PRIx64" %#"PRIx64" %#"PRIx64" %#"PRIx64")\n",
> -                          info->u.hyper_v.data->arg1,
> -                          info->u.hyper_v.data->arg2,
> -                          info->u.hyper_v.data->arg3,
> -                          info->u.hyper_v.data->arg4,
> -                          info->u.hyper_v.data->arg5);
> -        }
> -        qapi_free_GuestPanicInformation(info);
> -    }

...instead of just cleaning up this code in place?

But the rest of the patch looks fine.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org