Jiri Denemark <jdenemar@xxxxxxxxxx> [2018-07-11, 01:17PM +0200]: > On Wed, Jul 11, 2018 at 12:49:13 +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: disabled-wait core='1' psw-mask='0x000000000010f146' \ > > psw-addr='0x0002000180000000') > > I was thinking you introduced a new API with typed parameters so that > you can return each part of the info string as a separate parameter with > a defined semantics. But I was wrong, you're just adding a new opaque > string with more details about the reason. In that case typed parameters > don't actually bring any additional value, they just complicate the > usage of the API. The following prototype would be much better The extensibility for this new API was more regarding for future additions of any state related information. Since libvirt does not have a deprecation model for APIs, whenever we would want to return additional information for the state (like in this case) a new API function has to be created. > int > virDomainGetState...(virDomainPtr domain, > int *state, > int *reason, > char **info, > unsigned int flags) > > On the other hand, is an opaque string really a good idea? It makes the > additional info usable only for being shown to the user rather than > being processed by an upper management layer. That's probably fine for > crashed state, but perhaps other states would want to return something > that is supposed to be processed. Maybe I'm just overthinking this, but > I'd like to avoid having to add yet another API later. So the API could > have the following prototype Sure, if machine readability is a goal this approach makes certainly more sense. On the other hand, the same information can be queried by a qemu-monitor-command call and retrieved in JSON format. This information here is aimed at human interaction, similar to the log output. It is also unclear which platforms/hypervisors would provide what information and mapping all of them to a virTypedParameter entry could result in a rather large list. Certainly no arguments against your objection, and if the overall concensus is that this is something we want, I can definitely rework the API. > int > virDomainGetStateParams(virDomainPtr domain, > int *state, > int *reason, > virTypedParameterPtr *params, > int *nparams, > unsigned int flags) > > I'd still keep the state and reason parameters directly to make the API > easier to use. > > Jirka > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > -- 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: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
