From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 0 of 3] create kvm_x86 Date: Wed, 21 Nov 2007 11:18:23 +0200 Message-ID: <4743F7DF.4000107@qumranet.com> References: <4743F5AE.8090707@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, kvm-ppc-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Xiantao , Hollis Blanchard To: carsteno-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org Return-path: In-Reply-To: <4743F5AE.8090707-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Carsten Otte wrote: > Hollis Blanchard wrote: > >> These patches are based on Xiantao's work to create struct kvm_x86. Patch 1 replaces his "KVM Portability split: Splitting kvm structure (V2)", and patches 2 and 3 build on it. >> > Looks like a clean approach with to to_kvm_x86 macro. Whole series: > Acked-by: Carsten Otte > > 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? -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- 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/