On 02/08/2018 03:20 PM, Eric Blake wrote: > On 02/08/2018 03:57 AM, Christian Borntraeger wrote: >> This patch is the s390 implementation of guest crash information, >> similar to commit d187e08dc4 ("i386/cpu: add crash-information QOM >> property") and the related commits. We will detect several crash >> reasons, with the "disabled wait" being the most important one, since >> this is used by all s390 guests as a "panic like" notification. >> > >> Co-authored-by: Jing Liu <liuj...@linux.vnet.ibm.com> >> Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com> >> --- > >> +## >> +# @GuestPanicInformationS390: >> +# >> +# S390 specific guest panic information (PSW) >> +# >> +# @core: core id of the CPU that crashed >> +# @psw-mask: control fields of guest PSW >> +# @psw-addr: guest instruction address >> +# @reason: guest crash reason in human readable form >> +# >> +# Since: 2.12 >> +## >> +{'struct': 'GuestPanicInformationS390', >> + 'data': { 'core': 'uint32', > > Should core be optional,... > > >> +static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs) >> +{ >> + GuestPanicInformation *panic_info; >> + S390CPU *cpu = S390_CPU(cs); >> + >> + cpu_synchronize_state(cs); >> + panic_info = g_malloc0(sizeof(GuestPanicInformation)); >> + >> + panic_info->type = GUEST_PANIC_INFORMATION_TYPE_S390; >> +#if !defined(CONFIG_USER_ONLY) >> + panic_info->u.s390.core = cpu->env.core_id; >> +#endif > > ...given that it is only conditionally assigned? If so, you'd also need to > set panic_info->u.s390.has_core when you have a core id to expose.
So after some irc discussion with Daniel and Erik, I plan to add #else panic_info->u.s390.core = 0; for two reasons - this interface is likely never used for linux-user - linux-user does not know about CPU numbers, so using 0 is a good default an optional item seems like overkill interface-wise.