Re: [kvm-devel] [PATCH 1/2] KVM: Remove arch specific components from the general code

2007-07-29 Thread Avi Kivity
Gregory Haskins wrote:
 Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
   
Applied, thanks.

-- 
error compiling committee.c: too many arguments to function


-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/2] KVM: Remove arch specific components from the general code

2007-07-26 Thread Paul Turner
On 7/26/07, Anthony Liguori [EMAIL PROTECTED] wrote:
 Gregory Haskins wrote:
  Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
  ---
 
   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?

I have my series updated in this form also, I just got flooded and
have been too busy the last few days to repost.


   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/
 ___
 kvm-devel mailing list
 kvm-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/kvm-devel


-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/2] KVM: Remove arch specific components from the general code

2007-07-26 Thread Avi Kivity
Anthony Liguori wrote:
 Gregory Haskins wrote:
   
 Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
 ---

  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, 

I think this is the best plan, we're spinning too much on this 
janitorial stuff.

I'm only trying to ensure that this is the only patch that touches the 
entire world in the series.  Does anybody see why this should not be 
so?  Please holler asap.

 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.
   

I wouldn't recommend things like svm(vcpu)-blah generally, but for this 
use I think it is ok, especially for the smaller functions.

-- 
error compiling committee.c: too many arguments to function


-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/2] KVM: Remove arch specific components from the general code

2007-07-26 Thread Anthony Liguori
Gregory Haskins wrote:
 Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
 ---

  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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/2] KVM: Remove arch specific components from the general code

2007-07-26 Thread Gregory Haskins
On Thu, 2007-07-26 at 10:04 -0500, Anthony Liguori wrote:
 Gregory Haskins wrote:
  Signed-off-by: Gregory Haskins [EMAIL PROTECTED]
  ---
 
   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?

It will work either way, at Avi's discretion.

 
   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.

Heh...funny you should say that.  My first pass did exactly what you are
suggesting. :)  Then I felt as though the code was a little cleaner to
do it the way it stands now, so I changed it all.  I am not really
married to either form, so whatever people want is fine.




-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 1/2] KVM: Remove arch specific components from the general code

2007-07-26 Thread Rusty Russell
On Thu, 2007-07-26 at 10:04 -0500, Anthony Liguori wrote:
 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?

My patch is less mature, and am still arguing with Avi about the
details.

So I'll pull in Gregory's patch and try to work mine on top.  They
overlap somewhat, but mine should get rid of some of the warts,
including converting to container_of, and getting rid of valid.

   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 

Kernel convention seems to be converging on to_typename, so
to_vcpu_svm (although svm_vcpu rings better than vcpu_svm to me,
but that's minor).

static inline struct vcpu_svm *to_vcpu_svm(struct kvm_vcpu *vcpu)
{
#if LATER
return container_of(vcpu, struct vcpu_svm, vcpu);
#else
return vcpu-_priv;
#endif
}

Cheers,
Rusty.


-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel