Re: [PATCH] 2/2: Fall back to plain Qemu if KVM is not present, report errors from libkvm

[Simon Kagstrom <simon.kagstrom-6Uc+yI3S9P8@public.gmane.org>]

At Tue, 14 Nov 2006 14:24:20 +0200,
Avi Kivity wrote:
> Yes, but it leads to code duplication (there's a qemu_vmalloc() outside 
> USE_KVM). Also, if some code prior to this depended on kvm_allowed, it 
> would be invalidated.
> 
> Best to set kvm_allowed once you're sure it's really allowed, and then 
> don't touch it.

I've attached a new patch below. It's still setting kvm_allowed to 0
in two places - if kvm_qemu_init() fails and if the -no-kvm option is
passed, but it now does a kind of "try-and-see" and sets it to zero if
it fails. This is done regardless of if -no-kvm is passed or not.

I split initialization and creation of a vm in qemu-kvm.c.

The code duplication was there before, and I have not changed that.

> >> at present, libkvm_init() can just call access(2) on /dev/kvm.  In the 
> >> future, it is planned that open("/dev/kvm") will not create a VM (it 
> >> just opens a communication channel to the driver), so that libkvm_init() 
> >> will open the device and store the fd.
> >>     
> >
> > Personally, I would prefer something like
> >
> >    int kvm_open(kvm_t *kvm;)
> >    void kvm_close(kvm_t *kvm);
> >
> >    int kvm_create_vm(kvm_t *kvm, kvm_context_t *vm);
> >    void kvm_destroy_vm(kvm_t *kvm, kvm_context_t *vm);
> >   
> 
> That means that if sizeof(kvm_t) changes, we need to recompile 
> everything. With kvm_context_t and kvm_t as opaque pointers, we preserve 
> binary compatibility when their definitions change.

Sorry, true.

I was actually mostly thinking of the naming (I prefer the simpler
kvm_t).

// Simon

diff -u ../orig/kvm-2/qemu/qemu-kvm.c qemu/qemu-kvm.c
--- ../orig/kvm-2/qemu/qemu-kvm.c	2006-11-05 10:18:52.000000000 +0100
+++ qemu/qemu-kvm.c	2006-11-14 13:56:35.000000000 +0100
@@ -450,10 +450,30 @@
     .io_window = kvm_io_window,
 };
 
-void kvm_qemu_init()
+int kvm_qemu_init()
 {
+    /* Try to initialize kvm */
     kvm_context = kvm_init(&qemu_kvm_ops, saved_env);
-    kvm_create(kvm_context, phys_ram_size, (void**)&phys_ram_base);
+    if (!kvm_context) {
+      	return -1;
+    }
+
+    return 0;
+}
+
+int kvm_qemu_create_context(void)
+{
+    if (kvm_create(kvm_context, phys_ram_size, (void**)&phys_ram_base) < 0) {
+	kvm_qemu_destroy();
+	return -1;
+    }
+
+    return 0;
+}
+
+void kvm_qemu_destroy(void)
+{
+    kvm_finalize(kvm_context);
 }
 
 int kvm_update_debugger(CPUState *env)
diff -u ../orig/kvm-2/qemu/qemu-kvm.h qemu/qemu-kvm.h
--- ../orig/kvm-2/qemu/qemu-kvm.h	2006-10-16 11:01:02.000000000 +0200
+++ qemu/qemu-kvm.h	2006-11-14 13:55:56.000000000 +0100
@@ -3,7 +3,9 @@
 
 #include "kvmctl.h"
 
-void kvm_qemu_init(void);
+int kvm_qemu_init(void);
+int kvm_qemu_create_context(void);
+void kvm_qemu_destroy(void);
 void kvm_load_registers(CPUState *env);
 int kvm_cpu_exec(CPUState *env);
 int kvm_update_debugger(CPUState *env);
diff -u ../orig/kvm-2/qemu/vl.c qemu/vl.c
--- ../orig/kvm-2/qemu/vl.c	2006-11-05 10:18:52.000000000 +0100
+++ qemu/vl.c	2006-11-14 13:56:32.000000000 +0100
@@ -5706,6 +5706,10 @@
 
     nb_nics = 0;
     /* default mac address of the first network interface */
+#if USE_KVM
+    if (kvm_qemu_init() < 0)
+	kvm_allowed = 0;
+#endif
     
     optind = 1;
     for(;;) {
@@ -6099,11 +6103,15 @@
     }
 
     /* init the memory */
-#if USE_KVM
     phys_ram_size = ram_size + vga_ram_size + bios_size;
+#if USE_KVM
+    /* Initialize kvm */
     if (kvm_allowed) {
 	    phys_ram_size += KVM_EXTRA_PAGES * 4096;
-	    kvm_qemu_init();
+	    if (kvm_qemu_create_context() < 0) {
+		    fprintf(stderr, "Could not create KVM context\n");
+		    exit(1);
+	    }
     } else {
 	    phys_ram_base = qemu_vmalloc(phys_ram_size);
 	    if (!phys_ram_base) {
@@ -6112,7 +6120,6 @@
 	    }
     }
 #else
-    phys_ram_size = ram_size + vga_ram_size + bios_size;
     phys_ram_base = qemu_vmalloc(phys_ram_size);
     if (!phys_ram_base) {
         fprintf(stderr, "Could not allocate physical memory\n");

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642