[libvirt] [PATCH v2 2/7] qemu: domain: store and update panic info

Bjoern Walk posted 7 patches 6 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH v2 2/7] qemu: domain: store and update panic info
Posted by Bjoern Walk 6 years, 10 months ago
Let's store additional state information in the hypervisor-specific
private data to virDomainObj. For now, just consider panic state in QEMU
domains for which additional information is available from the guest
crash event handler.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
---
 src/qemu/qemu_domain.c | 36 ++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h |  5 +++++
 src/qemu/qemu_driver.c |  2 ++
 3 files changed, 43 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c6e07bcb..fb43035e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2052,6 +2052,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
     virBitmapFree(priv->migrationCaps);
     priv->migrationCaps = NULL;
 
+    qemuDomainStatePanicInfoFree(priv->panicInfo);
+    priv->panicInfo = NULL;
+
     qemuDomainObjResetJob(priv);
     qemuDomainObjResetAsyncJob(priv);
 }
@@ -14069,6 +14072,39 @@ qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info)
 }
 
 
+void
+qemuDomainStatePanicInfoSet(virDomainObjPtr vm,
+                            qemuDomainStatePanicInfoPtr info)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+
+    if (!priv->panicInfo && VIR_ALLOC(priv->panicInfo) < 0)
+        return;
+
+    priv->panicInfo->type = info->type;
+
+    switch (info->type) {
+    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV:
+        priv->panicInfo->data.hyperv.arg1 = info->data.hyperv.arg1;
+        priv->panicInfo->data.hyperv.arg2 = info->data.hyperv.arg2;
+        priv->panicInfo->data.hyperv.arg3 = info->data.hyperv.arg3;
+        priv->panicInfo->data.hyperv.arg4 = info->data.hyperv.arg4;
+        priv->panicInfo->data.hyperv.arg5 = info->data.hyperv.arg5;
+        break;
+    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390:
+        priv->panicInfo->data.s390.core = info->data.s390.core;
+        priv->panicInfo->data.s390.psw_mask = info->data.s390.psw_mask;
+        priv->panicInfo->data.s390.psw_addr = info->data.s390.psw_addr;
+        ignore_value(VIR_STRDUP(priv->panicInfo->data.s390.reason,
+                                info->data.s390.reason));
+        break;
+    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_NONE:
+    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST:
+        break;
+    }
+}
+
+
 void
 qemuDomainStatePanicInfoFree(qemuDomainStatePanicInfoPtr info)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 145b377e..aa7457b1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -308,6 +308,8 @@ struct _qemuDomainStatePanicInfo {
 };
 
 char *qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info);
+void qemuDomainStatePanicInfoSet(virDomainObjPtr vm,
+                                 qemuDomainStatePanicInfoPtr info);
 void qemuDomainStatePanicInfoFree(qemuDomainStatePanicInfoPtr info);
 
 typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
