On Tue, 26 Feb 2013 14:06:05 -0300 Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Tue, Feb 26, 2013 at 05:39:02PM +0100, Igor Mammedov wrote: > [...] > > > > > > > * I don't expect hv-* to appear on a machine-type compat_props > > > > > > > array in the near feature. > > > > > > > * I don't expect people to need to set per-CPU hv-* properties > > > > > > > on device_add for CPU hotplug. > > > > > > > > > > > > > > So we could keep them as special cases on parse_featurestr(), > > > > > > > and convert them to per-CPU properties only after we have the > > > > > > > subclasses and CPU hotplug working. > > > > > > it won't be a consistent interface, where user who has > > > > > > "-cpu XXX,+foo1,+hv_spinlocks,+foo2" on cmd-line > > > > > > would have to use "device_add XXX,foo1=true,foo2=true" manually > > > > > > excluding options from device_add, i.e. it propagates special > > > > > > casing to users as well. And when hv_ are moved to per-CPU > > > > > > fields, it might break users since they will still exclude some > > > > > > options. > > > > > > > > > > Won't -cpu/parse_featurestr() simply set global properties? In this > > > > > case, the common case would be to call "device_add XXX" with no > > > > > extra options at all, so there's no option to be excluded and no > > > > > special case to care about. > > > > That is if global properties will made to 1.5 which I highly doubt > > > > taking in account how fast patches are reviewed and accepted. > > > > Otherwise release would be broken. > > > > > > IMO it _has_ to make 1.5 and is a requirement to make device_add usable > > > for CPU hotplug. Otherwise we would have to change the behavior of -cpu > > > + device_add in an incompatible way. > > if all -cpu features are converted to static properties, we do not have to > > have global properties working. In absence of 'global properties', user > > will have to use the same properties at device_add that was specified at > > -cpu. And introduction of global properties won't regress it, it will > > just allow to use device_add without features specified on -cpu > > Strictly, we do not have to, but changing -cpu to set global properties > only later would change the behavior of "-cpu XXX,foo=1,bar=2" followed > by "device_add XXX" in an incompatible way. So if our long-term plan is Could you explain how ^^^ it will be incompatible, pls? > to make "-cpu" change the global-properties/defaults, we need to do it > since the beginning. > > > > > > > > > > > > > > > Considering that -cpu options will be translated to global > > > > > > > properties, it will be trivial to keep compatibility with > > > > > > > existing behavior of "-cpu hv_*=..." once we change them from > > > > > > > static variables to per-CPU fields. > > > > > > Global properties would just allow not to specify foo1,foo2 on > > > > > > device_add from hot-plug POV. > > > > > > > > > > > > If this and following patch are to complex we could fallback to > > > > > > alternative from v6 for hv_* features, which produce the same > > > > > > external property interface just with different internal approach. > > > > > > > > > > No, v6 exposes completely different (and unexpected) semantics. It > > > > > exposes a per-CPU property that affects all CPU objects when it gets > > > > > changed. > > > > Do you know any guest that will work with mixed CPUs that have and > > > > doesn't have hv_* set at the same time? > > > > > > I don't know and I don't care. But in either case you are introducing a > > > per-CPU QOM property for it. If it is a per-CPU QOM property, it just > > > makes sense that it will affect only the CPU object being changed. > > > > > > What I am proposing is to make it a per-CPU QOM property only after we > > > make it really per-CPU, so we don't introduce weird externally-visible > > > property semantics that are likely to change in the near future. > > Well this patch and next do it per-CPU as you asked, so no weird property > > semantics is introduced and no special-casing of anything is required. > > Yes, this patch does what I asked for, and I am not really against it. > :-) > > I am just wondering if we could do these changes later to make review > and testing easier. >
