simplify creation of kernel irqchip and pit

Commit Message

Glauber Costa July 24, 2009, 5:57 a.m. UTC

Creation of those devices can be a lot simpler. In particular,
if we start by assuming they will be on, and disabling later if
anything fails (user request counts as failing here), we can remove
two variables that really serves no reason.

Signed-off-by: Glauber Costa <[e-mail]>
---
 qemu-kvm-x86.c |   33 +++++++++++++++-------------
 qemu-kvm.c     |   65 +++++++++++++++++++++++++-------------------------------
 qemu-kvm.h     |    4 ---
 3 files changed, 47 insertions(+), 55 deletions(-)

Comments

Avi Kivity July 29, 2009, 12:01 p.m. UTC | #1

On 07/24/2009 08:57 AM, Glauber Costa wrote:
> Creation of those devices can be a lot simpler. In particular,
> if we start by assuming they will be on, and disabling later if
> anything fails (user request counts as failing here), we can remove
> two variables that really serves no reason.
>    

The patch introduces mixed indentation.  Please stick with one style or 
another.

Avi Kivity July 29, 2009, 1:49 p.m. UTC | #2

On 07/29/2009 04:50 PM, Glauber Costa wrote:
> On Wed, Jul 29, 2009 at 03:01:56PM +0300, Avi Kivity wrote:
>    
>> On 07/24/2009 08:57 AM, Glauber Costa wrote:
>>      
>>> Creation of those devices can be a lot simpler. In particular,
>>> if we start by assuming they will be on, and disabling later if
>>> anything fails (user request counts as failing here), we can remove
>>> two variables that really serves no reason.
>>>
>>>        
>> The patch introduces mixed indentation.  Please stick with one style or
>> another.
>>      
> Are you willin go "apply" my patch that changes everything to four spaces?
> If you are, I'm only resending things after that.
>    

I'll look at it.  I'm a bit wary about this, it may produce even 
brokener indentation.  Perhaps a run through indent would help.

Glauber Costa July 29, 2009, 1:50 p.m. UTC | #3

On Wed, Jul 29, 2009 at 03:01:56PM +0300, Avi Kivity wrote:
> On 07/24/2009 08:57 AM, Glauber Costa wrote:
>> Creation of those devices can be a lot simpler. In particular,
>> if we start by assuming they will be on, and disabling later if
>> anything fails (user request counts as failing here), we can remove
>> two variables that really serves no reason.
>>    
>
> The patch introduces mixed indentation.  Please stick with one style or  
> another.
Are you willin go "apply" my patch that changes everything to four spaces?
If you are, I'm only resending things after that.
>
> -- 
> error compiling committee.c: too many arguments to function
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 492dbc5..0a42455 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -116,24 +116,27 @@  static int kvm_init_identity_map_page(kvm_context_t kvm)
 
 static int kvm_create_pit(kvm_context_t kvm)
 {
+	int r = 0;
 #ifdef KVM_CAP_PIT
-	int r;
 
-	kvm->pit_in_kernel = 0;
-	if (!kvm->no_pit_creation) {
-		r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
-		if (r > 0) {
-			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_PIT);
-			if (r >= 0)
-				kvm->pit_in_kernel = 1;
-			else {
-				fprintf(stderr, "Create kernel PIC irqchip failed\n");
-				return r;
-			}
-		}
-	}
+    if (!qemu_kvm_pit_in_kernel())
+        return 0;
+
+    r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
+    if (r < 0) {
+        goto disable_pit;
+    }
+    r = kvm_vm_ioctl(kvm_state, KVM_CREATE_PIT);
+    if (r < 0) {
+        fprintf(stderr, "Create kernel PIC irqchip failed\n");
+        goto disable_pit;
+    }
+
+    return 0;
+disable_pit:
+    kvm->pit_in_kernel = 0;
 #endif
-	return 0;
+    return r;
 }
 
 int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 9d550d3..bca256c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -382,6 +382,7 @@  int kvm_dirty_pages_log_enable_all(kvm_context_t kvm)
 	if (kvm->dirty_pages_log_all)
 		return 0;
 	kvm->dirty_pages_log_all = 1;
