On Tue, 26 Feb 2013 13:01:12 -0300 Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Tue, Feb 26, 2013 at 04:42:17PM +0100, Igor Mammedov wrote: > > On Tue, 26 Feb 2013 11:50:23 -0300 > > Eduardo Habkost <ehabk...@redhat.com> wrote: > > > > > On Mon, Feb 25, 2013 at 02:03:02AM +0100, Igor Mammedov wrote: > > > > - since hyperv_* helper functions are used only in target-i386/kvm.c > > > > move them there as static helpers > > > > > > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > > > > Requestd-By: Eduardo Habkost <ehabk...@redhat.com> > > > > > > I wonder if we could do this later, to make review and testing easier. > > > We could simply keep the hv_* handling inside parse_featurestr() by now. > > > I don't think this would block any of the CPU hotplug or CPU model > > > probing/compatibility/reliability work we're doing. I mean: > > device_add from CPU hot-add POV would be usable once we have all features > > accepted on -cpu converted to properties + sub-classes. > > If we keep hv_* as special cases inside parse_featurestr() (the way they > already are), CPU hotplug should still work perfectly. I don't see why > it would block it. > > > > > > > > > > * 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. > > > > > > > > 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? > > > > > > > > > --- > > > > target-i386/Makefile.objs | 2 +- > > > > target-i386/cpu.c | 16 +++++++--- > > > > target-i386/cpu.h | 7 +++++ > > > > target-i386/hyperv.c | 64 > > > > --------------------------------------------- target-i386/hyperv.h > > > > | 45 ------------------------------- target-i386/kvm.c | > > > > 36 ++++++++++++++++++------- 6 files changed, 45 insertions(+), 125 > > > > deletions(-) delete mode 100644 target-i386/hyperv.c > > > > delete mode 100644 target-i386/hyperv.h > > > > > > > [...] > > > > > > > >
