On Wed, Aug 01, 2012 at 03:41:56PM -0500, Anthony Liguori wrote: > Eduardo Habkost <ehabk...@redhat.com> writes: > > > On Wed, Aug 01, 2012 at 10:04:50PM +0200, Andreas Färber wrote: > >> Am 01.08.2012 20:45, schrieb Eduardo Habkost: > >> > This makes the change we discussed on the latest KVM conf call[1], > >> > moving the > >> > existing cpudefs from cpus-x86_64.conf to the C code. > >> > > >> > The config file data was converted to C using a script, available at: > >> > https://gist.github.com/3229602 > >> > > >> > Except by the extra square brackets around the CPU model names > >> > (indicating they > >> > are built-in models), the output of "-cpu ?dump" is exactly the same > >> > before and > >> > after applying this series. > >> > > >> > [1] http://article.gmane.org/gmane.comp.emulators.kvm.devel/95328 > >> > > >> > Eduardo Habkost (3): > >> > i386: add missing CPUID_* constants > >> > move CPU models from cpus-x86_64.conf to C > >> > eliminate cpus-x86_64.conf file > >> > >> If we do this, we will need to refactor the C code again for CPU > >> subclasses. Can't we (you) do that in one go then? :-) > > > > Why again? The refactor for classes would be a one-time mechanical > > process, won't it? > > > > Anyway, I wouldn't do it in a single step. I prefer doing things one > > small step at a time. > > I tend to agree. > > >> I thought there was a broad consensus not to go my declarative > >> X86CPUInfo way but to initialize CPUs imperatively as done for ARM. > >> That would mean transforming your > >> > >> + { > >> + .family = 6, > >> + .model = 2, > >> ... > >> > >> into an initfn doing > >> > >> - { > >> +static void conroe_initfn(Object *obj) > >> +{ > >> + X86CPU *cpu = X86_CPU(obj); > >> + CPUX86State *env = &cpu->env; > >> + > >> - .family = 6, > >> + env->family = 6; > >> - .model = 2, > >> + env->model = 2; > >> ... > >> > >> or so (not all being as nicely aligned). > > > > Really? I am surprised that this is the consensus. > > Andreas is absolutely correct here. Obviously, it's always better to > represent things as data instead of code. The basic problem is that > this can't be easily represented as data (particularly when you start > getting into compat properties).
I agree that sometimes we can't represent everything easily as data and some code is required. I just don't see yet why that would be the case for the CPU model data. The fact that we're insisting in using global properties to implement the compat code is a demonstration that it can be completely data-driven, isn't it? We also need to expose some of the CPU model information to libvirt, so we will surely need to keep some of that information available as data. > > > Why would one want to > > transform machine-friendly data into a large set of opaque functions > > that look all the same and contains exactly the same data inside it, but > > in a too verbose, machine-unfriendly and refactor-unfriendly way? It > > doesn't make sense to me. > > > > I will look for previous discussions to try to understand the reason for > > that (was this discussed on qemu-devel?). Do you have any pointers > > handy? > > > > > >> > >> Unfortunately the move of the CPU definitions from config file to C does > >> not eliminate the issue that users can still specify CPU models in their > >> own config files or from the command line, right? Doesn't that mean that > >> either we need to keep all CPU definitions declarative as done here and > >> live with any field duplication between X86CPUInfo and X86CPUClass, or > >> have a special intermediate subclass UserX86CPUClass with such fields > >> for user-specified -cpudef models? > >> Assuming, dropping -cpudef for 1.2 is still not an option. > > > > I would be happy to drop support for cpudef config sections > > completely. > > Please add docs to your patch about -cpudef being deprecated. We'll remove > it for 1.3 provided no one screams. I'll do. > > I'll include something in the 1.2 release notes and ask anyone to > contact qemu-devel if they depend on this feature. > > We haven't really done this in the past, but I don't see a reasonable > way to keep supporting -cpudef moving forward. > > Regards, > > Anthony Liguori > > > The only problem is compatibility, but personally I wouldn't mind > > breaking it (I don't think many users have been writing their own > > cpudefs). > > > > Anyway, if we keep supporting it for compatibility (it could be simply > > one separated CPU subclass that uses the config file data), at least we > > don't have the need for versioning/compatibility mechanisms for > > user-provided cpudefs. > > > >> > >> Regards, > >> Andreas > >> > > > > -- > > Eduardo -- Eduardo
