Re: [PATCH 0 of 3] create kvm_x86

[Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>]

Avi Kivity wrote:
> Well, I hate to say it, but the resulting code doesn't look too well
> (all the kvm_x86 variables), and it's entirely my fault as I recommended
> this approach.  Not like it was difficult to predict.
> 
> I'm thinking again of
> 
>     struct kvm {
>         struct kvm_arch a;
>         ...
>     }
> 
> Where each arch defines its own kvm_arch.  Now the changes look like a
> bunch of "kvm->blah" to "kvm->a.blah" conversions.
> 
> IIRC a downside was mentioned that it is easier to cause a build failure
> for another arch now.
> 
> Opinions?  In theory correctness should win over style every time, no?
It's a matter of taste. I favor embedding an kvm_arch too, but I 
recommend to name member "a" different. "arch" maybe?

An advantage of this solution is, that some (not all) architectures 
can share common code. If kvm_arch contains member foo for x86 and 
ia64, a common function could do kvm->a.foo. With the posted approach, 
we'd have
#ifdef CONFIG_ARCH_X86
	val = to_kvm_x86(kvm).foo;
#else
	val = to_kvm_ia74(kvm).foo;
#endif


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/