John Ferlan <jferlan@redhat.com> [2018-02-20, 12:25PM -0500]:
On 02/15/2018 06:58 AM, Bjoern Walk wrote:
Since QEMU 2.12 guest crash information for S390 is available in the QEMU monitor, e.g.:
{ "timestamp": { "seconds": 1518004739, "microseconds": 552563 }, "event": "GUEST_PANICKED", "data": { "action": "pause", "info": { "core": 0, "psw-addr": 1102832, "reason": "disabled-wait", "psw-mask": 562956395872256, "type": "s390" } } }
Let's log this information into the domain log file, e.g.:
2018-02-08 13:11:26.075+0000: panic s390: core='0' psw-mask='0x0002000180000000' psw-addr='0x000000000010f146' reason='disabled-wait'
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 19 ++++++++++++++++++- src/qemu/qemu_monitor.h | 12 ++++++++++++ src/qemu/qemu_monitor_json.c | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-)
Not sure the patch :
http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg02580.html
has quite made it to git master yet... At least the pull I just didn't have it...
Yeah, I sent it early because of vacation.
In any case, is this news.xml worthy?
I don't know, probably. Still haven't figured out when to write an entry there.
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 242b92ea..5c1f6836 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -576,6 +576,44 @@ qemuMonitorJSONGuestPanicExtractInfoHyperv(virJSONValuePtr data) return NULL; }
+static qemuMonitorEventPanicInfoPtr +qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValuePtr data) +{ + qemuMonitorEventPanicInfoPtr ret; + int core; + unsigned long long psw_mask, psw_addr; + const char *reason = NULL; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + ret->type = QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390; + + if (virJSONValueObjectGetNumberInt(data, "core", &core) < 0 || + virJSONValueObjectGetNumberUlong(data, "psw-mask", &psw_mask) < 0 || + virJSONValueObjectGetNumberUlong(data, "psw-addr", &psw_addr) < 0) { + goto error; + } + + ret->data.s390.core = core; + ret->data.s390.psw_mask = psw_mask; + ret->data.s390.psw_addr = psw_addr; + + reason = virJSONValueObjectGetString(data, "reason");
Probably could have gone with including this into the above condition,
!(reason = virJSONValueObjectGetString(data, "reason"))
so that the error could be put in there to avoid the no_memory label..
This isn't a necessary change, but a "could be done" change if so desired.
I imagine having read somewhere here that we do want do discourage those assign-conditionals. But still, yes, this can be reordered to make it more compact and readable. I however don't see how we avoid the no_memory label, since we do the VIR_STRDUP, which is fundamentally different from the error when the QMP response is different. Any pointers?
Need to wait until patch shows up in qemu master, but otherwise, things seem fine for what I see...
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks. While we wait for QEMU I can spin up a v2 with the news.xml and the above. -- IBM Systems Linux on Z & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 ------------------------------------------------------------------------ Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294