[libvirt] [PATCH 4/4] qemu: Log additional data from hyperv crash notifier

[Jiri Denemark]

On Tue, Mar 21, 2017 at 08:47:50 +0100, Peter Krempa wrote:
> The hyperv panic notifier reports additional data in form of 5 registers
> that are reported in the crash event from qemu. Log them into the VM log
> file and report them as a warning so that admins can see the cause of
> crash of their windows VMs.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1426176
> ---
>  src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ab7c01b23..c558e0991 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3971,10 +3971,56 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver,
>      return ret;
>  }
> 
> +
> +static char *
> +qemuProcessGuestPanicEventInfoFormatData(qemuMonitorEventPanicInfoPtr info)

I think this should be implemented in qemu_monitor.c as
qemuMonitorEventPanicInfoFormat so that it's closer the structure is
defined.

> +{
> +    char *ret = NULL;
> +
> +    switch (info->type) {
> +    case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV:
> +        ignore_value(virAsprintf(&ret, "hyper-v: arg1='0x%llx', arg2='0x%llx', "
> +                                       "arg3='0x%llx', arg4='0x%llx', "
> +                                       "arg5='0x%llx'",
> +                                       info->data.hyperv.arg1,
> +                                       info->data.hyperv.arg2,
> +                                       info->data.hyperv.arg3,
> +                                       info->data.hyperv.arg4,
> +                                       info->data.hyperv.arg5));

Wrong indentation, the additional lines should be aligned with &ret :-)

> +        break;
> +
> +    case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_NONE:
> +    case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_LAST:
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +
> +static void
> +qemuProcessGuestPanicEventInfo(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               qemuMonitorEventPanicInfoPtr info)
> +{
> +    char *msg = qemuProcessGuestPanicEventInfoFormatData(info);
> +    char *timestamp = virTimeStringNow();
> +
> +    if (msg && timestamp) {
> +        qemuDomainLogAppendMessage(driver, vm, "%s: panic %s\n", timestamp, msg);
> +        VIR_WARN("domain '%s' panic data: %s", vm->def->name, msg);

Do we really need this warning? Especially when a guest panic triggers
the warning only if we have the additional data to report. We should
either emit the warning in all cases or never. I think we should just
drop the warning.

Jirka