[Qemu-devel] [PATCH v2 3/3] i386/cpu: net: Flatten simple union GuestPanicInformationType

Markus Armbruster posted 3 patches 8 years, 11 months ago
[Qemu-devel] [PATCH v2 3/3] i386/cpu: net: Flatten simple union GuestPanicInformationType
Posted by Markus Armbruster 8 years, 11 months ago
Simple unions are simpler than flat unions in the schema, but more
complicated in C and on the QMP wire: there's extra indirection in C
and extra nesting on the wire, both pointless.  They're best avoided
in new code.  Fix up recent commit d187e08 accordingly.

Cc: Anton Nefedov <anton.nefedov@virtuozzo.com>
Cc: Denis V. Lunev <den@openvz.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi-schema.json  | 14 ++++++++++++--
 target/i386/cpu.c | 17 ++++++-----------
 vl.c              | 12 ++++++------
 3 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 5347781..0eef37c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5893,14 +5893,24 @@
   'data': [ 'pause', 'poweroff' ] }
 
 ##
+# @GuestPanicInformationType:
+#
+# Since: 2.9
+##
+{ 'enum': 'GuestPanicInformationType',
+  'data': [ 'hyper-v' ] }
+
+##
 # @GuestPanicInformation:
 #
 # Information about a guest panic
 #
 # Since: 2.9
 ##
-{'union': 'GuestPanicInformation',
- 'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
+{ 'union': 'GuestPanicInformation',
+  'base': { 'type': 'GuestPanicInformationType' },
+  'discriminator': 'type',
+  'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
 
 ##
 # @GuestPanicInformationHyperV:
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index fd7add2..c3e6b74 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3502,19 +3502,14 @@ static GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs)
     GuestPanicInformation *panic_info = NULL;
 
     if (env->features[FEAT_HYPERV_EDX] & HV_X64_GUEST_CRASH_MSR_AVAILABLE) {
-        GuestPanicInformationHyperV *panic_info_hv =
-            g_malloc0(sizeof(GuestPanicInformationHyperV));
         panic_info = g_malloc0(sizeof(GuestPanicInformation));
-
-        panic_info->type = GUEST_PANIC_INFORMATION_KIND_HYPER_V;
-        panic_info->u.hyper_v.data = panic_info_hv;
-
+        panic_info->type = GUEST_PANIC_INFORMATION_TYPE_HYPER_V;
         assert(HV_X64_MSR_CRASH_PARAMS >= 5);
-        panic_info_hv->arg1 = env->msr_hv_crash_params[0];
-        panic_info_hv->arg2 = env->msr_hv_crash_params[1];
-        panic_info_hv->arg3 = env->msr_hv_crash_params[2];
-        panic_info_hv->arg4 = env->msr_hv_crash_params[3];
-        panic_info_hv->arg5 = env->msr_hv_crash_params[4];
+        panic_info->u.hyper_v.arg1 = env->msr_hv_crash_params[0];
+        panic_info->u.hyper_v.arg2 = env->msr_hv_crash_params[1];
+        panic_info->u.hyper_v.arg3 = env->msr_hv_crash_params[2];
+        panic_info->u.hyper_v.arg4 = env->msr_hv_crash_params[3];
+        panic_info->u.hyper_v.arg5 = env->msr_hv_crash_params[4];
     }
 
     return panic_info;
diff --git a/vl.c b/vl.c
index b5d0a19..e307ae0 100644
--- a/vl.c
+++ b/vl.c
@@ -1697,14 +1697,14 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
     }
 
     if (info) {
-        if (info->type == GUEST_PANIC_INFORMATION_KIND_HYPER_V) {
+        if (info->type == GUEST_PANIC_INFORMATION_TYPE_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);
+                          info->u.hyper_v.arg1,
+                          info->u.hyper_v.arg2,
+                          info->u.hyper_v.arg3,
+                          info->u.hyper_v.arg4,
+                          info->u.hyper_v.arg5);
         }
         qapi_free_GuestPanicInformation(info);
     }
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2 3/3] i386/cpu: net: Flatten simple union GuestPanicInformationType
Posted by Eric Blake 8 years, 11 months ago
On 02/21/2017 02:46 PM, Markus Armbruster wrote:
> Simple unions are simpler than flat unions in the schema, but more
> complicated in C and on the QMP wire: there's extra indirection in C
> and extra nesting on the wire, both pointless.  They're best avoided
> in new code.  Fix up recent commit d187e08 accordingly.
> 
> Cc: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Cc: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi-schema.json  | 14 ++++++++++++--
>  target/i386/cpu.c | 17 ++++++-----------
>  vl.c              | 12 ++++++------
>  3 files changed, 24 insertions(+), 19 deletions(-)

Very similar to Anton's patch which is on Paolo's queue:
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg04448.html

I don't care which version goes in, but will leave it to Markus and
Paolo to decide which queue it should go through.

Reviewed-by: Eric Blake <eblake@redhat.com>

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