Re: [Qemu-arm] [Qemu-devel] [PATCH RFC 0/8] cpus: make "-cpu cpux, features" global properties

[David Hildenbrand]

> On Thu, Jun 02, 2016 at 10:44:49PM +0200, David Hildenbrand wrote:
> > > Current CLI option -cpu cpux,features serves as template
> > > for all created cpus of type: cpux. However QEMU parses
> > > "features" every time it creates a cpu instance and applies
> > > them to it while doing parsing.
> > > 
> > > That doesn't work well with -device/device_add infrastructure
> > > as it has no idea about cpu specific hooks that's used for
> > > parsing "features".
> > > In order to make -device/device_add utilize "-cpu features"
> > > template convert it into a set of global properties, so that
> > > every new CPU created will have them applied automatically.
> > > 
> > > That also allows to parse features only once, instread of
> > > doing it for every CPU instance created.  
> > 
> > While I think this makes sense for most cases, we (s390x) are
> > currently working on a mechanism to compare and baseline cpu models via
> > a qmp interface, to not have to replicate CPU models in libvirt
> > every time we do some changes.
> > 
> > To do that, we are creating temporary CPUs to handle the model
> > parsing. So, with our current prototype, we rely on the mechanism
> > to parse properties multiple time, as we are really creating
> > different CPUs.  
> 
> This series only changes the code that exists for parsing the
> -cpu option, and nothing else. Is this (the code that parses
> -cpu) really what you need to reuse?

I was reading "every new CPU created will have them applied automatically".
If I was having a basic understanding problem here, very good :)

The problematic part is when the properties are applied where the
"changed" data is stored (class. vs. instance).

e.g. in terms of s390x: z13 includes both vx and tx
-cpu z13,vx=off,tx=off

Now, what would happen on
a) device_add z13-s390-cpu // I assume vx=off, tx=off ?

b) device_add z13-s390-cpu,vx=on // vx=on suddenly for all vcpus or one
instance? I assume just this instance

c) device_add zBC12-s390-cpu // will I suddenly get a z13?
Or a zBC12 without tx and vx? I assume the latter.

d) object_new("z13-s390-cpu")); // will I get a clean z13 with tx and vx on?

d) has to work for us. Otherwise we will have to fallback to manual
property parsing.

> 
> If all you need is to parse properties, why can't you reuse the
> existing QOM/Device mechanisms to handle properties (the one used
> by -device and device_add), instead of the -cpu code?

We can, if my given example works. And the global properties
don't interfere with cpus.

> 
> We need to use less of the infrastructure that exists for the
> legacy -cpu option (and use more of the generic QOM/Device
> mechanisms), not more of it.

It is better to have one way of creating cpus that two.

> 
> 
> > 
> > While we could somehow change our mechanism I don't think this is
> > the right thing to do.
> >   
> 
> If reusing the existing parsing code is something you absolutely
> need, we could split the process in two parts: 1) converting the
> feature string to a list of property=value pairs; 2) registering
> the property=value pairs as global properties. Then you coulde
> reuse (1) only. But do you really need to reuse the parser for
> the legacy -cpu option in your mechanism?

It's really not about the parser, more about the global properties.

> 
> > We will have to support heterogeneous cpu models (I think arm was one of
> > the guys requesting this if I'm not mistaking) and it somehow
> > contradicts to the general mechanism of device_add fully specifying
> > parameters. These would now be implicit parameters.  
> 
> The -cpu interface really does contradict the general mechanism
> of device_add. This whole series is about translating the
> handling of -cpu to a more generic mechanism (-global), to allow
> us to deprecate -cpu in the future. Why is that a bad thing?

It is a bad thing as soon as they affect other devices.
If I did a -cpu z13,tx=off, I don't expect

a) a hot-plugged z13 to suddenly have tx=off
b) a hot-plugged zBC12 to suddenly have tx off

Won't libvirt have to specify the cpu name either way in device-add?
And your plan seems to be that the properties are suddenly implicit.
I don't see a problem with libvirt having to specify the properties
manually on device add.

I agree, cleaning up the parsing function indeed makes sense.

David