Re: [libvirt] [PATCH v3 0/7] extend virsh domstate to show additional information
On Thu, Apr 04, 2019 at 11:59:50AM +0200, Bjoern Walk wrote:
Daniel P. Berrangé <berrange(a)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 :|