Re: [Qemu-devel] [PATCH qom-cpu 00/16 v10] target-i386: convert CPU features into properties

["Andreas Färber" <afaerber@suse.de>]

Am 06.02.2014 16:19, schrieb Igor Mammedov:
> On Wed, 5 Feb 2014 17:52:16 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
>> On Wed, 05 Feb 2014 17:14:27 +0100
>> Andreas Färber <afaerber@suse.de> wrote:
>>
>>> Am 05.02.2014 15:40, schrieb Igor Mammedov:
>>>> On Sun, 15 Dec 2013 23:50:47 +0100
>>>> Andreas Färber <afaerber@suse.de> wrote:
>>>>
>>>>> Am 27.11.2013 23:28, schrieb Igor Mammedov:
>>>>>> Igor Mammedov (16):
>>>>>>   target-i386: cleanup 'foo' feature handling'
>>>>>>   target-i386: cleanup 'foo=val' feature handling
>>>>>
>>>>> Thanks, I've queued these on qom-cpu-next:
>>>>> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next
>>>>>
>>>>>>   target-i386: cpu: convert 'level' to static property
>>>>>>   target-i386: cpu: convert 'xlevel' to static property
>>>>>>   target-i386: cpu: convert 'family' to static property
>>>>>>   target-i386: cpu: convert 'model' to static property
>>>>>>   target-i386: cpu: convert 'stepping' to static property
>>>>>>   target-i386: cpu: convert 'vendor' to static property
>>>>>>   target-i386: cpu: convert 'model-id' to static property
>>>>>>   target-i386: cpu: convert 'tsc-frequency' to static property
>>>>>
>>>>> But I still don't see the utility of this conversion after all the
>>>>> discussions we've had... :(
>>>> It seems there is movement to make DEVICE self describing for purpose
>>>> of QAPI schema introspection, where static properties would be used
>>>> (dynamic ones are not suitable for this purpose)
>>>
>>> Do you have a pointer to such a discussion? Sounds like I was not
>>> involved and Anthony probably not either...
>> Not at the moment, CCing people who might know more about the topic.
>>
>> But just thinking about creating QAPI schema for devices, it's not really
>> possible to generate one using dynamic properties (unless one resorts to
>> creating every supported device instance), while arrays of static properties
>> are there for every device and simplistically speaking just need conversion
>> to schema format.
> 
> There is one more reason to use static properties for external user-settable
> properties when it comes to device, i.e. to get list of properties user could
> use command:
> #qemu -device x86_64-cpu,?
> x86_64-cpu.pmu=boolean
> x86_64-cpu.hv-spinlocks=int
> x86_64-cpu.hv-relaxed=boolean
> x86_64-cpu.hv-vapic=boolean
> x86_64-cpu.check=boolean
> x86_64-cpu.enforce=boolean
> 
> which now yields only partial properties user is interested in, above
> mentioned conversion patches make previously not available properties
> visible to user via typical interface, perhaps eventually we could
> drop list_cpu() interface in favor of -device foo-cpu,? command.

We already brought that up specifically for decision on a KVM call and
Anthony's clear statement was that the expected way for management tools
to discover properties was to instantiate the object and run qom-list on it.

It is a known issue, both for info qtree and -device, that they do not
list all properties. But I don't want to repeat this discussion over and
over again: Paolo's patches for static properties were rejected by
Anthony, therefore I could not queue them on qom-next back then and
therefore I had to code my properties for the X86CPU (which was not yet
a device back then) the new QOM way, and now you're trying to override
Anthony's decision in forcing me to accept patches that Anthony had
vetoed against!

If you or libvirt need all properties for -device, then send a patch. No
one did for two years, so apparently no one cared.

Static properties are considered a valid, convenient way to define
properties for a device but not the sole one for a device or object.
Using info qtree or -device as justification for implementation
decisions is backwards and wrong since those are considered legacy.

And specifically for libvirt Eduardo pushed into a release properties
that could be used to inspect CPUIDs. If that's not being used by
libvirt, as Eduardo seems to imply now, why did we put work in that in
the first place?

If there's no relation between a CPU model named, e.g., "Haswell" and
the one on an Intel Haswell chip any more, then we should give them
artificial names like "qemu64"; I strongly believe that Haswell
definition in code should match that of the specs/hardware and if TCG
can't emulate something that's one thing (subtractive: no AVX) and if
KVM wants to speed up things that's another (additive: kvmvapic,
in-kernel IRQ/PIT). What I am arguing against is watering the meaning of
our definitions from "this is this model" to "this is handy". In
particular, if we use the post_initialize hook like I suggested then
-global is still able to override it and Eduardo's properties should
correctly report them to libvirt.

> Taking in account Paolo's cleanup of legacy properties in static properties,
> it might make them more suitable for moving concept to Object level.
> (As far as I remember, Anthony objected to it due to existence of legacy
> properties).

That'll be for Anthony to answer, but static properties at Object level
would still not expose child<> and especially not link<> properties to
the user.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg