[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState |
Date: |
Tue, 26 Feb 2013 13:22:22 -0300 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Feb 26, 2013 at 05:12:21PM +0100, Igor Mammedov wrote:
> On Tue, 26 Feb 2013 13:01:12 -0300
> Eduardo Habkost <address@hidden> 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 <address@hidden> 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 <address@hidden>
> > > > > Requestd-By: Eduardo Habkost <address@hidden>
> > > >
> > > > 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.
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.
>
> >
> >
> > > >
> > > > 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.
>
> > >
> > > >
> > > > > ---
> > > > > 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
> > > > >
> > > > [...]
> > > >
> > >
> > >
> >
>
--
Eduardo
- [Qemu-devel] [PATCH qom-cpu-next 00/10 v7] target-i386: convert CPU features into properties, Igor Mammedov, 2013/02/24
- [Qemu-devel] [PATCH 01/10] qdev: add qdev property for bool type, Igor Mammedov, 2013/02/24
- [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Igor Mammedov, 2013/02/24
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Eduardo Habkost, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Igor Mammedov, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Eduardo Habkost, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Igor Mammedov, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState,
Eduardo Habkost <=
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Igor Mammedov, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Eduardo Habkost, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Igor Mammedov, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Eduardo Habkost, 2013/02/26
- Re: [Qemu-devel] [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState, Igor Mammedov, 2013/02/26
- [Qemu-devel] cpu_set vs device_add (was Re: [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState), Eduardo Habkost, 2013/02/26
- Re: [Qemu-devel] cpu_set vs device_add (was Re: [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState), Igor Mammedov, 2013/02/26
- Re: [Qemu-devel] cpu_set vs device_add (was Re: [PATCH 03/10] target-i386: move hyperv_* static globals to CPUState), Eduardo Habkost, 2013/02/26
[Qemu-devel] [PATCH 02/10] target-i386: cpu: convert existing dynamic properties into static properties, Igor Mammedov, 2013/02/24
[Qemu-devel] [PATCH 07/10] target-i386: convert 'check' and 'enforce' to static properties, Igor Mammedov, 2013/02/24