From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH 1/2] KVM: Remove arch specific components from the general code Date: Thu, 26 Jul 2007 10:04:54 -0500 Message-ID: <46A8B816.7080303@codemonkey.ws> References: <20070726144602.4847.64724.stgit@novell1.haskins.net> <20070726145204.4847.53350.stgit@novell1.haskins.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Gregory Haskins Return-path: In-Reply-To: <20070726145204.4847.53350.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@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 Gregory Haskins wrote: > Signed-off-by: Gregory Haskins > --- > > drivers/kvm/kvm.h | 31 ----- > drivers/kvm/kvm_main.c | 26 +--- > drivers/kvm/kvm_svm.h | 3 > drivers/kvm/svm.c | 322 +++++++++++++++++++++++++----------------------- > drivers/kvm/vmx.c | 236 +++++++++++++++++++++-------------- > 5 files changed, 320 insertions(+), 298 deletions(-) > > struct kvm_vcpu { > + int valid; > struct kvm *kvm; > int vcpu_id; > - union { > - struct vmcs *vmcs; > - struct vcpu_svm *svm; > - }; > + void *_priv; > How are you planning on going about switching to container_of()? Commit this, commit Rusty's stuff, then commit a fix or commit Rusty's stuff, then update your patch set? > static void svm_inject_gp(struct kvm_vcpu *vcpu, unsigned error_code) > { > - vcpu->svm->vmcb->control.event_inj = SVM_EVTINJ_VALID | > + svm(vcpu)->vmcb->control.event_inj = SVM_EVTINJ_VALID | > SVM_EVTINJ_VALID_ERR | > SVM_EVTINJ_TYPE_EXEPT | > GP_VECTOR; > - vcpu->svm->vmcb->control.event_inj_err = error_code; > + svm(vcpu)->vmcb->control.event_inj_err = error_code; > } I'm willing to concede on using the name "svm()" here although I think it's a terrible function name but I really think it's important to store a reference to this instead of using it as if it's an lvalue. So I would change this to: struct vcpu_svm *svm = svm(vcpu); svm->vmcb->control.event_inj = ....; I think this is much easier to grok than having svm(vcpu) calls all over the place as psuedo-lvalues. Regards, Anthony Liguori ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/