| Message ID | 1487709988-14322-4-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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> writes: > 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 Wasn't cc'ed, so I missed it. No biggie. > I don't care which version goes in, but will leave it to Markus and > Paolo to decide which queue it should go through. My commit message is more verbose, and I fix space style in the QAPI schema. On the other hand, Anton has an extra QAPI schema comment line. Picking Anton's is probably simpler, because it's 2/3, and mine looks like it conflicts with his 1/3. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
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); }
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(-)