On Thu, Apr 04, 2019 at 11:59:50AM +0200, Bjoern Walk wrote: > Daniel P. Berrangé <berrange@xxxxxxxxxx> [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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
