[libvirt] [PATCH v3 0/7] extend virsh domstate to show additional information
This patch series introduces the ability to save additional information for the domain state and exposes this information in virsh domstate. For example in the case of QEMU guest panic events, we can provide additional information like the crash reason or register state of the domain. This information usually gets logged in the domain log but for debugging it is useful to have it accessible from the client. Therefore, let's introduce a new public API function, virDomainGetStateParams, an extensible version of virDomainGetState, which returns the complete state of the domain, including newly introduced additional information. Let's also extend virsh domstate and introduce a new parameter --info to show the domain state, reason and additional information when available. virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait v2 -> v3: * try to find a reasonable naming-scheme for parameters * make state/reason in virDomainGetStateParams optional * use new API only when requested * print additional information per line for easier consumption v1 -> v2: * refactored the public API according to discussions to provide a more machine-parsable interface Bjoern Walk (7): qemu: monitor: move event data structure to domain qemu: domain: store and update panic info lib: introduce virDomainGetStateParams function remote: implement remoteDomainGetStateParams qemu: implement qemuDomainGetStateParams virsh: domstate: report detailed state if available news: add entry for virDomainGetStateParams docs/news.xml | 11 +++ include/libvirt/libvirt-domain.h | 76 +++++++++++++++++ src/driver-hypervisor.h | 9 ++ src/libvirt-domain.c | 62 ++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_domain.c | 80 +++++++++++++++++- src/qemu/qemu_domain.h | 47 +++++++++++ src/qemu/qemu_driver.c | 126 +++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 53 +----------- src/qemu/qemu_monitor.h | 48 ++--------- src/qemu/qemu_monitor_json.c | 20 ++--- src/qemu/qemu_process.c | 2 +- src/remote/remote_daemon_dispatch.c | 50 +++++++++++ src/remote/remote_driver.c | 48 +++++++++++ src/remote/remote_protocol.x | 22 ++++- src/remote_protocol-structs | 13 +++ tools/virsh-domain-monitor.c | 94 +++++++++++++++++++-- tools/virsh.pod | 5 +- 18 files changed, 651 insertions(+), 120 deletions(-) -- 2.19.1
In order to later process the event data, we need to be able to access it over the lifetime of the domain. So, as an initial step, let's move the event data structure and utility functions to qemu_domain.[ch] and rename them appropriately to make it accessible for other users. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/qemu/qemu_domain.c | 53 +++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 42 ++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 6 ++-- src/qemu/qemu_monitor.c | 53 +----------------------------------- src/qemu/qemu_monitor.h | 48 ++++---------------------------- src/qemu/qemu_monitor_json.c | 20 +++++++------- src/qemu/qemu_process.c | 2 +- 7 files changed, 114 insertions(+), 110 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fab237d0..cfd88aff 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -13859,7 +13859,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) switch (event->eventType) { case QEMU_PROCESS_EVENT_GUESTPANIC: - qemuMonitorEventPanicInfoFree(event->data); + qemuDomainStatePanicInfoFree(event->data); break; case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED: qemuMonitorEventRdmaGidStatusFree(event->data); @@ -13988,3 +13988,54 @@ qemuDomainNVRAMPathGenerate(virQEMUDriverConfigPtr cfg, return 0; } + + +char * +qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info) +{ + char *ret = NULL; + + switch (info->type) { + case QEMU_DOMAIN_STATE_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)); + break; + case QEMU_DOMAIN_STATE_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_DOMAIN_STATE_PANIC_INFO_TYPE_NONE: + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST: + break; + } + + return ret; +} + + +void +qemuDomainStatePanicInfoFree(qemuDomainStatePanicInfoPtr info) +{ + if (!info) + return; + + switch (info->type) { + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390: + VIR_FREE(info->data.s390.reason); + break; + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_NONE: + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV: + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST: + break; + } + + VIR_FREE(info); +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 742632f6..df2a08f0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -269,6 +269,48 @@ struct _qemuDomainSecretInfo { } s; }; +typedef enum { + QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_NONE = 0, + QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV, + QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390, + + QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST +} qemuDomainStatePanicInfoType; + +typedef struct _qemuDomainStatePanicInfoHyperv qemuDomainStatePanicInfoHyperv; +typedef qemuDomainStatePanicInfoHyperv *qemuDomainStatePanicInfoHypervPtr; +struct _qemuDomainStatePanicInfoHyperv { + /* Hyper-V specific guest panic information (HV crash MSRs) */ + unsigned long long arg1; + unsigned long long arg2; + unsigned long long arg3; + unsigned long long arg4; + unsigned long long arg5; +}; + +typedef struct _qemuDomainStatePanicInfoS390 qemuDomainStatePanicInfoS390; +typedef qemuDomainStatePanicInfoS390 *qemuDomainStatePanicInfoS390Ptr; +struct _qemuDomainStatePanicInfoS390 { + /* S390 specific guest panic information */ + int core; + unsigned long long psw_mask; + unsigned long long psw_addr; + char *reason; +}; + +typedef struct _qemuDomainStatePanicInfo qemuDomainStatePanicInfo; +typedef qemuDomainStatePanicInfo *qemuDomainStatePanicInfoPtr; +struct _qemuDomainStatePanicInfo { + qemuDomainStatePanicInfoType type; + union { + qemuDomainStatePanicInfoHyperv hyperv; + qemuDomainStatePanicInfoS390 s390; + } data; +}; + +char *qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info); +void qemuDomainStatePanicInfoFree(qemuDomainStatePanicInfoPtr info); + typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr; struct _qemuDomainObjPrivate { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 85a93f9e..b6c9f7b8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4163,9 +4163,9 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, static void qemuProcessGuestPanicEventInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuMonitorEventPanicInfoPtr info) + qemuDomainStatePanicInfoPtr info) { - char *msg = qemuMonitorGuestPanicEventInfoFormatMsg(info); + char *msg = qemuDomainStatePanicInfoFormatMsg(info); char *timestamp = virTimeStringNow(); if (msg && timestamp) @@ -4180,7 +4180,7 @@ static void processGuestPanicEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, int action, - qemuMonitorEventPanicInfoPtr info) + qemuDomainStatePanicInfoPtr info) { qemuDomainObjPrivatePtr priv = vm->privateData; virObjectEventPtr event = NULL; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index babcbde8..38da68e4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1389,7 +1389,7 @@ qemuMonitorEmitResume(qemuMonitorPtr mon) int qemuMonitorEmitGuestPanic(qemuMonitorPtr mon, - qemuMonitorEventPanicInfoPtr info) + qemuDomainStatePanicInfoPtr info) { int ret = -1; VIR_DEBUG("mon=%p", mon); @@ -4280,57 +4280,6 @@ qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon) } -char * -qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info) -{ - 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)); - 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; - } - - return ret; -} - - -void -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); -} - - void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusPtr info) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 086195ff..7b675835 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -39,6 +39,9 @@ typedef qemuMonitor *qemuMonitorPtr; typedef struct _qemuMonitorMessage qemuMonitorMessage; typedef qemuMonitorMessage *qemuMonitorMessagePtr; +typedef struct _qemuDomainStatePanicInfo qemuDomainStatePanicInfo; +typedef qemuDomainStatePanicInfo *qemuDomainStatePanicInfoPtr; + typedef int (*qemuMonitorPasswordHandler)(qemuMonitorPtr mon, qemuMonitorMessagePtr msg, const char *data, @@ -67,45 +70,6 @@ struct _qemuMonitorMessage { void *passwordOpaque; }; -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; - -typedef struct _qemuMonitorEventPanicInfoHyperv qemuMonitorEventPanicInfoHyperv; -typedef qemuMonitorEventPanicInfoHyperv *qemuMonitorEventPanicInfoHypervPtr; -struct _qemuMonitorEventPanicInfoHyperv { - /* Hyper-V specific guest panic information (HV crash MSRs) */ - unsigned long long arg1; - unsigned long long arg2; - unsigned long long arg3; - unsigned long long arg4; - 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; -}; - typedef struct _qemuMonitorRdmaGidStatus qemuMonitorRdmaGidStatus; typedef qemuMonitorRdmaGidStatus *qemuMonitorRdmaGidStatusPtr; @@ -117,8 +81,6 @@ struct _qemuMonitorRdmaGidStatus { }; -char *qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info); -void qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info); void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusPtr info); typedef void (*qemuMonitorDestroyCallback)(qemuMonitorPtr mon, @@ -209,7 +171,7 @@ typedef int (*qemuMonitorDomainPMSuspendDiskCallback)(qemuMonitorPtr mon, void *opaque); typedef int (*qemuMonitorDomainGuestPanicCallback)(qemuMonitorPtr mon, virDomainObjPtr vm, - qemuMonitorEventPanicInfoPtr info, + qemuDomainStatePanicInfoPtr info, void *opaque); typedef int (*qemuMonitorDomainDeviceDeletedCallback)(qemuMonitorPtr mon, virDomainObjPtr vm, @@ -431,7 +393,7 @@ int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, unsigned long long actual); int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon); int qemuMonitorEmitGuestPanic(qemuMonitorPtr mon, - qemuMonitorEventPanicInfoPtr info); + qemuDomainStatePanicInfoPtr info); int qemuMonitorEmitDeviceDeleted(qemuMonitorPtr mon, const char *devAlias); int qemuMonitorEmitNicRxFilterChanged(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8e6c3ccd..10b00767 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -629,15 +629,15 @@ static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr data } -static qemuMonitorEventPanicInfoPtr +static qemuDomainStatePanicInfoPtr qemuMonitorJSONGuestPanicExtractInfoHyperv(virJSONValuePtr data) { - qemuMonitorEventPanicInfoPtr ret; + qemuDomainStatePanicInfoPtr ret; if (VIR_ALLOC(ret) < 0) return NULL; - ret->type = QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV; + ret->type = QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV; if (virJSONValueObjectGetNumberUlong(data, "arg1", &ret->data.hyperv.arg1) < 0 || virJSONValueObjectGetNumberUlong(data, "arg2", &ret->data.hyperv.arg2) < 0 || @@ -652,14 +652,14 @@ qemuMonitorJSONGuestPanicExtractInfoHyperv(virJSONValuePtr data) return ret; error: - qemuMonitorEventPanicInfoFree(ret); + qemuDomainStatePanicInfoFree(ret); return NULL; } -static qemuMonitorEventPanicInfoPtr +static qemuDomainStatePanicInfoPtr qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValuePtr data) { - qemuMonitorEventPanicInfoPtr ret; + qemuDomainStatePanicInfoPtr ret; int core; unsigned long long psw_mask, psw_addr; const char *reason = NULL; @@ -667,7 +667,7 @@ qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValuePtr data) if (VIR_ALLOC(ret) < 0) return NULL; - ret->type = QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_S390; + ret->type = QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390; if (virJSONValueObjectGetNumberInt(data, "core", &core) < 0 || virJSONValueObjectGetNumberUlong(data, "psw-mask", &psw_mask) < 0 || @@ -687,11 +687,11 @@ qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValuePtr data) return ret; error: - qemuMonitorEventPanicInfoFree(ret); + qemuDomainStatePanicInfoFree(ret); return NULL; } -static qemuMonitorEventPanicInfoPtr +static qemuDomainStatePanicInfoPtr qemuMonitorJSONGuestPanicExtractInfo(virJSONValuePtr data) { const char *type = virJSONValueObjectGetString(data, "type"); @@ -712,7 +712,7 @@ qemuMonitorJSONHandleGuestPanic(qemuMonitorPtr mon, virJSONValuePtr data) { virJSONValuePtr infojson = virJSONValueObjectGetObject(data, "info"); - qemuMonitorEventPanicInfoPtr info = NULL; + qemuDomainStatePanicInfoPtr info = NULL; if (infojson) info = qemuMonitorJSONGuestPanicExtractInfo(infojson); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index dc7317b7..aec4af8f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1259,7 +1259,7 @@ qemuProcessHandlePMSuspendDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, static int qemuProcessHandleGuestPanic(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm, - qemuMonitorEventPanicInfoPtr info, + qemuDomainStatePanicInfoPtr info, void *opaque) { virQEMUDriverPtr driver = opaque; -- 2.19.1
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 | 27 +++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 +++++ src/qemu/qemu_driver.c | 2 ++ 3 files changed, 34 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cfd88aff..6949628d 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); } @@ -14021,6 +14024,30 @@ qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info) } +void +qemuDomainStatePanicInfoSet(virDomainObjPtr vm, + qemuDomainStatePanicInfoPtr info) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!priv->panicInfo && VIR_ALLOC(priv->panicInfo) < 0) + return; + + memcpy(priv->panicInfo, info, sizeof(*info)); + + switch (info->type) { + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390: + 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_HYPERV: + 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 df2a08f0..31de8ee1 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -309,6 +309,8 @@ struct _qemuDomainStatePanicInfo { }; char *qemuDomainStatePanicInfoFormatMsg(qemuDomainStatePanicInfoPtr info); +void qemuDomainStatePanicInfoSet(virDomainObjPtr vm, + qemuDomainStatePanicInfoPtr info); void qemuDomainStatePanicInfoFree(qemuDomainStatePanicInfoPtr info); typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; @@ -418,6 +420,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 b6c9f7b8..b9d1aaa1 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.19.1
This API function extends the virDomainGetState function by returning additional state information as a dictionary of typed parameters. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- include/libvirt/libvirt-domain.h | 76 ++++++++++++++++++++++++++++++++ src/driver-hypervisor.h | 9 ++++ src/libvirt-domain.c | 62 ++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 4 files changed, 152 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7d36820b..b0118e4a 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1292,6 +1292,82 @@ int virDomainGetState (virDomainPtr domain, int *reason, unsigned int flags); +typedef enum { + VIR_DOMAIN_STATE_PARAM_TYPE_NONE, + VIR_DOMAIN_STATE_PARAM_TYPE_QEMU_HYPERV, + VIR_DOMAIN_STATE_PARAM_TYPE_QEMU_S390, + + VIR_DOMAIN_STATE_PARAM_TYPE_LAST +} virDomainStateParamType; + +/** + * VIR_DOMAIN_STATE_PARAM_TYPE: + * specifies the type of additional state information available + */ +# define VIR_DOMAIN_STATE_PARAM_TYPE "info_type" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG1: + * Hyper-V-specific panic state information: HV crash MSR1 + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG1 "crashed.panicked.hyperv.arg1" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG2: + * Hyper-V-specific panic state information: HV crash MSR2 + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG2 "crashed.panicked.hyperv.arg2" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG3: + * Hyper-V-specific panic state information: HV crash MSR3 + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG3 "crashed.panicked.hyperv.arg3" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG4: + * Hyper-V-specific panic state information: HV crash MSR4 + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG4 "crashed.panicked.hyperv.arg4" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG5: + * Hyper-V-specific panic state information: HV crash MSR5 + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG5 "crashed.panicked.hyperv.arg5" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_CORE: + * S390-specific panic state information: panicked core + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_CORE "crashed.panicked.s390.core" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_MASK: + * S390-specific panic state information: PSW mask + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_MASK "crashed.panicked.s390.psw_mask" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_ADDR: + * S390-specific panic state information: PSW address + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_ADDR "crashed.panicked.s390.psw_addr" + +/** + * VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_REASON: + * S390-specific panic state information: human-readable reason + */ +# define VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_REASON "crashed.panicked.s390.reason" + +int virDomainGetStateParams(virDomainPtr domain, + int *state, + int *reason, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + + /** * VIR_DOMAIN_CPU_STATS_CPUTIME: * cpu usage (sum of both vcpu and hypervisor usage) in nanoseconds, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 5315e33d..fd6ed690 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -220,6 +220,14 @@ typedef int int *reason, unsigned int flags); +typedef int +(*virDrvDomainGetStateParams)(virDomainPtr domain, + int *state, + int *reason, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + typedef int (*virDrvDomainGetControlInfo)(virDomainPtr domain, virDomainControlInfoPtr info, @@ -1390,6 +1398,7 @@ struct _virHypervisorDriver { virDrvDomainGetBlkioParameters domainGetBlkioParameters; virDrvDomainGetInfo domainGetInfo; virDrvDomainGetState domainGetState; + virDrvDomainGetStateParams domainGetStateParams; virDrvDomainGetControlInfo domainGetControlInfo; virDrvDomainSave domainSave; virDrvDomainSaveFlags domainSaveFlags; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index be5b1f67..4fabae82 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -2502,6 +2502,68 @@ virDomainGetState(virDomainPtr domain, } +/** + * virDomainGetStateParams: + * @domain: a domain object + * @state: returned state of the domain (one of virDomainState) + * @reason: returned reason which led to @state (one of virDomain*Reason) + * @params: where to store additional state information, must be freed by + * the caller + * @nparams: number of items in @params + * @flags: bitwise-OR of virDomainGetStateFlags, + * not currently used yet, callers should always pass 0 + * + * Extract domain state. Each state is accompanied by a reason (if known) + * and optional detailed information. + * + * Possible fields returned in @params are defined by VIR_DOMAIN_STATE_PARAMS_* + * macros and new fields will likely be introduced in the future so callers + * may receive fields that they do not understand in case they talk to a newer + * server. The caller is responsible to free allocated memory returned in + * @params by calling virTypedParamsFree. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainGetStateParams(virDomainPtr domain, + int *state, + int *reason, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn = domain->conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p, flags=0x%x", + params, nparams, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + if (params) + virCheckNonNullArgGoto(nparams, error); + + if (VIR_DRV_SUPPORTS_FEATURE(conn->driver, conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + if (conn->driver->domainGetStateParams) { + int ret; + ret = conn->driver->domainGetStateParams(domain, state, reason, + params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + /** * virDomainGetControlInfo: * @domain: a domain object diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index dbce3336..26744e27 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -819,4 +819,9 @@ LIBVIRT_5.2.0 { virConnectGetStoragePoolCapabilities; } LIBVIRT_4.10.0; +LIBVIRT_5.3.0 { + global: + virDomainGetStateParams; +} LIBVIRT_5.2.0; + # .... define new API here using predicted next version number .... -- 2.19.1
On Thu, Apr 04, 2019 at 10:01:30AM +0200, Bjoern Walk wrote:
This API function extends the virDomainGetState function by returning additional state information as a dictionary of typed parameters.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- include/libvirt/libvirt-domain.h | 76 ++++++++++++++++++++++++++++++++ src/driver-hypervisor.h | 9 ++++ src/libvirt-domain.c | 62 ++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 4 files changed, 152 insertions(+) +/** + * virDomainGetStateParams: + * @domain: a domain object + * @state: returned state of the domain (one of virDomainState) + * @reason: returned reason which led to @state (one of virDomain*Reason) + * @params: where to store additional state information, must be freed by + * the caller + * @nparams: number of items in @params + * @flags: bitwise-OR of virDomainGetStateFlags, + * not currently used yet, callers should always pass 0 + * + * Extract domain state. Each state is accompanied by a reason (if known) + * and optional detailed information. + * + * Possible fields returned in @params are defined by VIR_DOMAIN_STATE_PARAMS_* + * macros and new fields will likely be introduced in the future so callers + * may receive fields that they do not understand in case they talk to a newer + * server. The caller is responsible to free allocated memory returned in + * @params by calling virTypedParamsFree. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainGetStateParams(virDomainPtr domain, + int *state, + int *reason, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn = domain->conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p, flags=0x%x", + params, nparams, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + if (params) + virCheckNonNullArgGoto(nparams, error);
While I remember, we must forbid this for "read only" connections, as some of the parameters are CPU registers which may hold sensitive data.
+ + if (VIR_DRV_SUPPORTS_FEATURE(conn->driver, conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + if (conn->driver->domainGetStateParams) { + int ret; + ret = conn->driver->domainGetStateParams(domain, state, reason, + params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} +
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> [2019-04-04, 11:41AM +0100]:
On Thu, Apr 04, 2019 at 10:01:30AM +0200, Bjoern Walk wrote:
+ virCheckDomainReturn(domain, -1); + if (params) + virCheckNonNullArgGoto(nparams, error);
While I remember, we must forbid this for "read only" connections, as some of the parameters are CPU registers which may hold sensitive data.
Sure, done. -- 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: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Implement the API function virDomainGetStateParams for the remote driver and wire up the remote protocol. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/remote/remote_daemon_dispatch.c | 50 +++++++++++++++++++++++++++++ src/remote/remote_driver.c | 48 +++++++++++++++++++++++++++ src/remote/remote_protocol.x | 22 ++++++++++++- src/remote_protocol-structs | 13 ++++++++ 4 files changed, 132 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index df282590..e1e48559 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -7392,3 +7392,53 @@ remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors, } return -1; } + +static int +remoteDispatchDomainGetStateParams(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_state_params_args *args, + remote_domain_get_state_params_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetStateParams(dom, &ret->state, &ret->reason, ¶ms, + &nparams, args->flags) < 0) + goto cleanup; + + if (nparams > REMOTE_DOMAIN_STATE_PARAMS_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many state information entries '%d' for limit '%d'"), + nparams, REMOTE_DOMAIN_STATE_PARAMS_MAX); + goto cleanup; + } + + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virTypedParamsFree(params, nparams); + virObjectUnref(dom); + return rv; +} diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5c4dd412..6907e457 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8131,6 +8131,53 @@ remoteStorageVolGetInfoFlags(virStorageVolPtr vol, return rv; } +static int +remoteDomainGetStateParams(virDomainPtr domain, + int *state, + int *reason, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_state_params_args args; + remote_domain_get_state_params_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_STATE_PARAMS, + (xdrproc_t) xdr_remote_domain_get_state_params_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_state_params_ret, (char *) &ret) == -1) + goto done; + + if (state) + *state = ret.state; + if (reason) + *reason = ret.reason; + + if (params && + virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_STATE_PARAMS_MAX, + params, nparams) < 0) { + goto cleanup; + } + + rv = 0; + + cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_state_params_ret, + (char *) &ret); + done: + remoteDriverUnlock(priv); + return rv; +} + /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. @@ -8326,6 +8373,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSetPerfEvents = remoteDomainSetPerfEvents, /* 1.3.3 */ .domainGetInfo = remoteDomainGetInfo, /* 0.3.0 */ .domainGetState = remoteDomainGetState, /* 0.9.2 */ + .domainGetStateParams = remoteDomainGetStateParams, /* 5.3.0 */ .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */ .domainSave = remoteDomainSave, /* 0.3.0 */ .domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 74be4b37..6db1ccc1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -263,6 +263,9 @@ const REMOTE_NODE_SEV_INFO_MAX = 64; /* Upper limit on number of launch security information entries */ const REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX = 64; +/* Upper limit on number of state information entries */ +const REMOTE_DOMAIN_STATE_PARAMS_MAX = 16; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2789,6 +2792,17 @@ struct remote_domain_get_state_ret { int reason; }; +struct remote_domain_get_state_params_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_state_params_ret { + int state; + int reason; + remote_typed_param params<REMOTE_DOMAIN_STATE_PARAMS_MAX>; +}; + struct remote_domain_migrate_begin3_args { remote_nonnull_domain dom; remote_string xmlin; @@ -6342,5 +6356,11 @@ enum remote_procedure { * @generate: both * @acl: connect:read */ - REMOTE_PROC_CONNECT_GET_STORAGE_POOL_CAPABILITIES = 403 + REMOTE_PROC_CONNECT_GET_STORAGE_POOL_CAPABILITIES = 403, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_STATE_PARAMS = 404 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 768189c5..edecc4e4 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2162,6 +2162,18 @@ struct remote_domain_get_state_ret { int state; int reason; }; +struct remote_domain_get_state_params_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_state_params_ret { + int state; + int reason; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; struct remote_domain_migrate_begin3_args { remote_nonnull_domain dom; remote_string xmlin; @@ -3385,4 +3397,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NWFILTER_BINDINGS = 401, REMOTE_PROC_DOMAIN_SET_IOTHREAD_PARAMS = 402, REMOTE_PROC_CONNECT_GET_STORAGE_POOL_CAPABILITIES = 403, + REMOTE_PROC_DOMAIN_GET_STATE_PARAMS = 404, }; -- 2.19.1
Implement the API function virDomainGetStateParams for the QEMU hypervisor driver. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- src/qemu/qemu_driver.c | 118 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b9d1aaa1..864579da 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2641,6 +2641,123 @@ qemuDomainGetState(virDomainPtr dom, return ret; } +static int +qemuDomainGetStateParams(virDomainPtr dom, + int *state, + int *reason, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int ret = -1; + virDomainObjPtr vm; + qemuDomainStatePanicInfoPtr info = NULL; + virTypedParameterPtr par = NULL; + int npar = 0, maxparams = 0; + + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainGetStateParamsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + *state = virDomainObjGetState(vm, reason); + + info = QEMU_DOMAIN_PRIVATE(vm)->panicInfo; + + if (!info) { + if (virTypedParamsAddInt(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_TYPE, + VIR_DOMAIN_STATE_PARAM_TYPE_NONE) < 0) { + goto cleanup; + } + } else if (*state == VIR_DOMAIN_CRASHED && + *reason == VIR_DOMAIN_CRASHED_PANICKED) { + switch (info->type) { + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_HYPERV: + if (virTypedParamsAddInt(&par, &npar, + &maxparams, + VIR_DOMAIN_STATE_PARAM_TYPE, + VIR_DOMAIN_STATE_PARAM_TYPE_QEMU_HYPERV) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG1, + info->data.hyperv.arg1) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG2, + info->data.hyperv.arg2) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG3, + info->data.hyperv.arg3) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG4, + info->data.hyperv.arg4) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG5, + info->data.hyperv.arg5) < 0) { + goto cleanup; + } + break; + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_S390: + if (virTypedParamsAddInt(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_TYPE, + VIR_DOMAIN_STATE_PARAM_TYPE_QEMU_S390) < 0) { + goto cleanup; + } + if (virTypedParamsAddInt(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_CORE, + info->data.s390.core) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_MASK, + info->data.s390.psw_mask) < 0) { + goto cleanup; + } + if (virTypedParamsAddULLong(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_ADDR, + info->data.s390.psw_addr) < 0) { + goto cleanup; + } + if (virTypedParamsAddString(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_REASON, + info->data.s390.reason) < 0) { + goto cleanup; + } + break; + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_NONE: + case QEMU_DOMAIN_STATE_PANIC_INFO_TYPE_LAST: + if (virTypedParamsAddInt(&par, &npar, &maxparams, + VIR_DOMAIN_STATE_PARAM_TYPE, + VIR_DOMAIN_STATE_PARAM_TYPE_NONE) < 0) { + goto cleanup; + } + break; + } + } + + VIR_STEAL_PTR(*params, par); + *nparams = npar; + npar = 0; + ret = 0; + + cleanup: + virTypedParamsFree(par, npar); + virDomainObjEndAPI(&vm); + return ret; +} + static int qemuDomainGetControlInfo(virDomainPtr dom, virDomainControlInfoPtr info, @@ -22337,6 +22454,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetBlkioParameters = qemuDomainGetBlkioParameters, /* 0.9.0 */ .domainGetInfo = qemuDomainGetInfo, /* 0.2.0 */ .domainGetState = qemuDomainGetState, /* 0.9.2 */ + .domainGetStateParams = qemuDomainGetStateParams, /* 5.3.0 */ .domainGetControlInfo = qemuDomainGetControlInfo, /* 0.9.3 */ .domainSave = qemuDomainSave, /* 0.2.0 */ .domainSaveFlags = qemuDomainSaveFlags, /* 0.9.4 */ -- 2.19.1
Add a new parameter to virsh domstate, --info, to report additional information for the domain state, e.g. for a QEMU guest running a S390 domain: virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait When the --info parameter is specified and the new API virDomainGetStateParams is not available for the server or not supported by the hypervisor driver an error is reported. The --info parameter implies the --reason parameter and if additional information is not available, the output is identical. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- tools/virsh-domain-monitor.c | 94 +++++++++++++++++++++++++++++++++--- tools/virsh.pod | 5 +- 2 files changed, 90 insertions(+), 9 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index ad739a9d..a26f7371 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1391,35 +1391,115 @@ static const vshCmdOptDef opts_domstate[] = { .type = VSH_OT_BOOL, .help = N_("also print reason for the state") }, + {.name = "info", + .type = VSH_OT_BOOL, + .help = N_("also print reason and information for the state") + }, {.name = NULL} }; +static void +vshStateInfoMsgPrint(vshControl *ctl, + virTypedParameterPtr params, + int nparams) +{ + int type; + + if (virTypedParamsGetInt(params, nparams, + VIR_DOMAIN_STATE_PARAM_TYPE, &type) < 0) { + return; + } + + switch (type) { + case VIR_DOMAIN_STATE_PARAM_TYPE_QEMU_HYPERV: { + unsigned long long arg1, arg2, arg3, arg4, arg5; + + if (virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG1, &arg1) < 0 || + virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG2, &arg2) < 0 || + virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG3, &arg3) < 0 || + virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG4, &arg4) < 0 || + virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_HYPERV_ARG5, &arg5) < 0) { + return; + } + + vshPrint(ctl, _("hyperv.arg1 = 0x%llx\n"), arg1); + vshPrint(ctl, _("hyperv.arg2 = 0x%llx\n"), arg2); + vshPrint(ctl, _("hyperv.arg3 = 0x%llx\n"), arg3); + vshPrint(ctl, _("hyperv.arg4 = 0x%llx\n"), arg4); + vshPrint(ctl, _("hyperv.arg5 = 0x%llx\n"), arg5); + + break; + } + case VIR_DOMAIN_STATE_PARAM_TYPE_QEMU_S390: { + int core; + unsigned long long psw_mask; + unsigned long long psw_addr; + const char *reason; + + if (virTypedParamsGetInt(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_CORE, &core) < 0 || + virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_MASK, &psw_mask) < 0 || + virTypedParamsGetULLong(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_PSW_ADDR, &psw_addr) < 0 || + virTypedParamsGetString(params, nparams, + VIR_DOMAIN_STATE_PARAM_CRASHED_PANICKED_S390_REASON, &reason) < 0) { + return; + } + + vshPrint(ctl, _("s390.core = %d\n"), core); + vshPrint(ctl, _("s390.psw-mask = 0x%016llx\n"), psw_mask); + vshPrint(ctl, _("s390.psw-addr = 0x%016llx\n"), psw_addr); + vshPrint(ctl, _("s390.reason = %s\n"), reason); + + break; + } + case VIR_DOMAIN_STATE_PARAM_TYPE_NONE: + case VIR_DOMAIN_STATE_PARAM_TYPE_LAST: + break; + } +} + static bool cmdDomstate(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool ret = true; bool showReason = vshCommandOptBool(cmd, "reason"); + bool showInfo = vshCommandOptBool(cmd, "info"); + virTypedParameterPtr params = NULL; + int nparams = 0; int state, reason; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if ((state = virshDomainState(ctl, dom, &reason)) < 0) { + if ((showInfo && + virDomainGetStateParams(dom, &state, &reason, ¶ms, &nparams, 0) < 0) || + (!showInfo && (state = virshDomainState(ctl, dom, &reason)) < 0)) { + vshError(ctl, _("Unable to get state of domain %s"), virDomainGetName(dom)); ret = false; goto cleanup; } - if (showReason) { - vshPrint(ctl, "%s (%s)\n", - virshDomainStateToString(state), - virshDomainStateReasonToString(state, reason)); + vshPrint(ctl, "%s", virshDomainStateToString(state)); + + if (showReason || showInfo) { + vshPrint(ctl, " (%s)\n", virshDomainStateReasonToString(state, reason)); + + if (showInfo) + vshStateInfoMsgPrint(ctl, params, nparams); } else { - vshPrint(ctl, "%s\n", - virshDomainStateToString(state)); + vshPrint(ctl, "\n"); } cleanup: + virTypedParamsFree(params, nparams); virshDomainFree(dom); return ret; } diff --git a/tools/virsh.pod b/tools/virsh.pod index b7ceba8d..d7a06dc9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1572,10 +1572,11 @@ specified in the second argument. B<Note>: Domain must be inactive and without snapshots. -=item B<domstate> I<domain> [I<--reason>] +=item B<domstate> I<domain> [I<--reason>] [I<--info>] Returns state about a domain. I<--reason> tells virsh to also print -reason for the state. +reason for the state. I<--info> prints additional state information if +available, I<--info> implies I<--reason>. =item B<domcontrol> I<domain> -- 2.19.1
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 0729e0ba..6a7394c7 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,17 @@ <libvirt> <release version="v5.3.0" date="unreleased"> <section title="New features"> + <change> + <summary> + Ability to set/get additional state information + </summary> + <description> + Add the ability to set/get additional state information, e.g. guest + CPU state for crashed guests. Introduce a new extensible API + function, virDomainGetStateParams, and add new argument --info to + the virsh domstate command. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.19.1
On Thu, Apr 04, 2019 at 10:01:27AM +0200, Bjoern Walk wrote:
This patch series introduces the ability to save additional information for the domain state and exposes this information in virsh domstate.
For example in the case of QEMU guest panic events, we can provide additional information like the crash reason or register state of the domain. This information usually gets logged in the domain log but for debugging it is useful to have it accessible from the client. Therefore, let's introduce a new public API function, virDomainGetStateParams, an extensible version of virDomainGetState, which returns the complete state of the domain, including newly introduced additional information.
Let's also extend virsh domstate and introduce a new parameter --info to show the domain state, reason and additional information when available.
virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait
This info is all just guest panick related data, so I'm not covinced we should overload "domstate" for this random set of low level hardware parameters. Why not just have virDomainGetPanicInfo() and "virsh dompanicinfo" Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Apr 04, 2019 at 09:49:16 +0100, Daniel Berrange wrote:
On Thu, Apr 04, 2019 at 10:01:27AM +0200, Bjoern Walk wrote:
This patch series introduces the ability to save additional information for the domain state and exposes this information in virsh domstate.
For example in the case of QEMU guest panic events, we can provide additional information like the crash reason or register state of the domain. This information usually gets logged in the domain log but for debugging it is useful to have it accessible from the client. Therefore, let's introduce a new public API function, virDomainGetStateParams, an extensible version of virDomainGetState, which returns the complete state of the domain, including newly introduced additional information.
Let's also extend virsh domstate and introduce a new parameter --info to show the domain state, reason and additional information when available.
virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait
This info is all just guest panick related data, so I'm not covinced we should overload "domstate" for this random set of low level hardware parameters.
I'm not even sure whether it's worth having an API to query it at all. We don't really have means to store the data reliably across libvirtd restarts as there is no status XML for inactive VMs. This means that the data will get lost. It also will become instantly invalidated when starting the VM. Currently we log the data into the domain log file which in my opinion feels good enough for this kind of low level information which is not of much use for non-devs. Additionally the advantage of logging is that bug reporting tools usually capture the VM log files.
On Thu, Apr 04, 2019 at 11:48:40AM +0200, Peter Krempa wrote:
On Thu, Apr 04, 2019 at 09:49:16 +0100, Daniel Berrange wrote:
On Thu, Apr 04, 2019 at 10:01:27AM +0200, Bjoern Walk wrote:
This patch series introduces the ability to save additional information for the domain state and exposes this information in virsh domstate.
For example in the case of QEMU guest panic events, we can provide additional information like the crash reason or register state of the domain. This information usually gets logged in the domain log but for debugging it is useful to have it accessible from the client. Therefore, let's introduce a new public API function, virDomainGetStateParams, an extensible version of virDomainGetState, which returns the complete state of the domain, including newly introduced additional information.
Let's also extend virsh domstate and introduce a new parameter --info to show the domain state, reason and additional information when available.
virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait
This info is all just guest panick related data, so I'm not covinced we should overload "domstate" for this random set of low level hardware parameters.
I'm not even sure whether it's worth having an API to query it at all. We don't really have means to store the data reliably across libvirtd restarts as there is no status XML for inactive VMs. This means that the data will get lost. It also will become instantly invalidated when starting the VM.
I'm not bothered about loosing it when starting the VM. I tend to view it as "point in time" data about CPU state. I think the most immediately useful is to actually include this in an async event when the crash happens. It is possible to configure the panic action so that either QEMU is killed (and optionally) restarted), or for QEMU to simply have its CPUs stopped. In the latter case I think it is reasonable to have an API to report it and this lets us save it in the domain state XML too. As soon as QEMU is actually stopped though, I think we should no longer try to report it. IOW, apps should use the event primarily. If they want to get it via the API, they must configure QEMU to stop CPUs on panic, instead of shutting down/starting. I think this would fit well with my suggestion to consider this API as a way to report live CPU registers (eg a libvirt API to expose QEMU's "info registers" data). Obviously that woudl require a QMP impl first. Just saying this from a conceptual design POV.
Currently we log the data into the domain log file which in my opinion feels good enough for this kind of low level information which is not of much use for non-devs.
We really should add an API via virStream to fetch logs
Additionally the advantage of logging is that bug reporting tools usually capture the VM log files.
It is definitely good to have it in the logs, but at the same time this is structured data and so it is good to preseve the structure for apps if they want todo live analysis without needing human to read logs. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> [2019-04-04, 09:49AM +0100]:
On Thu, Apr 04, 2019 at 10:01:27AM +0200, Bjoern Walk wrote:
This patch series introduces the ability to save additional information for the domain state and exposes this information in virsh domstate.
For example in the case of QEMU guest panic events, we can provide additional information like the crash reason or register state of the domain. This information usually gets logged in the domain log but for debugging it is useful to have it accessible from the client. Therefore, let's introduce a new public API function, virDomainGetStateParams, an extensible version of virDomainGetState, which returns the complete state of the domain, including newly introduced additional information.
Let's also extend virsh domstate and introduce a new parameter --info to show the domain state, reason and additional information when available.
virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait
This info is all just guest panick related data, so I'm not covinced we should overload "domstate" for this random set of low level hardware parameters.
I want to have a flexible and extensible API function for all states that provide any additional information. The crashed/panicked state just happens to be the only one that does currently... We discussed the API in v1 here: https://www.redhat.com/archives/libvir-list/2018-July/msg00690.html
Why not just have virDomainGetPanicInfo() and "virsh dompanicinfo"
Do we want to later add an additional public API function per state that implements any additional information?
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Apr 04, 2019 at 11:59:50 +0200, Bjoern Walk wrote:
Daniel P. Berrangé <berrange@redhat.com> [2019-04-04, 09:49AM +0100]:
On Thu, Apr 04, 2019 at 10:01:27AM +0200, Bjoern Walk wrote:
[...]
Why not just have virDomainGetPanicInfo() and "virsh dompanicinfo"
Do we want to later add an additional public API function per state that implements any additional information?
Any elaboration on the additional information you'd want to report? We have https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectGetAllDomainS... which is able to report any information including state info (VIR_DOMAIN_STATS_STATE) using typed parameters. The API works even for inactive VMs so should be usable in this case.
On Thu, Apr 04, 2019 at 11:59:50AM +0200, Bjoern Walk wrote:
Daniel P. Berrangé <berrange@redhat.com> [2019-04-04, 09:49AM +0100]:
On Thu, Apr 04, 2019 at 10:01:27AM +0200, Bjoern Walk wrote:
This patch series introduces the ability to save additional information for the domain state and exposes this information in virsh domstate.
For example in the case of QEMU guest panic events, we can provide additional information like the crash reason or register state of the domain. This information usually gets logged in the domain log but for debugging it is useful to have it accessible from the client. Therefore, let's introduce a new public API function, virDomainGetStateParams, an extensible version of virDomainGetState, which returns the complete state of the domain, including newly introduced additional information.
Let's also extend virsh domstate and introduce a new parameter --info to show the domain state, reason and additional information when available.
virsh # domstate --info guest-1 crashed (panicked) s390.core = 0 s390.psw-mask = 0x0002000180000000 s390.psw-addr = 0x000000000010f146 s390.reason = disabled-wait
This info is all just guest panick related data, so I'm not covinced we should overload "domstate" for this random set of low level hardware parameters.
I want to have a flexible and extensible API function for all states that provide any additional information. The crashed/panicked state just happens to be the only one that does currently... We discussed the API in v1 here: https://www.redhat.com/archives/libvir-list/2018-July/msg00690.html
Why not just have virDomainGetPanicInfo() and "virsh dompanicinfo"
Do we want to later add an additional public API function per state that implements any additional information?
I guess the obvious extra thing to want to report is CPU registers, since the crash info is largely containing register and/or memory address info. QEMU has "info registers" but no QMP variant of it at this time. With this in mind though, the proposed API is not satisfactory. Specifically the field # define VIR_DOMAIN_STATE_PARAM_TYPE "info_type" As that assumes an either / or reporting need. If we added register info, then we would potentially need to report crash info *and* register info at the same time. I think this is easy to solve though - just delete the VIR_DOMAIN_STATE_PARAM_TYPE field as it is redundant. Apps can just look at whatever named parameters exist & use those they care about. The more critical thing is that in an SMP system, we'll need to report registers per CPU, but this API is a per-VM level reporting. This is something we do with virDomainGetCPUStats(). So I think we need a design closer to that API, and perhaps call it "virDomainGetCPUState" / virsh domcpustate Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
So, what's my course of action here? Daniel P. Berrangé <berrange@redhat.com> [2019-04-04, 11:32AM +0100]:
I guess the obvious extra thing to want to report is CPU registers, since the crash info is largely containing register and/or memory address info.
Sounds reasonable if we go with the virDomainGetCPUState API.
QEMU has "info registers" but no QMP variant of it at this time.
Can I still implement my stuff with the proposed API without the actual register information until this is implemented in QEMU? Meaning, just reporting the relevant register information from the panic event. While it is nice to have the full feature set available, implementing the QMP command for this would be a bit out of the scope of this proposal.
With this in mind though, the proposed API is not satisfactory. Specifically the field
# define VIR_DOMAIN_STATE_PARAM_TYPE "info_type"
As that assumes an either / or reporting need. If we added register info, then we would potentially need to report crash info *and* register info at the same time.
I think this is easy to solve though - just delete the VIR_DOMAIN_STATE_PARAM_TYPE field as it is redundant. Apps can just look at whatever named parameters exist & use those they care about.
Sure, the type parameter was basically an easy way for the client to figure out what data it has retrieved, especially when the parameter space was that large with all the (potential) different states.
The more critical thing is that in an SMP system, we'll need to report registers per CPU, but this API is a per-VM level reporting.
This is something we do with virDomainGetCPUStats().
So I think we need a design closer to that API, and perhaps call it "virDomainGetCPUState" / virsh domcpustate
Any hint how this API actually should look like? Exactly like virDomainGetCPUStats with the per-CPU-dance or should we just report back all available information? In the case of a panicked domain, how should we report the relevant information (crashed core/reason)?
Regards, Daniel
Thanks for the discussion and suggestions. Bjoern -- 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: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (3)
-
Bjoern Walk -
Daniel P. Berrangé -
Peter Krempa