[libvirt] [PATCH libvirt] qemu: log the crash information for S390
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(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9b5ad72c..8c394583 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4363,7 +4363,14 @@ qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info) info->data.hyperv.arg3, info->data.hyperv.arg4, info->data.hyperv.arg5)); break; - + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390: + ignore_value(virAsprintf(&ret, "s390: core='%d' psw-mask='0x%016llx' " + "psw-addr='0x%016llx' reason='%s'", + info->data.s390.core, + info->data.s390.psw_mask, + info->data.s390.psw_addr, + info->data.s390.reason)); + break; case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE: case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST: break; @@ -4379,6 +4386,16 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info) if (!info) return; + switch (info->type) { + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390: + VIR_FREE(info->data.s390.reason); + break; + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE: + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV: + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST: + break; + } + VIR_FREE(info); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index ea0c01ae..4b3a5e21 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -73,6 +73,7 @@ struct _qemuMonitorMessage { typedef enum { QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE = 0, QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV, + QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390, QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST } qemuMonitorEventPanicInfoType; @@ -88,12 +89,23 @@ struct _qemuMonitorEventPanicInfoHyperv { unsigned long long arg5; }; +typedef struct _qemuMonitorEventPanicInfoS390 qemuMonitorEventPanicInfoS390; +typedef qemuMonitorEventPanicInfoS390 *qemuMonitorEventPanicInfoS390Ptr; +struct _qemuMonitorEventPanicInfoS390 { + /* S390 specific guest panic information */ + int core; + unsigned long long psw_mask; + unsigned long long psw_addr; + char *reason; +}; + typedef struct _qemuMonitorEventPanicInfo qemuMonitorEventPanicInfo; typedef qemuMonitorEventPanicInfo *qemuMonitorEventPanicInfoPtr; struct _qemuMonitorEventPanicInfo { qemuMonitorEventPanicInfoType type; union { qemuMonitorEventPanicInfoHyperv hyperv; + qemuMonitorEventPanicInfoS390 s390; } data; }; 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"); + if (!reason) + goto error; + + if (VIR_STRDUP(ret->data.s390.reason, reason) < 0) + goto no_memory; + + return ret; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed s390 panic data")); + no_memory: + qemuMonitorEventPanicInfoFree(ret); + return NULL; +} static qemuMonitorEventPanicInfoPtr qemuMonitorJSONGuestPanicExtractInfo(virJSONValuePtr data) @@ -584,6 +622,8 @@ qemuMonitorJSONGuestPanicExtractInfo(virJSONValuePtr data) if (STREQ_NULLABLE(type, "hyper-v")) return qemuMonitorJSONGuestPanicExtractInfoHyperv(data); + else if (STREQ_NULLABLE(type, "s390")) + return qemuMonitorJSONGuestPanicExtractInfoS390(data); virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown panic info type '%s'"), NULLSTR(type)); -- 2.13.4
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... In any case, is this news.xml worthy?
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9b5ad72c..8c394583 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4363,7 +4363,14 @@ qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info) info->data.hyperv.arg3, info->data.hyperv.arg4, info->data.hyperv.arg5)); break; - + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390: + ignore_value(virAsprintf(&ret, "s390: core='%d' psw-mask='0x%016llx' " + "psw-addr='0x%016llx' reason='%s'", + info->data.s390.core, + info->data.s390.psw_mask, + info->data.s390.psw_addr, + info->data.s390.reason)); + break; case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE: case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST: break; @@ -4379,6 +4386,16 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info) if (!info) return;
+ switch (info->type) { + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390: + VIR_FREE(info->data.s390.reason); + break; + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE: + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV: + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST: + break; + } + VIR_FREE(info); }
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index ea0c01ae..4b3a5e21 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -73,6 +73,7 @@ struct _qemuMonitorMessage { typedef enum { QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE = 0, QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV, + QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390,
QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST } qemuMonitorEventPanicInfoType; @@ -88,12 +89,23 @@ struct _qemuMonitorEventPanicInfoHyperv { unsigned long long arg5; };
+typedef struct _qemuMonitorEventPanicInfoS390 qemuMonitorEventPanicInfoS390; +typedef qemuMonitorEventPanicInfoS390 *qemuMonitorEventPanicInfoS390Ptr; +struct _qemuMonitorEventPanicInfoS390 { + /* S390 specific guest panic information */ + int core; + unsigned long long psw_mask; + unsigned long long psw_addr; + char *reason; +}; + typedef struct _qemuMonitorEventPanicInfo qemuMonitorEventPanicInfo; typedef qemuMonitorEventPanicInfo *qemuMonitorEventPanicInfoPtr; struct _qemuMonitorEventPanicInfo { qemuMonitorEventPanicInfoType type; union { qemuMonitorEventPanicInfoHyperv hyperv; + qemuMonitorEventPanicInfoS390 s390; } data; };
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. 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> John
+ if (!reason) + goto error; + + if (VIR_STRDUP(ret->data.s390.reason, reason) < 0) + goto no_memory; + + return ret; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed s390 panic data")); + no_memory: + qemuMonitorEventPanicInfoFree(ret); + return NULL; +}
static qemuMonitorEventPanicInfoPtr qemuMonitorJSONGuestPanicExtractInfo(virJSONValuePtr data) @@ -584,6 +622,8 @@ qemuMonitorJSONGuestPanicExtractInfo(virJSONValuePtr data)
if (STREQ_NULLABLE(type, "hyper-v")) return qemuMonitorJSONGuestPanicExtractInfoHyperv(data); + else if (STREQ_NULLABLE(type, "s390")) + return qemuMonitorJSONGuestPanicExtractInfoS390(data);
virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown panic info type '%s'"), NULLSTR(type));
On Tue, Feb 20, 2018 at 12:25:46 -0500, John Ferlan wrote:
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...
In any case, is this news.xml worthy?
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9b5ad72c..8c394583 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4363,7 +4363,14 @@ qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info) info->data.hyperv.arg3, info->data.hyperv.arg4, info->data.hyperv.arg5)); break; - + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390: + ignore_value(virAsprintf(&ret, "s390: core='%d' psw-mask='0x%016llx' " + "psw-addr='0x%016llx' reason='%s'", + info->data.s390.core, + info->data.s390.psw_mask, + info->data.s390.psw_addr, + info->data.s390.reason)); + break; case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE: case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST: break; @@ -4379,6 +4386,16 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info) if (!info) return;
+ switch (info->type) { + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390: + VIR_FREE(info->data.s390.reason); + break; + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE: + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV: + case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST: + break; + } + VIR_FREE(info); }
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index ea0c01ae..4b3a5e21 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -73,6 +73,7 @@ struct _qemuMonitorMessage { typedef enum { QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE = 0, QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV, + QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390,
QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST } qemuMonitorEventPanicInfoType; @@ -88,12 +89,23 @@ struct _qemuMonitorEventPanicInfoHyperv { unsigned long long arg5; };
+typedef struct _qemuMonitorEventPanicInfoS390 qemuMonitorEventPanicInfoS390; +typedef qemuMonitorEventPanicInfoS390 *qemuMonitorEventPanicInfoS390Ptr; +struct _qemuMonitorEventPanicInfoS390 { + /* S390 specific guest panic information */ + int core; + unsigned long long psw_mask; + unsigned long long psw_addr; + char *reason; +}; + typedef struct _qemuMonitorEventPanicInfo qemuMonitorEventPanicInfo; typedef qemuMonitorEventPanicInfo *qemuMonitorEventPanicInfoPtr; struct _qemuMonitorEventPanicInfo { qemuMonitorEventPanicInfoType type; union { qemuMonitorEventPanicInfoHyperv hyperv; + qemuMonitorEventPanicInfoS390 s390; } data; };
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 think it should actually be changed that way so that the error message is close to the check rather than being hidden in the error label. Jirka
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
On Mon, Feb 26, 2018 at 08:25:11 +0100, Bjoern Walk wrote:
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?
Sure. The essential part is moving the virReportError(s) closer to the condition which raise them and jump to the "error" label with the error already set. In general, we don't use dedicated labels for each error, we just report the error and jump to a common error label. In other words: if (virJSON...() < 0 || ...) { virReportError("oops"); goto error; } if (somethingElse < 0) { virReportError("oops"); goto error; } if (VIR_STRDUP() < 0) goto error; Jirka
participants (3)
-
Bjoern Walk -
Jiri Denemark -
John Ferlan