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 :|