Re: [PATCH 0 of 3] create kvm_x86

Carsten Otte
2007-11-21T09:39:02Z

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.