[libvirt] [PATCH libvirt] qemu: log the crash information for S390

[Jiri Denemark]

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 at linux.vnet.ibm.com>
> > Signed-off-by: Bjoern Walk <bwalk at 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