@@ -417,6 +419,9 @@ struct _qemuDomainObjPrivate {
 
     /* true if global -mem-prealloc appears on cmd line */
     bool memPrealloc;
+
+    /* store extra information for panicked domain state */
+    qemuDomainStatePanicInfoPtr panicInfo;
 };
 
 # define QEMU_DOMAIN_PRIVATE(vm) \
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 808d66cd..16e34a98 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4171,6 +4171,8 @@ qemuProcessGuestPanicEventInfo(virQEMUDriverPtr driver,
     if (msg && timestamp)
         qemuDomainLogAppendMessage(driver, vm, "%s: panic %s\n", timestamp, msg);
 
+    qemuDomainStatePanicInfoSet(vm, info);
+
     VIR_FREE(timestamp);
     VIR_FREE(msg);
 }
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/7] qemu: domain: store and update panic info
Posted by Michal Privoznik 6 years, 10 months ago
On 3/25/19 9:04 AM, Bjoern Walk wrote:
> Let's store additional state information in the hypervisor-specific
> private data to virDomainObj. For now, just consider panic state in QEMU
> domains for which additional information is available from the guest
> crash event handler.
> 
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
> ---
>   src/qemu/qemu_domain.c | 36 ++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_domain.h |  5 +++++
>   src/qemu/qemu_driver.c |  2 ++
>   3 files changed, 43 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c6e07bcb..fb43035e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2052,6 +2052,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
>       virBitmapFree(priv->migrationCaps);
>       priv->migrationCaps = NULL;
>   
> +    qemuDomainStatePanicInfoFree(priv->panicInfo);
> +    priv->panicInfo = NULL;
> +
>       qemuDomainObjResetJob(priv);
>       qemuDomainObjResetAsyncJob(priv);
>   }
> @@ -14069,6 +14072,39 @@ qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info)
>   }
>   
>   
> +void
> +qemuDomainStatePanicInfoSet(virDomainObjPtr vm,
> +                            qemuDomainStatePanicInfoPtr info)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +    if (!priv->panicInfo && VIR_ALLOC(priv->panicInfo) < 0)
> +        return;
> +
> +    priv->panicInfo->type = info->type;
> +
> +    switch (info->type) {
> +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV:
> +        priv->panicInfo->data.hyperv.arg1 = info->data.hyperv.arg1;
> +        priv->panicInfo->data.hyperv.arg2 = info->data.hyperv.arg2;
> +        priv->panicInfo->data.hyperv.arg3 = info->data.hyperv.arg3;
> +        priv->panicInfo->data.hyperv.arg4 = info->data.hyperv.arg4;
> +        priv->panicInfo->data.hyperv.arg5 = info->data.hyperv.arg5;
> +        break;
> +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390:
> +        priv->panicInfo->data.s390.core = info->data.s390.core;
> +        priv->panicInfo->data.s390.psw_mask = info->data.s390.psw_mask;
> +        priv->panicInfo->data.s390.psw_addr = info->data.s390.psw_addr;
> +        ignore_value(VIR_STRDUP(priv->panicInfo->data.s390.reason,
> +                                info->data.s390.reason));
> +        break;
> +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_NONE:
> +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST:
> +        break;
> +    }

How about memcpy() and then just STRDUP() in the one place we need to?

No need to send v3 for that, just asking and if so, I can fix that 
before pushing.

My reasoning is that it might be easy to forget update this function if 
qemuDomainStatePanicInfo struct gets a new member.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/7] qemu: domain: store and update panic info
Posted by Bjoern Walk 6 years, 10 months ago
Michal Privoznik <mprivozn@redhat.com> [2019-03-27, 02:05PM +0100]:
> On 3/25/19 9:04 AM, Bjoern Walk wrote:
> > @@ -14069,6 +14072,39 @@ qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info)
> >   }
> > +void
> > +qemuDomainStatePanicInfoSet(virDomainObjPtr vm,
> > +                            qemuDomainStatePanicInfoPtr info)
> > +{
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> > +
> > +    if (!priv->panicInfo && VIR_ALLOC(priv->panicInfo) < 0)
> > +        return;
> > +
> > +    priv->panicInfo->type = info->type;
> > +
> > +    switch (info->type) {
> > +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV:
> > +        priv->panicInfo->data.hyperv.arg1 = info->data.hyperv.arg1;
> > +        priv->panicInfo->data.hyperv.arg2 = info->data.hyperv.arg2;
> > +        priv->panicInfo->data.hyperv.arg3 = info->data.hyperv.arg3;
> > +        priv->panicInfo->data.hyperv.arg4 = info->data.hyperv.arg4;
> > +        priv->panicInfo->data.hyperv.arg5 = info->data.hyperv.arg5;
> > +        break;
> > +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390:
> > +        priv->panicInfo->data.s390.core = info->data.s390.core;
> > +        priv->panicInfo->data.s390.psw_mask = info->data.s390.psw_mask;
> > +        priv->panicInfo->data.s390.psw_addr = info->data.s390.psw_addr;
> > +        ignore_value(VIR_STRDUP(priv->panicInfo->data.s390.reason,
> > +                                info->data.s390.reason));
> > +        break;
> > +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_NONE:
> > +    case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST:
> > +        break;
> > +    }
> 
> How about memcpy() and then just STRDUP() in the one place we need to?
> 
> No need to send v3 for that, just asking and if so, I can fix that before
> pushing.
> 
> My reasoning is that it might be easy to forget update this function if
> qemuDomainStatePanicInfo struct gets a new member.

Yes, I agree, that's much better.

> 
> Michal
> 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list