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

[John Ferlan]

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.

Need to wait until patch shows up in qemu master, but otherwise, things
seem fine for what I see...

Reviewed-by: John Ferlan <jferlan at 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));
>