Re: [Qemu-devel] [PATCH RFC 1/8] target-i386: cpu: move features logic that requires CPUState to realize time

[Eduardo Habkost]

On Thu, Jun 02, 2016 at 11:59:30AM +0200, Igor Mammedov wrote:
> On Wed, 1 Jun 2016 14:43:09 -0300
> Eduardo Habkost <address@hidden> wrote:
> [...]
> 
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index 3fbc6f3..6159a7f 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -1932,6 +1932,11 @@ static inline void feat2prop(char *s)
> > >      }
> > >  }
> > >  
> > > +/* Features to be added */
> > 
> > Please add something like "Features to be added. Will be replaced
> > by global variables in the future".
> > 
> > > +static FeatureWordArray plus_features = { 0 };
> > > +/* Features to be removed */
> > > +static FeatureWordArray minus_features = { 0 };
> > > +
> > 
> > I see that this hack is replaced by the following patches, but is
> > there an easy way to remove the CPUState argument from
> > x86_cpu_parse_featurestr() before we introduce these static
> > variables? (No problem if there's no way to do that, as long as
> > the static variables are explicitly documented as a temporary
> > hack)
> It's hack to keep legacy +- semantic (i.e. it overrides feat1=x,feat2)
> local to x86 that probably would stay here forever.
> I should add comment that explains why +- can't be replaced
> with normal properties.

Oh, I assumed it would be temporary. In that case, I would like
to avoid adding the static variables if possible.

> 
> I don't plan to replace plus/minus_features with anything nor to
> make this variables a global ones to spread +- x86/sparc legacy
> format everywhere.

Can't the +/- semantics be emulated by simply registering
plus_features/minus_features after the other global properties
are registered inside x86_cpu_parse_featurestr()?

>  
> What I would do though before enabling -device/device_add for X86CPU is
> to disable +- handling for new machine types so that CPUs would
> follow generic property semantic of device used everywhere else.

We can't do that unless we give libvirt (and users that have
their own scripts) time to adapt to the new syntax (and we warn
users that newer QEMU versions will require newer libvirt).

-- 
Eduardo