Re: [libvirt] [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
On 05/03/2013 08:58 AM, Andreas Färber wrote:
> I agree that libvirt would very much like to have this in 1.5. How
> can I help in reviewing things?
Apart from the usual QMP considerations that you will know much better
than me, I have two concerns here:
1) Polluting the QOM namespace with this dump-all implementation for
libvirt and interfering with more fine-grained property getters/setters.
Ultimately, it would be nice to have full QOM introspection. We are
part way there: we know WHAT properties we can query ('qom-list' exists
now), but right now it returns details on the type of a property as a
string, rather than as a full-blown JSON representation of the full
details of that type. Maybe if we had an additional QMP command that
gave back a full JSON representation of any type, so you can feed the
'type':'str' field from ObjectPropertyInfo in 'qom-list' into the
new
command to get a full idea of what each property contains. With full
introspection, we could then have the option of changing the layout of
the feature-words and filtered-features properties, but if we do make a
change in the future, it would be nice to remain backwards compatible
(adding features, rather than removing).
2) Basing its design on current code of which we are not sure yet
how
it may evolve and having to live with that for ABI stability.
Like I said, I hadn't reviewed that part yet, so couldn't pick it up
on short notice. If we get it respun and reviewed today, I can (try
to) prepare a PULL on Sunday.
I guess I see where you are coming from - once we bake in the QOM
property name and libvirt starts relying on it, it becomes harder to
support the generation of that property in the future if the underlying
implementation of how feature bits are represented in qemu changed. But
the hope is that we have already sanitized the feature word generation
into something that is a lot more maintainable (iterating over a struct
of descriptions of how to get at each feature), and where future changes
would merely be extending (not deleting) from that struct (and therefore
making the corresponding JSON type returned via the QOM property
larger). And since these two properties are read-only, extending the
JSON struct shouldn't be a problem.
On Igor's series (latest: v7 from Feb 25) I had more or less nack'ed
the attempt to introduce f-* properties due to Anthony asking for
verbose QOM property names, so we're in need of a better name, likely
something with "feature" in it, similar to what is being proposed here.
I had also argued with Anthony that QOM's object_property_add_bool()
should allow us to create a container object for accessing features in
a more simple way, such as .../icc/child[0]/cpuid-features/foo rather
than f-foo or feature-foo or foo-feature to avoid the constant
repetition and an unreadable long list of CPU properties, but the
addition of an opaque to support this was turned down.
The problem with adding container objects is that you can only access
one feature at a time, instead of all of them in a single array. I
guess libvirt would cope with either style, but it is nicer to be able
to get everything at once via JSON struct than it is to have to make
multiple qom-get calls.
So it boils down to the questions of where do we want to expose which
information, how should it be structured and where does/will that
information come from. Thanks.
I guess that decision is up to the qemu maintainers - all I can do is
say whether the proposed interface is usable, not whether it is the
ideal interface compared to some other layout of where the property is
attached.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)