John Ferlan <jferlan@xxxxxxxxxx> [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@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
