[libvirt] CPU models and feature probing (was Re: [Qemu-devel] [PATCH qom-cpu 00/16 v10] target-i386: convert CPU) features into properties
(CCing libvir-list again, as this is continuing a discussion about a subject that interests libvirt developers, from another thread.) On Thu, Feb 06, 2014 at 04:51:17PM +0100, Andreas Färber wrote:
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.
As far as I remember, this was decided as the recommended way to list the feature _default values_ (e.g. discover which features are going to be enabled on each CPU model). Is this really the recommended way to discover which properties can be set on device_add, too? We are not going to have any other instrospection mechanism that won't require objects to be created?
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.
So, let me ask again, explicitly: we are not going to ever have a QMP-based interface that will allow all available device_add options to be queried without instantiating an object first? This really surprises me. (That's not a question just for Andreas. I would like to hear from Paolo, Anthony, and others.)
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?
It is not being used by libvirt because the current interface is unusable without running QEMU once for each CPU model. With the CPU model subclasses, we will be able to make it possible to create/destroy CPUs of different models in a single QEMU instance, and then libvirt will be able to use it.
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.
"Haswell" is named this way not only because it looks like Haswell, but also because it has useful features you are going to find only on Haswell, and you (probably) are not going to be able to run it on hosts older than Haswell. That's the main real-world application of CPU models: making sure the VMs can run on a specific subset of hosts. So, if you choose "Haswell", you are telling the management stack "I know this is going to run only on Haswell and newer CPUs". That's why x2apic is being proposed as an exception: it can be enabled on any host, because it doesn't depend on host-side support. That's why I propose we enable it on CPU models that don't have x2apic in the real world. (BTW, what is the relation between this subject and static properties? I was expecting this to be discussed in the other existing thread about x2apic)
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
-- Eduardo
Am 06.02.2014 17:16, schrieb Eduardo Habkost:
(CCing libvir-list again, as this is continuing a discussion about a subject that interests libvirt developers, from another thread.)
On Thu, Feb 06, 2014 at 04:51:17PM +0100, Andreas Färber wrote:
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.
As far as I remember, this was decided as the recommended way to list the feature _default values_ (e.g. discover which features are going to be enabled on each CPU model). Is this really the recommended way to discover which properties can be set on device_add, too? We are not going to have any other instrospection mechanism that won't require objects to be created?
It is, and no, no other introspection mechanism on the horizon. I specifically suggested to have a copy of the ObjectProperty list in ObjectClass (today it is just in Object) but Anthony rejected that idea due to the dynamic nature of properties in QOM and pointed us to said qom-list and object instantiation as the sole means for discovering availability of properties. And it's true that we could in fact just instantiate the object for -device foo,? - it's just that nobody wrote code for that. I didn't do the original QOM conversion so I don't feel guilty, I don't normally use -device foo,? so not affected, I followed Anthony's instructions for how to and how not to implement things. In a few cases Anthony has changed his mind (method inheritence for instance) but for that you'll need to discuss with him and not just argue with me about something that Anthony has decided in the past. That's just pissing me off because it feels like a waste of my time.
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.
So, let me ask again, explicitly: we are not going to ever have a QMP-based interface that will allow all available device_add options to be queried without instantiating an object first? This really surprises me.
(That's not a question just for Andreas. I would like to hear from Paolo, Anthony, and others.)
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?
It is not being used by libvirt because the current interface is unusable without running QEMU once for each CPU model. With the CPU model subclasses, we will be able to make it possible to create/destroy CPUs of different models in a single QEMU instance, and then libvirt will be able to use it.
OK, that I understand now, so let's be sure to get (the remainder of) that subclasses series through my review mills in time for 2.0. :)
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.
"Haswell" is named this way not only because it looks like Haswell, but also because it has useful features you are going to find only on Haswell, and you (probably) are not going to be able to run it on hosts older than Haswell. That's the main real-world application of CPU models: making sure the VMs can run on a specific subset of hosts. So, if you choose "Haswell", you are telling the management stack "I know this is going to run only on Haswell and newer CPUs".
That's why x2apic is being proposed as an exception: it can be enabled on any host, because it doesn't depend on host-side support. That's why I propose we enable it on CPU models that don't have x2apic in the real world.
(BTW, what is the relation between this subject and static properties? I was expecting this to be discussed in the other existing thread about x2apic)
Sorry if I replied to two different series at once, that was for "Enabling x2apic by default in KvM (was Re: [Qemu-devel] [PATCH] target-i386: enable x2apic by default on more recent CPU models)". Which is connected to CPU models/subclasses in what those types are supposed to be good for if we define them. Let's take an obvious example. Jan wants to emulate a legacy isapc machine with -cpu 486. Then it feels counterproductive to enable the latest and greatest features such a machine never had. If the user wants to have the latest and greatest features, she can choose a newer -cpu model which already has the flag today. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On Thu, Feb 06, 2014 at 05:57:38PM +0100, Andreas Färber wrote: [...]
And it's true that we could in fact just instantiate the object for -device foo,? - it's just that nobody wrote code for that. I didn't do the original QOM conversion so I don't feel guilty, I don't normally use -device foo,? so not affected, I followed Anthony's instructions for how to and how not to implement things. In a few cases Anthony has changed his mind (method inheritence for instance) but for that you'll need to discuss with him and not just argue with me about something that Anthony has decided in the past. That's just pissing me off because it feels like a waste of my time.
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused. -- Eduardo
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans. At some point you have to acknowledge that the grand vision of QEMU as a small core and management only poking at QOM objects with properties has never materialized, and probably never will. After seeing no progress whatsoever on http://wiki.qemu.org/Features/QOM#TODO after 2 years, we should perhaps try to get fresh ideas into QOM based on how we've been using it and what we'd like to do with it. "Design by committee" (more accurately: "design by prophet") will not lead us anywhere. QOM definitely needs dynamic properties for child<>, and for tricks such as simulation of array properties. However, assume Igor or Eduardo or Marcel can come up with a new QAPI-friendly static properties design, that combines the best feature of QOM dynamic properties and qdev static properties, why should it be rejected? Code should not be frozen against some abstract design, it must evolve to solve concrete problems until it can solve all of them easily. Or do we want to become a project where good code is not anymore the best way to have other developers change their mind? Paolo
On Fri, Feb 07, 2014 at 11:55:25AM +0100, Paolo Bonzini wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
At some point you have to acknowledge that the grand vision of QEMU as a small core and management only poking at QOM objects with properties has never materialized, and probably never will.
After seeing no progress whatsoever on http://wiki.qemu.org/Features/QOM#TODO after 2 years, we should perhaps try to get fresh ideas into QOM based on how we've been using it and what we'd like to do with it. "Design by committee" (more accurately: "design by prophet") will not lead us anywhere.
QOM definitely needs dynamic properties for child<>, and for tricks such as simulation of array properties. However, assume Igor or Eduardo or Marcel can come up with a new QAPI-friendly static properties design, that combines the best feature of QOM dynamic properties and qdev static properties, why should it be rejected?
Code should not be frozen against some abstract design, it must evolve to solve concrete problems until it can solve all of them easily. Or do we want to become a project where good code is not anymore the best way to have other developers change their mind?
Also, I don't see the point in deprecating and rejecting code that use something which: * Is used ~230 times, in ~180 different source files in QEMU; * Is useful; * Doesn't have a replacement yet. Currently, static properties are useful, facilitate analyzing the code before compile time, facilitate exposing class information through QMP (with the existing "device-list-properties" command), and is better than requiring objects to be created just to find out which properties are available. It may replaced by something better in the future, but that's what we have today. -- Eduardo
On Fri, Feb 7, 2014 at 2:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
There's no real convincing. It's just a question of code. There are no defaults in classes for dynamic properties to modify. compat_props are a nice mechanism, making them work for all properties is a reasonable thing to do. Regards, Anthony Liguori
On Tue, Feb 11, 2014 at 06:31:35AM -0800, Anthony Liguori wrote:
On Fri, Feb 7, 2014 at 2:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
There's no real convincing. It's just a question of code.
I am sure there's a lot of convincing involved, even after the code is written (in this case, 15 months after the code was written).
There are no defaults in classes for dynamic properties to modify. compat_props are a nice mechanism, making them work for all properties is a reasonable thing to do.
That's exactly the opposite of what you said before[1]. But that isn't supposed to be a problem, I understand there may be change of plans (we should be able to change our minds). What I don't understand is the rejection of code that works, matches the style used by 200+ other source files, adds more useful introspectable information, done in the way that was suggested 16 months ago, because we have some rough idea about how a new grand design will look like in the far future. [1] http://lists.gnu.org/archive/html/qemu-devel/2013-06/msg00990.html -- Eduardo
On Tue, Feb 11, 2014 at 7:25 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Feb 11, 2014 at 06:31:35AM -0800, Anthony Liguori wrote:
On Fri, Feb 7, 2014 at 2:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
There's no real convincing. It's just a question of code.
I am sure there's a lot of convincing involved, even after the code is written (in this case, 15 months after the code was written).
N.B. the code you refer to doesn't "make global propeties and compat_props affect dynamic properties." It converts CPU properties to static properties which I'm pretty sure I said many times is a perfectly reasonable thing to do.
There are no defaults in classes for dynamic properties to modify. compat_props are a nice mechanism, making them work for all properties is a reasonable thing to do.
That's exactly the opposite of what you said before[1]. But that isn't supposed to be a problem, I understand there may be change of plans (we should be able to change our minds).
I think you're confusing a few things. You cannot make dynamic properties work with globals today. Globals change class default values and there are no class defaults for dynamic properties.[*] There's a perfectly valid discussion to have about whether we should even have dynamic properties. It's certainly been a long time since they were introduced and they haven't made their way into all that many devices so it's reasonable to say that perhaps we'd be better off without them. I would not object to a patch series that moved properties to classes entirely provided it removed existing uses of dynamic properties and didn't just introduce yet another mechanism. But compat properties as a concept could be made to work with dynamic properties. They would have to be evaluated after instance init. There's quite a few places they would end up touching I suspect. Another point of confusion worth mention is legacy properties since this usually comes up in the discussion. Legacy properties (the properties that are set/get as strings) are something that we should try to avoid. They end up as strings on the wire and make it harder to write client code. * I recognize that compat_props are implemented as globals. I'm really talking about the current implementation of globals, not the concept of -global which could be made with dynamic properties. Regards, Anthony Liguori
What I don't understand is the rejection of code that works, matches the style used by 200+ other source files, adds more useful introspectable information, done in the way that was suggested 16 months ago, because we have some rough idea about how a new grand design will look like in the far future.
[1] http://lists.gnu.org/archive/html/qemu-devel/2013-06/msg00990.html
-- Eduardo
On Tue, Feb 11, 2014 at 07:58:30AM -0800, Anthony Liguori wrote:
On Tue, Feb 11, 2014 at 7:25 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Feb 11, 2014 at 06:31:35AM -0800, Anthony Liguori wrote:
On Fri, Feb 7, 2014 at 2:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
There's no real convincing. It's just a question of code.
I am sure there's a lot of convincing involved, even after the code is written (in this case, 15 months after the code was written).
N.B. the code you refer to doesn't "make global propeties and compat_props affect dynamic properties." It converts CPU properties to static properties which I'm pretty sure I said many times is a perfectly reasonable thing to do.
Exactly. Have you read the rest of this thread?
There are no defaults in classes for dynamic properties to modify. compat_props are a nice mechanism, making them work for all properties is a reasonable thing to do.
That's exactly the opposite of what you said before[1]. But that isn't supposed to be a problem, I understand there may be change of plans (we should be able to change our minds).
I think you're confusing a few things. You cannot make dynamic properties work with globals today. Globals change class default values and there are no class defaults for dynamic properties.[*]
They work today. Not that people _should_ use -global with them, but it works. All we really needed (before this series) was to make compat_props and -cpu be able to affect the dynamic properties.
There's a perfectly valid discussion to have about whether we should even have dynamic properties. It's certainly been a long time since they were introduced and they haven't made their way into all that many devices so it's reasonable to say that perhaps we'd be better off without them. I would not object to a patch series that moved properties to classes entirely provided it removed existing uses of dynamic properties and didn't just introduce yet another mechanism.
That sounds like the opposite of what I have been reading in this thread. Now I am even more confused.
But compat properties as a concept could be made to work with dynamic properties. They would have to be evaluated after instance init. There's quite a few places they would end up touching I suspect.
They already work. -- Eduardo
Il 11/02/2014 17:43, Eduardo Habkost ha scritto:
But compat properties as a concept could be made to work with dynamic properties. They would have to be evaluated after instance init. There's quite a few places they would end up touching I suspect. They already work.
But if they work by an ad hoc mechanism, this series actually improves the situation. Paolo
Am 11.02.2014 16:58, schrieb Anthony Liguori:
On Tue, Feb 11, 2014 at 7:25 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Feb 11, 2014 at 06:31:35AM -0800, Anthony Liguori wrote:
On Fri, Feb 7, 2014 at 2:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
There's no real convincing. It's just a question of code.
I am sure there's a lot of convincing involved, even after the code is written (in this case, 15 months after the code was written).
N.B. the code you refer to doesn't "make global propeties and compat_props affect dynamic properties." It converts CPU properties to static properties which I'm pretty sure I said many times is a perfectly reasonable thing to do.
There are no defaults in classes for dynamic properties to modify. compat_props are a nice mechanism, making them work for all properties is a reasonable thing to do.
That's exactly the opposite of what you said before[1]. But that isn't supposed to be a problem, I understand there may be change of plans (we should be able to change our minds).
I think you're confusing a few things. You cannot make dynamic properties work with globals today. Globals change class default values and there are no class defaults for dynamic properties.[*]
There's a perfectly valid discussion to have about whether we should even have dynamic properties. It's certainly been a long time since they were introduced and they haven't made their way into all that many devices so it's reasonable to say that perhaps we'd be better off without them. I would not object to a patch series that moved properties to classes entirely provided it removed existing uses of dynamic properties and didn't just introduce yet another mechanism.
But compat properties as a concept could be made to work with dynamic properties. They would have to be evaluated after instance init. There's quite a few places they would end up touching I suspect.
Erm, sorry, that is already implemented in qemu.git!? instance_post_init by Eduardo plus glue by me. Andreas
Another point of confusion worth mention is legacy properties since this usually comes up in the discussion. Legacy properties (the properties that are set/get as strings) are something that we should try to avoid. They end up as strings on the wire and make it harder to write client code.
* I recognize that compat_props are implemented as globals. I'm really talking about the current implementation of globals, not the concept of -global which could be made with dynamic properties.
Regards,
Anthony Liguori
What I don't understand is the rejection of code that works, matches the style used by 200+ other source files, adds more useful introspectable information, done in the way that was suggested 16 months ago, because we have some rough idea about how a new grand design will look like in the far future.
[1] http://lists.gnu.org/archive/html/qemu-devel/2013-06/msg00990.html
-- Eduardo
-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On Tue, Feb 11, 2014 at 8:55 AM, Andreas Färber <afaerber@suse.de> wrote:
Am 11.02.2014 16:58, schrieb Anthony Liguori:
On Tue, Feb 11, 2014 at 7:25 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Tue, Feb 11, 2014 at 06:31:35AM -0800, Anthony Liguori wrote:
On Fri, Feb 7, 2014 at 2:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
Il 07/02/2014 11:16, Eduardo Habkost ha scritto:
You are not alone. I remember we spent lots of time trying to convince Anthony to allow global properties and compat_props affect dynamic properties not just static properties, and static properties were a big deal due to reasons I didn't understand completely. Now I am hearing the opposite message, and I don't understand the reasons for the change of plans. I am confused.
Picture me confused as well, but at the same I think I understand the reasons for the change of plans.
There's no real convincing. It's just a question of code.
I am sure there's a lot of convincing involved, even after the code is written (in this case, 15 months after the code was written).
N.B. the code you refer to doesn't "make global propeties and compat_props affect dynamic properties." It converts CPU properties to static properties which I'm pretty sure I said many times is a perfectly reasonable thing to do.
There are no defaults in classes for dynamic properties to modify. compat_props are a nice mechanism, making them work for all properties is a reasonable thing to do.
That's exactly the opposite of what you said before[1]. But that isn't supposed to be a problem, I understand there may be change of plans (we should be able to change our minds).
I think you're confusing a few things. You cannot make dynamic properties work with globals today. Globals change class default values and there are no class defaults for dynamic properties.[*]
There's a perfectly valid discussion to have about whether we should even have dynamic properties. It's certainly been a long time since they were introduced and they haven't made their way into all that many devices so it's reasonable to say that perhaps we'd be better off without them. I would not object to a patch series that moved properties to classes entirely provided it removed existing uses of dynamic properties and didn't just introduce yet another mechanism.
But compat properties as a concept could be made to work with dynamic properties. They would have to be evaluated after instance init. There's quite a few places they would end up touching I suspect.
Erm, sorry, that is already implemented in qemu.git!? instance_post_init by Eduardo plus glue by me.
Ah, even better then :-) Regards, Anthony Liguori
Andreas
Another point of confusion worth mention is legacy properties since this usually comes up in the discussion. Legacy properties (the properties that are set/get as strings) are something that we should try to avoid. They end up as strings on the wire and make it harder to write client code.
* I recognize that compat_props are implemented as globals. I'm really talking about the current implementation of globals, not the concept of -global which could be made with dynamic properties.
Regards,
Anthony Liguori
What I don't understand is the rejection of code that works, matches the style used by 200+ other source files, adds more useful introspectable information, done in the way that was suggested 16 months ago, because we have some rough idea about how a new grand design will look like in the far future.
[1] http://lists.gnu.org/archive/html/qemu-devel/2013-06/msg00990.html
-- Eduardo
-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Il 11/02/2014 19:57, Anthony Liguori ha scritto:
Erm, sorry, that is already implemented in qemu.git!? instance_post_init by Eduardo plus glue by me. Ah, even better then :-)
Still, the code is a bit ad hoc. Static properties would let us remove that code and just read dc->props arrays along the hierarchy as usual. Also, Igor reminded me offlist that right now we cannot dump the default values of the properties. With this series, one could extend the device-list-properties command with that information. Paolo
On Thu, Feb 06, 2014 at 05:57:38PM +0100, Andreas Färber wrote: [...]
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.
"Haswell" is named this way not only because it looks like Haswell, but also because it has useful features you are going to find only on Haswell, and you (probably) are not going to be able to run it on hosts older than Haswell. That's the main real-world application of CPU models: making sure the VMs can run on a specific subset of hosts. So, if you choose "Haswell", you are telling the management stack "I know this is going to run only on Haswell and newer CPUs".
That's why x2apic is being proposed as an exception: it can be enabled on any host, because it doesn't depend on host-side support. That's why I propose we enable it on CPU models that don't have x2apic in the real world.
(BTW, what is the relation between this subject and static properties? I was expecting this to be discussed in the other existing thread about x2apic)
Sorry if I replied to two different series at once, that was for "Enabling x2apic by default in KvM (was Re: [Qemu-devel] [PATCH] target-i386: enable x2apic by default on more recent CPU models)".
Which is connected to CPU models/subclasses in what those types are supposed to be good for if we define them.
Let's take an obvious example. Jan wants to emulate a legacy isapc machine with -cpu 486. Then it feels counterproductive to enable the latest and greatest features such a machine never had. If the user wants to have the latest and greatest features, she can choose a newer -cpu model which already has the flag today.
OK, trying to be pragmatic: First, my main assumption: we don't want to make CPU models look different in KVM and TCG mode, to keep the CPU model information APIs simple and sane. Based on that: * I understand you don't want to force Jan to use "-cpu 486,-x2apic" * I don't want to force libvirt to use "-cpu Opteron_G1,+x2apic" I don't care about not having x2apic on 486 and other older CPU models that are fully supported by TCG, and I believe people using TCG won't care about having x2apic on Opteron_G1 and newer in the near future. One day TCG may be able to run Opteron_G1 too. But I also believe that the QEMU interfaces and the libvirt code that handles CPU features will eventually get mature enough so libvirt will be less affected by QEMU's decision about feature flags in CPU models. When that happen, we will be able to disable x2apic on Opteron_G1 and let libvirt enable it explicitly. Considering that, isn't it a reasonable compromise to add x2apic to the CPU models that today are useful only for KVM? -- Eduardo
participants (4)
-
Andreas Färber -
Anthony Liguori -
Eduardo Habkost -
Paolo Bonzini