+
 	return kvm_dirty_pages_log_change_all(kvm,
 					      kvm_dirty_pages_log_enable_slot);
 }
@@ -439,8 +440,6 @@  int kvm_init(int smp_cpus)
 	kvm_state->vmfd = -1;
 	kvm_context->opaque = cpu_single_env;
 	kvm_context->dirty_pages_log_all = 0;
-	kvm_context->no_irqchip_creation = 0;
-	kvm_context->no_pit_creation = 0;
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     TAILQ_INIT(&kvm_state->kvm_sw_breakpoints);
@@ -480,16 +479,6 @@  static void kvm_finalize(KVMState *s)
 	free(s);
 }
 
-void kvm_disable_irqchip_creation(kvm_context_t kvm)
-{
-	kvm->no_irqchip_creation = 1;
-}
-
-void kvm_disable_pit_creation(kvm_context_t kvm)
-{
-	kvm->no_pit_creation = 1;
-}
-
 kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
 {
 	long mmap_size;
@@ -575,28 +564,35 @@  static int kvm_create_default_phys_mem(kvm_context_t kvm,
 
 void kvm_create_irqchip(kvm_context_t kvm)
 {
-	int r;
+	int r = 0;
 
-	kvm->irqchip_in_kernel = 0;
 #ifdef KVM_CAP_IRQCHIP
-	if (!kvm->no_irqchip_creation) {
-		r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_IRQCHIP);
-		if (r > 0) {	/* kernel irqchip supported */
-			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_IRQCHIP);
-			if (r >= 0) {
-				kvm->irqchip_inject_ioctl = KVM_IRQ_LINE;
+	if (!qemu_kvm_irqchip_in_kernel()) {
+        return;
+    }
+
+    r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_IRQCHIP);
+    if (r < 0) {
+        goto disable_irqchip;
+    }
+
+    r = kvm_vm_ioctl(kvm_state, KVM_CREATE_IRQCHIP);
+    if (r < 0) {
+		fprintf(stderr, "Create kernel PIC irqchip failed\n");
+        goto disable_irqchip;
+    }
+
+	kvm->irqchip_inject_ioctl = KVM_IRQ_LINE;
 #if defined(KVM_CAP_IRQ_INJECT_STATUS) && defined(KVM_IRQ_LINE_STATUS)
-				r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION,
-			  KVM_CAP_IRQ_INJECT_STATUS);
-				if (r > 0)
-					kvm->irqchip_inject_ioctl = KVM_IRQ_LINE_STATUS;
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_IRQ_INJECT_STATUS);
+	if (r > 0) {
+		kvm->irqchip_inject_ioctl = KVM_IRQ_LINE_STATUS;
+    }
 #endif
-				kvm->irqchip_in_kernel = 1;
-			}
-			else
-				fprintf(stderr, "Create kernel PIC irqchip failed\n");
-		}
-	}
+    return;
+
+disable_irqchip:
+    kvm->irqchip_in_kernel = 0;
 #endif
 }
 
@@ -2169,12 +2165,9 @@  static int kvm_create_context()
 {
     int r;
 
-    if (!kvm_irqchip) {
-        kvm_disable_irqchip_creation(kvm_context);
-    }
-    if (!kvm_pit) {
-        kvm_disable_pit_creation(kvm_context);
-    }
+    kvm_context->irqchip_in_kernel = kvm_irqchip;
+    kvm_context->pit_in_kernel = kvm_pit;
+
     if (kvm_create(kvm_context, 0, NULL) < 0) {
        kvm_finalize(kvm_state);
        return -1;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index b186c9d..5eb1ee5 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -55,14 +55,10 @@  struct kvm_context {
 	void *opaque;
 	/// is dirty pages logging enabled for all regions or not
 	int dirty_pages_log_all;
-	/// do not create in-kernel irqchip if set
-	int no_irqchip_creation;
 	/// in-kernel irqchip status
 	int irqchip_in_kernel;
 	/// ioctl to use to inject interrupts
 	int irqchip_inject_ioctl;
-	/// do not create in-kernel pit if set
-	int no_pit_creation;
 	/// in-kernel pit status
 	int pit_in_kernel;
 	/// in-kernel coalesced mmio