diff mbox

[2/3] qapi: flatten GuestPanicInformation union

Message ID 1487614915-18710-3-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Feb. 20, 2017, 6:21 p.m. UTC 
From: Anton Nefedov <anton.nefedov@virtuozzo.com>

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>
---
 qapi-schema.json  | 12 ++++++++++++
 target/i386/cpu.c | 15 ++++++---------
 vl.c              | 12 ++++++------
 3 files changed, 24 insertions(+), 15 deletions(-)

Comments

Eric Blake Feb. 20, 2017, 7:48 p.m. UTC | #1 
On 02/20/2017 12:21 PM, Denis V. Lunev wrote:
> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> 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>
> ---
>  qapi-schema.json  | 12 ++++++++++++
>  target/i386/cpu.c | 15 ++++++---------
>  vl.c              | 12 ++++++------
>  3 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index e9a6364..b142e15 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -5872,6 +5872,16 @@
>    'data': [ 'pause', 'poweroff' ] }
>  
>  ##
> +# @GuestPanicInformationType:
> +#
> +# An enumeration of the guest panic information types
> +#
> +# Since: 2.9
> +##
> +{ 'enum': 'GuestPanicInformationType',
> +  'data': [ 'hyper-v'] }

Perhaps 'hyperv' is better? It's the difference between
GUEST_PANIC_INFORMATION_TYPE_HYPER_V and
GUEST_PANIC_INFORMATION_TYPE_HYPERV. But that's bikeshedding, so no need
to change it.

Must go into 2.9, so we aren't baking in bad API.

Reviewed-by: Eric Blake <eblake@redhat.com>
Denis V. Lunev Feb. 20, 2017, 7:58 p.m. UTC | #2 
On 02/20/2017 08:48 PM, Eric Blake wrote:
> On 02/20/2017 12:21 PM, Denis V. Lunev wrote:
>> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>
>> 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>
>> ---
>>  qapi-schema.json  | 12 ++++++++++++
>>  target/i386/cpu.c | 15 ++++++---------
>>  vl.c              | 12 ++++++------
>>  3 files changed, 24 insertions(+), 15 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index e9a6364..b142e15 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -5872,6 +5872,16 @@
>>    'data': [ 'pause', 'poweroff' ] }
>>  
>>  ##
>> +# @GuestPanicInformationType:
>> +#
>> +# An enumeration of the guest panic information types
>> +#
>> +# Since: 2.9
>> +##
>> +{ 'enum': 'GuestPanicInformationType',
>> +  'data': [ 'hyper-v'] }
> Perhaps 'hyperv' is better? It's the difference between
> GUEST_PANIC_INFORMATION_TYPE_HYPER_V and
> GUEST_PANIC_INFORMATION_TYPE_HYPERV. But that's bikeshedding, so no need
> to change it.
>
> Must go into 2.9, so we aren't baking in bad API.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
We can you plain 'hv', like all command line options controlling
HyperV features.

Roman, Evgeniy, do you have any opinion on that?

Den
Evgeny Yakovlev Feb. 21, 2017, 8:15 a.m. UTC | #3 
On 20.02.2017 22:58, Denis V. Lunev wrote:
> On 02/20/2017 08:48 PM, Eric Blake wrote:
>> On 02/20/2017 12:21 PM, Denis V. Lunev wrote:
>>> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>>
>>> 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>
>>> ---
>>>   qapi-schema.json  | 12 ++++++++++++
>>>   target/i386/cpu.c | 15 ++++++---------
>>>   vl.c              | 12 ++++++------
>>>   3 files changed, 24 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index e9a6364..b142e15 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -5872,6 +5872,16 @@
>>>     'data': [ 'pause', 'poweroff' ] }
>>>   
>>>   ##
>>> +# @GuestPanicInformationType:
>>> +#
>>> +# An enumeration of the guest panic information types
>>> +#
>>> +# Since: 2.9
>>> +##
>>> +{ 'enum': 'GuestPanicInformationType',
>>> +  'data': [ 'hyper-v'] }
>> Perhaps 'hyperv' is better? It's the difference between
>> GUEST_PANIC_INFORMATION_TYPE_HYPER_V and
>> GUEST_PANIC_INFORMATION_TYPE_HYPERV. But that's bikeshedding, so no need
>> to change it.
>>
>> Must go into 2.9, so we aren't baking in bad API.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
> We can you plain 'hv', like all command line options controlling
> HyperV features.
>
> Roman, Evgeniy, do you have any opinion on that?

In vmbus we've settled on using "hyperv-" prefixes for stuff related to 
QAPI namespace and QOM objects' property names: "hyperv-synic", 
"hyperv-sint-route", etc. We might as well stick to it.

>
> Den
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index e9a6364..b142e15 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5872,6 +5872,16 @@ 
   'data': [ 'pause', 'poweroff' ] }
 
 ##
+# @GuestPanicInformationType:
+#
+# An enumeration of the guest panic information types
+#
+# Since: 2.9
+##
+{ 'enum': 'GuestPanicInformationType',
+  'data': [ 'hyper-v'] }
+
+##
 # @GuestPanicInformation:
 #
 # Information about a guest panic
@@ -5879,6 +5889,8 @@ 
 # Since: 2.9
 ##
 {'union': 'GuestPanicInformation',
+ 'base': {'type': 'GuestPanicInformationType'},
+ 'discriminator': 'type',
  'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
 
 ##
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index fd7add2..63be816 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3502,19 +3502,16 @@  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 81382b6..2781716 100644
--- a/vl.c
+++ b/vl.c
@@ -1682,14 +1682,14 @@  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) {
+    if (info && 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);
     }
 
     if (current_cpu) {