[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]