[PATCH 0/5] Further cleanups

[PATCH 0/5] Further cleanups

[Glauber Costa]

Hi guys,

I'm sending a series that moves further in reusing upstream code

Here I'm reusing kvm_vm_ioctl, kvm_ioctl, kvm_check_extension and
the cpuid bits for i386.

[PATCH 1/5] remove kvm types from handle unhandled

[Glauber Costa]

I'm in an ongoing process of not using kvm-specific types in function
declarations. handle_unhandled() is the first victim. Since we don't
really use this data, but just the reason, remove them entirely.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 355adf4..904e751 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -176,8 +176,7 @@ int kvm_mmio_write(void *opaque, uint64_t addr, uint8_t *data, int len)
 	return 0;
 }
 
-static int handle_unhandled(kvm_context_t kvm, kvm_vcpu_context_t vcpu,
-                            uint64_t reason)
+static int handle_unhandled(uint64_t reason)
 {
     fprintf(stderr, "kvm: unhandled exit %"PRIx64"\n", reason);
     return -EINVAL;
@@ -1085,12 +1084,10 @@ again:
 	if (1) {
 		switch (run->exit_reason) {
 		case KVM_EXIT_UNKNOWN:
-			r = handle_unhandled(kvm, vcpu,
-				run->hw.hardware_exit_reason);
+			r = handle_unhandled(run->hw.hardware_exit_reason);
 			break;
 		case KVM_EXIT_FAIL_ENTRY:
-			r = handle_unhandled(kvm, vcpu,
-				run->fail_entry.hardware_entry_failure_reason);
+			r = handle_unhandled(run->fail_entry.hardware_entry_failure_reason);
 			break;
 		case KVM_EXIT_EXCEPTION:
 			fprintf(stderr, "exception %d (%x)\n",
-- 
1.6.2.2

[PATCH 2/5] reuse kvm_vm_ioctl

[Glauber Costa]

Start using kvm_vm_ioctl's code.
For type safety, delete vm_fd from kvm_context entirely, so the
compiler can play along with us helping to detect errors I might
have made.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c      |    2 ++
 qemu-kvm-x86.c |   18 +++++++++---------
 qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
 qemu-kvm.h     |    6 +++---
 4 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 67908a7..9373d99 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -809,6 +809,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
 
     return ret;
 }
+#endif
 
 int kvm_vm_ioctl(KVMState *s, int type, ...)
 {
@@ -827,6 +828,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
     return ret;
 }
 
+#ifdef KVM_UPSTREAM
 int kvm_vcpu_ioctl(CPUState *env, int type, ...)
 {
     int ret;
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 350f272..c7393cd 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -40,7 +40,7 @@ int kvm_set_tss_addr(kvm_context_t kvm, unsigned long addr)
 
 	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
 	if (r > 0) {
-		r = ioctl(kvm->vm_fd, KVM_SET_TSS_ADDR, addr);
+		r = kvm_vm_ioctl(kvm_state, KVM_SET_TSS_ADDR, addr);
 		if (r == -1) {
 			fprintf(stderr, "kvm_set_tss_addr: %m\n");
 			return -errno;
@@ -82,7 +82,7 @@ static int kvm_create_pit(kvm_context_t kvm)
 	if (!kvm->no_pit_creation) {
 		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
 		if (r > 0) {
-			r = ioctl(kvm->vm_fd, KVM_CREATE_PIT);
+			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_PIT);
 			if (r >= 0)
 				kvm->pit_in_kernel = 1;
 			else {
@@ -211,7 +211,7 @@ int kvm_create_memory_alias(kvm_context_t kvm,
 		.memory_size = len,
 		.target_phys_addr = target_phys,
 	};
-	int fd = kvm->vm_fd;
+	int fd = kvm_state->vmfd;
 	int r;
 	int slot;
 
@@ -272,7 +272,7 @@ int kvm_get_pit(kvm_context_t kvm, struct kvm_pit_state *s)
 	int r;
 	if (!kvm->pit_in_kernel)
 		return 0;
-	r = ioctl(kvm->vm_fd, KVM_GET_PIT, s);
+	r = kvm_vm_ioctl(kvm_state, KVM_GET_PIT, s);
 	if (r == -1) {
 		r = -errno;
 		perror("kvm_get_pit");
@@ -285,7 +285,7 @@ int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s)
 	int r;
 	if (!kvm->pit_in_kernel)
 		return 0;
-	r = ioctl(kvm->vm_fd, KVM_SET_PIT, s);
+	r = kvm_vm_ioctl(kvm_state, KVM_SET_PIT, s);
 	if (r == -1) {
 		r = -errno;
 		perror("kvm_set_pit");
@@ -299,7 +299,7 @@ int kvm_get_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
 	int r;
 	if (!kvm->pit_in_kernel)
 		return 0;
-	r = ioctl(kvm->vm_fd, KVM_GET_PIT2, ps2);
+	r = kvm_vm_ioctl(kvm_state, KVM_GET_PIT2, ps2);
 	if (r == -1) {
 		r = -errno;
 		perror("kvm_get_pit2");
@@ -312,7 +312,7 @@ int kvm_set_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
 	int r;
 	if (!kvm->pit_in_kernel)
 		return 0;
-	r = ioctl(kvm->vm_fd, KVM_SET_PIT2, ps2);
+	r = kvm_vm_ioctl(kvm_state, KVM_SET_PIT2, ps2);
 	if (r == -1) {
 		r = -errno;
 		perror("kvm_set_pit2");
@@ -549,7 +549,7 @@ int kvm_set_shadow_pages(kvm_context_t kvm, unsigned int nrshadow_pages)
 	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
 		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
 	if (r > 0) {
-		r = ioctl(kvm->vm_fd, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
+		r = kvm_vm_ioctl(kvm_state, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
 		if (r == -1) {
 			fprintf(stderr, "kvm_set_shadow_pages: %m\n");
 			return -errno;
@@ -568,7 +568,7 @@ int kvm_get_shadow_pages(kvm_context_t kvm, unsigned int *nrshadow_pages)
 	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
 		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
 	if (r > 0) {
-		*nrshadow_pages = ioctl(kvm->vm_fd, KVM_GET_NR_MMU_PAGES);
+		*nrshadow_pages = kvm_vm_ioctl(kvm_state, KVM_GET_NR_MMU_PAGES);
 		return 0;
 	}
 #endif
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 904e751..f09bcca 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -44,7 +44,7 @@ int kvm_pit_reinject = 1;
 int kvm_nested = 0;
 
 
-static KVMState *kvm_state;
+KVMState *kvm_state;
 kvm_context_t kvm_context;
 
 pthread_mutex_t qemu_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -332,7 +332,7 @@ static int kvm_dirty_pages_log_change(kvm_context_t kvm,
 			mem.guest_phys_addr,
 			mem.memory_size,
 			mem.flags);
-		r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &mem);
+		r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &mem);
 		if (r == -1)
 			fprintf(stderr, "%s: %m\n", __FUNCTION__);
 	}
@@ -452,7 +452,7 @@ int kvm_init(int smp_cpus)
     kvm_context = &kvm_state->kvm_context;
 
 	kvm_context->fd = fd;
-	kvm_context->vm_fd = -1;
+	kvm_state->vmfd = -1;
 	kvm_context->opaque = cpu_single_env;
 	kvm_context->dirty_pages_log_all = 0;
 	kvm_context->no_irqchip_creation = 0;
@@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
 	vcpu_ctx->kvm = kvm;
 	vcpu_ctx->id = id;
 
-	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
+	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
 	if (r == -1) {
 		fprintf(stderr, "kvm_create_vcpu: %m\n");
 		goto err;
@@ -550,7 +550,7 @@ static int kvm_set_boot_vcpu_id(kvm_context_t kvm, uint32_t id)
 #ifdef KVM_CAP_SET_BOOT_CPU_ID
     int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_BOOT_CPU_ID);
     if (r > 0)
-        return ioctl(kvm->vm_fd, KVM_SET_BOOT_CPU_ID, id);
+        return kvm_vm_ioctl(kvm_state, KVM_SET_BOOT_CPU_ID, id);
     return -ENOSYS;
 #else
     return -ENOSYS;
@@ -571,7 +571,7 @@ int kvm_create_vm(kvm_context_t kvm)
 		fprintf(stderr, "kvm_create_vm: %m\n");
 		return -1;
 	}
-	kvm->vm_fd = fd;
+	kvm_state->vmfd = fd;
 	return 0;
 }
 
@@ -594,7 +594,7 @@ int kvm_check_extension(kvm_context_t kvm, int ext)
 {
 	int ret;
 
-	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, ext);
+	ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, ext);
 	if (ret > 0)
 		return ret;
 	return 0;
@@ -609,7 +609,7 @@ void kvm_create_irqchip(kvm_context_t kvm)
 	if (!kvm->no_irqchip_creation) {
 		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_IRQCHIP);
 		if (r > 0) {	/* kernel irqchip supported */
-			r = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
+			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_IRQCHIP);
 			if (r >= 0) {
 				kvm->irqchip_inject_ioctl = KVM_IRQ_LINE;
 #if defined(KVM_CAP_IRQ_INJECT_STATUS) && defined(KVM_IRQ_LINE_STATUS)
@@ -664,7 +664,7 @@ int kvm_register_phys_mem(kvm_context_t kvm,
 	DPRINTF("memory: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
 		memory.guest_phys_addr, memory.memory_size,
 		memory.userspace_addr, memory.slot, memory.flags);
-	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
+	r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &memory);
 	if (r == -1) {
 		fprintf(stderr, "create_userspace_phys_mem: %s\n", strerror(errno));
 		return -1;
@@ -710,7 +710,7 @@ void kvm_destroy_phys_mem(kvm_context_t kvm, unsigned long phys_start,
 		memory.guest_phys_addr,
 		memory.memory_size,
 		memory.flags);
-	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
+	r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &memory);
 	if (r == -1) {
 		fprintf(stderr, "destroy_userspace_phys_mem: %s",
 			strerror(errno));
@@ -741,7 +741,7 @@ static int kvm_get_map(kvm_context_t kvm, int ioctl_num, int slot, void *buf)
 
 	log.dirty_bitmap = buf;
 
-	r = ioctl(kvm->vm_fd, ioctl_num, &log);
+	r = kvm_vm_ioctl(kvm_state, ioctl_num, &log);
 	if (r == -1)
 		return -errno;
 	return 0;
@@ -794,7 +794,7 @@ int kvm_set_irq_level(kvm_context_t kvm, int irq, int level, int *status)
 		return 0;
 	event.level = level;
 	event.irq = irq;
-	r = ioctl(kvm->vm_fd, kvm->irqchip_inject_ioctl, &event);
+	r = kvm_vm_ioctl(kvm_state, kvm->irqchip_inject_ioctl, &event);
 	if (r == -1)
 		perror("kvm_set_irq_level");
 
@@ -816,7 +816,7 @@ int kvm_get_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
 
 	if (!kvm->irqchip_in_kernel)
 		return 0;
-	r = ioctl(kvm->vm_fd, KVM_GET_IRQCHIP, chip);
+	r = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
 	if (r == -1) {
 		r = -errno;
 		perror("kvm_get_irqchip\n");
@@ -830,7 +830,7 @@ int kvm_set_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
 
 	if (!kvm->irqchip_in_kernel)
 		return 0;
-	r = ioctl(kvm->vm_fd, KVM_SET_IRQCHIP, chip);
+	r = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip);
 	if (r == -1) {
 		r = -errno;
 		perror("kvm_set_irqchip\n");
@@ -1230,7 +1230,7 @@ int kvm_coalesce_mmio_region(target_phys_addr_t addr, ram_addr_t size)
 		zone.addr = addr;
 		zone.size = size;
 
-		r = ioctl(kvm->vm_fd, KVM_REGISTER_COALESCED_MMIO, &zone);
+		r = kvm_vm_ioctl(kvm_state, KVM_REGISTER_COALESCED_MMIO, &zone);
 		if (r == -1) {
 			perror("kvm_register_coalesced_mmio_zone");
 			return -errno;
@@ -1253,7 +1253,7 @@ int kvm_uncoalesce_mmio_region(target_phys_addr_t addr, ram_addr_t size)
 		zone.addr = addr;
 		zone.size = size;
 
-		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_COALESCED_MMIO, &zone);
+		r = kvm_vm_ioctl(kvm_state, KVM_UNREGISTER_COALESCED_MMIO, &zone);
 		if (r == -1) {
 			perror("kvm_unregister_coalesced_mmio_zone");
 			return -errno;
@@ -1271,7 +1271,7 @@ int kvm_assign_pci_device(kvm_context_t kvm,
 {
 	int ret;
 
-	ret = ioctl(kvm->vm_fd, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
+	ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
 	if (ret < 0)
 		return -errno;
 
@@ -1283,7 +1283,7 @@ static int kvm_old_assign_irq(kvm_context_t kvm,
 {
 	int ret;
 
-	ret = ioctl(kvm->vm_fd, KVM_ASSIGN_IRQ, assigned_irq);
+	ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_IRQ, assigned_irq);
 	if (ret < 0)
 		return -errno;
 
@@ -1298,7 +1298,7 @@ int kvm_assign_irq(kvm_context_t kvm,
 
 	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_ASSIGN_DEV_IRQ);
 	if (ret > 0) {
-		ret = ioctl(kvm->vm_fd, KVM_ASSIGN_DEV_IRQ, assigned_irq);
+		ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_DEV_IRQ, assigned_irq);
 		if (ret < 0)
 			return -errno;
 		return ret;
@@ -1312,7 +1312,7 @@ int kvm_deassign_irq(kvm_context_t kvm,
 {
 	int ret;
 
-	ret = ioctl(kvm->vm_fd, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
+	ret = kvm_vm_ioctl(kvm_state, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
 	if (ret < 0)
             return -errno;
 
@@ -1333,7 +1333,7 @@ int kvm_deassign_pci_device(kvm_context_t kvm,
 {
 	int ret;
 
-	ret = ioctl(kvm->vm_fd, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
+	ret = kvm_vm_ioctl(kvm_state, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
 	if (ret < 0)
 		return -errno;
 
@@ -1364,7 +1364,7 @@ int kvm_reinject_control(kvm_context_t kvm, int pit_reinject)
 
 	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_REINJECT_CONTROL);
 	if (r > 0) {
-		r = ioctl(kvm->vm_fd, KVM_REINJECT_CONTROL, &control);
+		r = kvm_vm_ioctl(kvm_state, KVM_REINJECT_CONTROL, &control);
 		if (r == -1)
 			return -errno;
 		return r;
@@ -1584,7 +1584,7 @@ int kvm_commit_irq_routes(kvm_context_t kvm)
 	int r;
 
 	kvm->irq_routes->flags = 0;
-	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, kvm->irq_routes);
+	r = kvm_vm_ioctl(kvm_state, KVM_SET_GSI_ROUTING, kvm->irq_routes);
 	if (r == -1)
 		r = -errno;
 	return r;
@@ -1616,7 +1616,7 @@ int kvm_assign_set_msix_nr(kvm_context_t kvm,
 {
         int ret;
 
-        ret = ioctl(kvm->vm_fd, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
+        ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
         if (ret < 0)
                 return -errno;
 
@@ -1628,7 +1628,7 @@ int kvm_assign_set_msix_entry(kvm_context_t kvm,
 {
         int ret;
 
-        ret = ioctl(kvm->vm_fd, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
+        ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
         if (ret < 0)
                 return -errno;
 
@@ -1649,7 +1649,7 @@ static int _kvm_irqfd(kvm_context_t kvm, int fd, int gsi, int flags)
 		.flags = flags,
 	};
 
-	r = ioctl(kvm->vm_fd, KVM_IRQFD, &data);
+	r = kvm_vm_ioctl(kvm_state, KVM_IRQFD, &data);
 	if (r == -1)
 		r = -errno;
 	return r;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index d5291a3..f27da7c 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -54,9 +54,6 @@ extern int kvm_abi;
 struct kvm_context {
 	/// Filedescriptor to /dev/kvm
 	int fd;
-	int vm_fd;
-	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
-	struct kvm_callbacks *callbacks;
 	void *opaque;
 	/// is dirty pages logging enabled for all regions or not
 	int dirty_pages_log_all;
@@ -1180,5 +1177,8 @@ typedef struct KVMState
     struct kvm_context kvm_context;
 } KVMState;
 
+extern KVMState *kvm_state;
+
+int kvm_vm_ioctl(KVMState *s, int type, ...);
 
 #endif
-- 
1.6.2.2

[PATCH 3/5] reuse kvm_ioctl

[Glauber Costa]

Start using kvm_ioctl's code.
For type safety, delete fd from kvm_context entirely, so the
compiler can play along with us helping to detect errors I might
have made.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm-all.c      |    2 +-
 qemu-kvm-x86.c |   18 +++++++++---------
 qemu-kvm.c     |   32 ++++++++++++++++----------------
 qemu-kvm.h     |    3 +--
 4 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9373d99..0ec6475 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -793,6 +793,7 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
     }
 }
 
+#endif
 int kvm_ioctl(KVMState *s, int type, ...)
 {
     int ret;
@@ -809,7 +810,6 @@ int kvm_ioctl(KVMState *s, int type, ...)
 
     return ret;
 }
-#endif
 
 int kvm_vm_ioctl(KVMState *s, int type, ...)
 {
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index c7393cd..fb3ac9a 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -38,7 +38,7 @@ int kvm_set_tss_addr(kvm_context_t kvm, unsigned long addr)
 #ifdef KVM_CAP_SET_TSS_ADDR
 	int r;
 
-	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
 	if (r > 0) {
 		r = kvm_vm_ioctl(kvm_state, KVM_SET_TSS_ADDR, addr);
 		if (r == -1) {
@@ -56,7 +56,7 @@ static int kvm_init_tss(kvm_context_t kvm)
 #ifdef KVM_CAP_SET_TSS_ADDR
 	int r;
 
-	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
 	if (r > 0) {
 		/*
 		 * this address is 3 pages before the bios, and the bios should present
@@ -80,7 +80,7 @@ static int kvm_create_pit(kvm_context_t kvm)
 
 	kvm->pit_in_kernel = 0;
 	if (!kvm->no_pit_creation) {
-		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
+		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)
@@ -384,7 +384,7 @@ struct kvm_msr_list *kvm_get_msr_list(kvm_context_t kvm)
 	int r, e;
 
 	sizer.nmsrs = 0;
-	r = ioctl(kvm->fd, KVM_GET_MSR_INDEX_LIST, &sizer);
+	r = kvm_ioctl(kvm_state, KVM_GET_MSR_INDEX_LIST, &sizer);
 	if (r == -1 && errno != E2BIG)
 		return NULL;
 	/* Old kernel modules had a bug and could write beyond the provided
@@ -393,7 +393,7 @@ struct kvm_msr_list *kvm_get_msr_list(kvm_context_t kvm)
 				       sizer.nmsrs * sizeof(*msrs->indices)));
 
 	msrs->nmsrs = sizer.nmsrs;
-	r = ioctl(kvm->fd, KVM_GET_MSR_INDEX_LIST, msrs);
+	r = kvm_ioctl(kvm_state, KVM_GET_MSR_INDEX_LIST, msrs);
 	if (r == -1) {
 		e = errno;
 		free(msrs);
@@ -546,7 +546,7 @@ int kvm_set_shadow_pages(kvm_context_t kvm, unsigned int nrshadow_pages)
 #ifdef KVM_CAP_MMU_SHADOW_CACHE_CONTROL
 	int r;
 
-	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION,
 		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
 	if (r > 0) {
 		r = kvm_vm_ioctl(kvm_state, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
@@ -565,7 +565,7 @@ int kvm_get_shadow_pages(kvm_context_t kvm, unsigned int *nrshadow_pages)
 #ifdef KVM_CAP_MMU_SHADOW_CACHE_CONTROL
 	int r;
 
-	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION,
 		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
 	if (r > 0) {
 		*nrshadow_pages = kvm_vm_ioctl(kvm_state, KVM_GET_NR_MMU_PAGES);
@@ -584,7 +584,7 @@ static int tpr_access_reporting(kvm_vcpu_context_t vcpu, int enabled)
 		.enabled = enabled,
 	};
 
-	r = ioctl(vcpu->kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_VAPIC);
+	r = ioctl(kvm_state->fd, KVM_CHECK_EXTENSION, KVM_CAP_VAPIC);
 	if (r == -1 || r == 0)
 		return -ENOSYS;
 	r = ioctl(vcpu->fd, KVM_TPR_ACCESS_REPORTING, &tac);
@@ -618,7 +618,7 @@ static struct kvm_cpuid2 *try_get_cpuid(kvm_context_t kvm, int max)
 	size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
 	cpuid = qemu_malloc(size);
 	cpuid->nent = max;
-	r = ioctl(kvm->fd, KVM_GET_SUPPORTED_CPUID, cpuid);
+	r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_CPUID, cpuid);
 	if (r == -1)
 		r = -errno;
 	else if (r == 0 && cpuid->nent >= max)
diff --git a/qemu-kvm.c b/qemu-kvm.c
index f09bcca..2654f91 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -227,7 +227,7 @@ static int get_free_slot(kvm_context_t kvm)
 	int tss_ext;
 
 #if defined(KVM_CAP_SET_TSS_ADDR) && !defined(__s390__)
-	tss_ext = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
+	tss_ext = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
 #else
 	tss_ext = 0;
 #endif
@@ -451,7 +451,7 @@ int kvm_init(int smp_cpus)
 	kvm_state = qemu_mallocz(sizeof(*kvm_state));
     kvm_context = &kvm_state->kvm_context;
 
-	kvm_context->fd = fd;
+	kvm_state->fd = fd;
 	kvm_state->vmfd = -1;
 	kvm_context->opaque = cpu_single_env;
 	kvm_context->dirty_pages_log_all = 0;
@@ -492,7 +492,7 @@ static void kvm_finalize(KVMState *s)
 	if (kvm->vm_fd != -1)
 		close(kvm->vm_fd);
 	*/
-	close(s->kvm_context.fd);
+	close(s->fd);
 	free(s);
 }
 
@@ -526,7 +526,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
     env->kvm_fd = r;
     env->kvm_state = kvm_state;
 
-	mmap_size = ioctl(kvm->fd, KVM_GET_VCPU_MMAP_SIZE, 0);
+	mmap_size = kvm_ioctl(kvm_state, KVM_GET_VCPU_MMAP_SIZE, 0);
 	if (mmap_size == -1) {
 		fprintf(stderr, "get vcpu mmap size: %m\n");
 		goto err_fd;
@@ -548,7 +548,7 @@ err:
 static int kvm_set_boot_vcpu_id(kvm_context_t kvm, uint32_t id)
 {
 #ifdef KVM_CAP_SET_BOOT_CPU_ID
-    int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_BOOT_CPU_ID);
+    int r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_SET_BOOT_CPU_ID);
     if (r > 0)
         return kvm_vm_ioctl(kvm_state, KVM_SET_BOOT_CPU_ID, id);
     return -ENOSYS;
@@ -559,7 +559,7 @@ static int kvm_set_boot_vcpu_id(kvm_context_t kvm, uint32_t id)
 
 int kvm_create_vm(kvm_context_t kvm)
 {
-	int fd = kvm->fd;
+	int fd = kvm_state->fd;
 
 #ifdef KVM_CAP_IRQ_ROUTING
 	kvm->irq_routes = qemu_mallocz(sizeof(*kvm->irq_routes));
@@ -580,7 +580,7 @@ static int kvm_create_default_phys_mem(kvm_context_t kvm,
 				       void **vm_mem)
 {
 #ifdef KVM_CAP_USER_MEMORY
-	int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_USER_MEMORY);
+	int r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_USER_MEMORY);
 	if (r > 0)
 		return 0;
 	fprintf(stderr, "Hypervisor too old: KVM_CAP_USER_MEMORY extension not supported\n");
@@ -607,13 +607,13 @@ void kvm_create_irqchip(kvm_context_t kvm)
 	kvm->irqchip_in_kernel = 0;
 #ifdef KVM_CAP_IRQCHIP
 	if (!kvm->no_irqchip_creation) {
-		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_IRQCHIP);
+		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 defined(KVM_CAP_IRQ_INJECT_STATUS) && defined(KVM_IRQ_LINE_STATUS)
-				r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
+				r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION,
 			  KVM_CAP_IRQ_INJECT_STATUS);
 				if (r > 0)
 					kvm->irqchip_inject_ioctl = KVM_IRQ_LINE_STATUS;
@@ -944,7 +944,7 @@ int kvm_get_mpstate(kvm_vcpu_context_t vcpu, struct kvm_mp_state *mp_state)
 {
     int r;
 
-    r = ioctl(vcpu->kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_MP_STATE);
+    r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_MP_STATE);
     if (r > 0)
         return ioctl(vcpu->fd, KVM_GET_MP_STATE, mp_state);
     return -ENOSYS;
@@ -954,7 +954,7 @@ int kvm_set_mpstate(kvm_vcpu_context_t vcpu, struct kvm_mp_state *mp_state)
 {
     int r;
 
-    r = ioctl(vcpu->kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_MP_STATE);
+    r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_MP_STATE);
     if (r > 0)
         return ioctl(vcpu->fd, KVM_SET_MP_STATE, mp_state);
     return -ENOSYS;
@@ -1190,7 +1190,7 @@ int kvm_has_sync_mmu(void)
 {
         int r = 0;
 #ifdef KVM_CAP_SYNC_MMU
-        r = ioctl(kvm_context->fd, KVM_CHECK_EXTENSION, KVM_CAP_SYNC_MMU);
+        r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_SYNC_MMU);
 #endif
         return r;
 }
@@ -1209,7 +1209,7 @@ int kvm_init_coalesced_mmio(kvm_context_t kvm)
 	int r = 0;
 	kvm->coalesced_mmio = 0;
 #ifdef KVM_CAP_COALESCED_MMIO
-	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_COALESCED_MMIO);
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_COALESCED_MMIO);
 	if (r > 0) {
 		kvm->coalesced_mmio = r;
 		return 0;
@@ -1296,7 +1296,7 @@ int kvm_assign_irq(kvm_context_t kvm,
 {
 	int ret;
 
-	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_ASSIGN_DEV_IRQ);
+	ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_ASSIGN_DEV_IRQ);
 	if (ret > 0) {
 		ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_DEV_IRQ, assigned_irq);
 		if (ret < 0)
@@ -1346,7 +1346,7 @@ int kvm_destroy_memory_region_works(kvm_context_t kvm)
 	int ret = 0;
 
 #ifdef KVM_CAP_DESTROY_MEMORY_REGION_WORKS
-	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
+	ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION,
 		    KVM_CAP_DESTROY_MEMORY_REGION_WORKS);
 	if (ret <= 0)
 		ret = 0;
@@ -1362,7 +1362,7 @@ int kvm_reinject_control(kvm_context_t kvm, int pit_reinject)
 
 	control.pit_reinject = pit_reinject;
 
-	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_REINJECT_CONTROL);
+	r = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, KVM_CAP_REINJECT_CONTROL);
 	if (r > 0) {
 		r = kvm_vm_ioctl(kvm_state, KVM_REINJECT_CONTROL, &control);
 		if (r == -1)
diff --git a/qemu-kvm.h b/qemu-kvm.h
index f27da7c..a6261fd 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -52,8 +52,6 @@ extern int kvm_abi;
  */
 
 struct kvm_context {
-	/// Filedescriptor to /dev/kvm
-	int fd;
 	void *opaque;
 	/// is dirty pages logging enabled for all regions or not
 	int dirty_pages_log_all;
@@ -1179,6 +1177,7 @@ typedef struct KVMState
 
 extern KVMState *kvm_state;
 
+int kvm_ioctl(KVMState *s, int type, ...);
 int kvm_vm_ioctl(KVMState *s, int type, ...);
 
 #endif
-- 
1.6.2.2

[PATCH 4/5] check extension

[Glauber Costa]

use upstream check_extension code

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/device-assignment.c |    2 +-
 kvm-all.c              |    2 ++
 qemu-kvm-x86.c         |    6 +++---
 qemu-kvm.c             |   18 ++++--------------
 qemu-kvm.h             |    2 +-
 5 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 88c3baf..75db546 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -639,7 +639,7 @@ static int assign_device(AssignedDevInfo *adev)
     /* We always enable the IOMMU if present
      * (or when not disabled on the command line)
      */
-    r = kvm_check_extension(kvm_context, KVM_CAP_IOMMU);
+    r = kvm_check_extension(kvm_state, KVM_CAP_IOMMU);
     if (r && !adev->disable_iommu)
 	assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index 0ec6475..b4b5a35 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -383,6 +383,7 @@ int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
     return ret;
 }
 
+#endif
 int kvm_check_extension(KVMState *s, unsigned int extension)
 {
     int ret;
@@ -394,6 +395,7 @@ int kvm_check_extension(KVMState *s, unsigned int extension)
 
     return ret;
 }
+#ifdef KVM_UPSTREAM
 
 int kvm_init(int smp_cpus)
 {
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index fb3ac9a..128a792 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -328,7 +328,7 @@ int kvm_has_pit_state2(kvm_context_t kvm)
 	int r = 0;
 
 #ifdef KVM_CAP_PIT_STATE2
-	r = kvm_check_extension(kvm, KVM_CAP_PIT_STATE2);
+	r = kvm_check_extension(kvm_state, KVM_CAP_PIT_STATE2);
 #endif
 	return r;
 }
@@ -652,7 +652,7 @@ uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
 	uint32_t ret = 0;
 	uint32_t cpuid_1_edx;
 
-	if (!kvm_check_extension(kvm, KVM_CAP_EXT_CPUID)) {
+	if (!kvm_check_extension(kvm_state, KVM_CAP_EXT_CPUID)) {
 		return -1U;
 	}
 
@@ -1184,7 +1184,7 @@ static int get_para_features(kvm_context_t kvm_context)
 	int i, features = 0;
 
 	for (i = 0; i < ARRAY_SIZE(para_features)-1; i++) {
-		if (kvm_check_extension(kvm_context, para_features[i].cap))
+		if (kvm_check_extension(kvm_state, para_features[i].cap))
 			features |= (1 << para_features[i].feature);
 	}
 
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 2654f91..d5f8464 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -590,16 +590,6 @@ static int kvm_create_default_phys_mem(kvm_context_t kvm,
 	return -1;
 }
 
-int kvm_check_extension(kvm_context_t kvm, int ext)
-{
-	int ret;
-
-	ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, ext);
-	if (ret > 0)
-		return ret;
-	return 0;
-}
-
 void kvm_create_irqchip(kvm_context_t kvm)
 {
 	int r;
@@ -1378,7 +1368,7 @@ int kvm_has_gsi_routing(kvm_context_t kvm)
     int r = 0;
 
 #ifdef KVM_CAP_IRQ_ROUTING
-    r = kvm_check_extension(kvm, KVM_CAP_IRQ_ROUTING);
+    r = kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING);
 #endif
     return r;
 }
@@ -1386,7 +1376,7 @@ int kvm_has_gsi_routing(kvm_context_t kvm)
 int kvm_get_gsi_count(kvm_context_t kvm)
 {
 #ifdef KVM_CAP_IRQ_ROUTING
-	return kvm_check_extension(kvm, KVM_CAP_IRQ_ROUTING);
+	return kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING);
 #else
 	return -EINVAL;
 #endif
@@ -1660,7 +1650,7 @@ int kvm_irqfd(kvm_context_t kvm, int gsi, int flags)
 	int r;
 	int fd;
 
-	if (!kvm_check_extension(kvm, KVM_CAP_IRQFD))
+	if (!kvm_check_extension(kvm_state, KVM_CAP_IRQFD))
 		return -ENOENT;
 
 	fd = eventfd(0, 0);
@@ -2431,7 +2421,7 @@ int kvm_setup_guest_memory(void *area, unsigned long size)
 
 int kvm_qemu_check_extension(int ext)
 {
-    return kvm_check_extension(kvm_context, ext);
+    return kvm_check_extension(kvm_state, ext);
 }
 
 int kvm_qemu_init_env(CPUState *cenv)
diff --git a/qemu-kvm.h b/qemu-kvm.h
index a6261fd..afb6fef 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -163,7 +163,6 @@ int kvm_create(kvm_context_t kvm,
 	       unsigned long phys_mem_bytes,
 	       void **phys_mem);
 int kvm_create_vm(kvm_context_t kvm);
-int kvm_check_extension(kvm_context_t kvm, int ext);
 void kvm_create_irqchip(kvm_context_t kvm);
 
 /*!
@@ -1179,5 +1178,6 @@ extern KVMState *kvm_state;
 
 int kvm_ioctl(KVMState *s, int type, ...);
 int kvm_vm_ioctl(KVMState *s, int type, ...);
+int kvm_check_extension(KVMState *s, unsigned int ext);
 
 #endif
-- 
1.6.2.2

[PATCH 5/5] use upstream cpuid code

[Glauber Costa]

use cpuid code from upstream. By doing that, we lose the following snippet
in kvm_get_supported_cpuid():

    ret |= 1 << 12; /* MTRR */
    ret |= 1 << 16; /* PAT */
    ret |= 1 << 7;  /* MCE */
    ret |= 1 << 14; /* MCA */

A quick search in mailing lists says this code is not really necessary, and we're
keeping it just for backwards compatibility. This is not that important, because
we'd lose it anyway in the golden day in which we totally merge with qemu.
Anyway, if it do _is_ important, we can send a patch to qemu with it.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm-x86.c    |  121 -----------------------------------------------------
 target-i386/kvm.c |    2 +
 2 files changed, 2 insertions(+), 121 deletions(-)

diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 128a792..8d6a6a8 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -608,108 +608,6 @@ int kvm_disable_tpr_access_reporting(kvm_vcpu_context_t vcpu)
 
 #endif
 
-#ifdef KVM_CAP_EXT_CPUID
-
-static struct kvm_cpuid2 *try_get_cpuid(kvm_context_t kvm, int max)
-{
-	struct kvm_cpuid2 *cpuid;
-	int r, size;
-
-	size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
-	cpuid = qemu_malloc(size);
-	cpuid->nent = max;
-	r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_CPUID, cpuid);
-	if (r == -1)
-		r = -errno;
-	else if (r == 0 && cpuid->nent >= max)
-		r = -E2BIG;
-	if (r < 0) {
-		if (r == -E2BIG) {
-			free(cpuid);
-			return NULL;
-		} else {
-			fprintf(stderr, "KVM_GET_SUPPORTED_CPUID failed: %s\n",
-				strerror(-r));
-			exit(1);
-		}
-	}
-	return cpuid;
-}
-
-#define R_EAX 0
-#define R_ECX 1
-#define R_EDX 2
-#define R_EBX 3
-#define R_ESP 4
-#define R_EBP 5
-#define R_ESI 6
-#define R_EDI 7
-
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
-{
-	struct kvm_cpuid2 *cpuid;
-	int i, max;
-	uint32_t ret = 0;
-	uint32_t cpuid_1_edx;
-
-	if (!kvm_check_extension(kvm_state, KVM_CAP_EXT_CPUID)) {
-		return -1U;
-	}
-
-	max = 1;
-	while ((cpuid = try_get_cpuid(kvm, max)) == NULL) {
-		max *= 2;
-	}
-
-	for (i = 0; i < cpuid->nent; ++i) {
-		if (cpuid->entries[i].function == function) {
-			switch (reg) {
-			case R_EAX:
-				ret = cpuid->entries[i].eax;
-				break;
-			case R_EBX:
-				ret = cpuid->entries[i].ebx;
-				break;
-			case R_ECX:
-				ret = cpuid->entries[i].ecx;
-				break;
-			case R_EDX:
-				ret = cpuid->entries[i].edx;
-                                if (function == 1) {
-                                    /* kvm misreports the following features
-                                     */
-                                    ret |= 1 << 12; /* MTRR */
-                                    ret |= 1 << 16; /* PAT */
-                                    ret |= 1 << 7;  /* MCE */
-                                    ret |= 1 << 14; /* MCA */
-                                }
-
-				/* On Intel, kvm returns cpuid according to
-				 * the Intel spec, so add missing bits
-				 * according to the AMD spec:
-				 */
-				if (function == 0x80000001) {
-					cpuid_1_edx = kvm_get_supported_cpuid(kvm, 1, R_EDX);
-					ret |= cpuid_1_edx & 0xdfeff7ff;
-				}
-				break;
-			}
-		}
-	}
-
-	free(cpuid);
-
-	return ret;
-}
-
-#else
-
-uint32_t kvm_get_supported_cpuid(kvm_context_t kvm, uint32_t function, int reg)
-{
-	return -1U;
-}
-
-#endif
 int kvm_qemu_create_memory_alias(uint64_t phys_start,
                                  uint64_t len,
                                  uint64_t target_phys)
@@ -1191,19 +1089,6 @@ static int get_para_features(kvm_context_t kvm_context)
 	return features;
 }
 
-static void kvm_trim_features(uint32_t *features, uint32_t supported)
-{
-    int i;
-    uint32_t mask;
-
-    for (i = 0; i < 32; ++i) {
-        mask = 1U << i;
-        if ((*features & mask) && !(supported & mask)) {
-            *features &= ~mask;
-        }
-    }
-}
-
 int kvm_arch_qemu_init_env(CPUState *cenv)
 {
     struct kvm_cpuid_entry2 cpuid_ent[100];
@@ -1599,12 +1484,6 @@ int kvm_arch_init_irq_routing(void)
     return 0;
 }
 
-uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
-                                      int reg)
-{
-    return kvm_get_supported_cpuid(kvm_context, function, reg);
-}
-
 void kvm_arch_process_irqchip_events(CPUState *env)
 {
     kvm_arch_save_regs(env);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index cfa5b80..359d27d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -35,6 +35,7 @@
     do { } while (0)
 #endif
 
+#endif
 #ifdef KVM_CAP_EXT_CPUID
 
 static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
@@ -130,6 +131,7 @@ static void kvm_trim_features(uint32_t *features, uint32_t supported)
         }
     }
 }
+#ifdef KVM_UPSTREAM
 
 int kvm_arch_init_vcpu(CPUState *env)
 {
-- 
1.6.2.2

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Marcelo Tosatti]

On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> Start using kvm_vm_ioctl's code.
> For type safety, delete vm_fd from kvm_context entirely, so the
> compiler can play along with us helping to detect errors I might
> have made.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  kvm-all.c      |    2 ++
>  qemu-kvm-x86.c |   18 +++++++++---------
>  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
>  qemu-kvm.h     |    6 +++---
>  4 files changed, 40 insertions(+), 38 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 67908a7..9373d99 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -809,6 +809,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
>  
>      return ret;
>  }
> +#endif
>  
>  int kvm_vm_ioctl(KVMState *s, int type, ...)
>  {
> @@ -827,6 +828,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
>      return ret;
>  }
>  
> +#ifdef KVM_UPSTREAM
>  int kvm_vcpu_ioctl(CPUState *env, int type, ...)
>  {
>      int ret;
> diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> index 350f272..c7393cd 100644
> --- a/qemu-kvm-x86.c
> +++ b/qemu-kvm-x86.c
> @@ -40,7 +40,7 @@ int kvm_set_tss_addr(kvm_context_t kvm, unsigned long addr)
>  
>  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
>  	if (r > 0) {
> -		r = ioctl(kvm->vm_fd, KVM_SET_TSS_ADDR, addr);
> +		r = kvm_vm_ioctl(kvm_state, KVM_SET_TSS_ADDR, addr);
>  		if (r == -1) {
>  			fprintf(stderr, "kvm_set_tss_addr: %m\n");
>  			return -errno;
> @@ -82,7 +82,7 @@ static int kvm_create_pit(kvm_context_t kvm)
>  	if (!kvm->no_pit_creation) {
>  		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
>  		if (r > 0) {
> -			r = ioctl(kvm->vm_fd, KVM_CREATE_PIT);
> +			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_PIT);
>  			if (r >= 0)
>  				kvm->pit_in_kernel = 1;
>  			else {
> @@ -211,7 +211,7 @@ int kvm_create_memory_alias(kvm_context_t kvm,
>  		.memory_size = len,
>  		.target_phys_addr = target_phys,
>  	};
> -	int fd = kvm->vm_fd;
> +	int fd = kvm_state->vmfd;
>  	int r;
>  	int slot;
>  
> @@ -272,7 +272,7 @@ int kvm_get_pit(kvm_context_t kvm, struct kvm_pit_state *s)
>  	int r;
>  	if (!kvm->pit_in_kernel)
>  		return 0;
> -	r = ioctl(kvm->vm_fd, KVM_GET_PIT, s);
> +	r = kvm_vm_ioctl(kvm_state, KVM_GET_PIT, s);
>  	if (r == -1) {
>  		r = -errno;
>  		perror("kvm_get_pit");
> @@ -285,7 +285,7 @@ int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s)
>  	int r;
>  	if (!kvm->pit_in_kernel)
>  		return 0;
> -	r = ioctl(kvm->vm_fd, KVM_SET_PIT, s);
> +	r = kvm_vm_ioctl(kvm_state, KVM_SET_PIT, s);
>  	if (r == -1) {
>  		r = -errno;
>  		perror("kvm_set_pit");
> @@ -299,7 +299,7 @@ int kvm_get_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
>  	int r;
>  	if (!kvm->pit_in_kernel)
>  		return 0;
> -	r = ioctl(kvm->vm_fd, KVM_GET_PIT2, ps2);
> +	r = kvm_vm_ioctl(kvm_state, KVM_GET_PIT2, ps2);
>  	if (r == -1) {
>  		r = -errno;
>  		perror("kvm_get_pit2");
> @@ -312,7 +312,7 @@ int kvm_set_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
>  	int r;
>  	if (!kvm->pit_in_kernel)
>  		return 0;
> -	r = ioctl(kvm->vm_fd, KVM_SET_PIT2, ps2);
> +	r = kvm_vm_ioctl(kvm_state, KVM_SET_PIT2, ps2);
>  	if (r == -1) {
>  		r = -errno;
>  		perror("kvm_set_pit2");
> @@ -549,7 +549,7 @@ int kvm_set_shadow_pages(kvm_context_t kvm, unsigned int nrshadow_pages)
>  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
>  		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
>  	if (r > 0) {
> -		r = ioctl(kvm->vm_fd, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
> +		r = kvm_vm_ioctl(kvm_state, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
>  		if (r == -1) {
>  			fprintf(stderr, "kvm_set_shadow_pages: %m\n");
>  			return -errno;
> @@ -568,7 +568,7 @@ int kvm_get_shadow_pages(kvm_context_t kvm, unsigned int *nrshadow_pages)
>  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
>  		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
>  	if (r > 0) {
> -		*nrshadow_pages = ioctl(kvm->vm_fd, KVM_GET_NR_MMU_PAGES);
> +		*nrshadow_pages = kvm_vm_ioctl(kvm_state, KVM_GET_NR_MMU_PAGES);
>  		return 0;
>  	}
>  #endif
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 904e751..f09bcca 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -44,7 +44,7 @@ int kvm_pit_reinject = 1;
>  int kvm_nested = 0;
>  
>  
> -static KVMState *kvm_state;
> +KVMState *kvm_state;
>  kvm_context_t kvm_context;
>  
>  pthread_mutex_t qemu_mutex = PTHREAD_MUTEX_INITIALIZER;
> @@ -332,7 +332,7 @@ static int kvm_dirty_pages_log_change(kvm_context_t kvm,
>  			mem.guest_phys_addr,
>  			mem.memory_size,
>  			mem.flags);
> -		r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &mem);
> +		r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &mem);
>  		if (r == -1)
>  			fprintf(stderr, "%s: %m\n", __FUNCTION__);
>  	}
> @@ -452,7 +452,7 @@ int kvm_init(int smp_cpus)
>      kvm_context = &kvm_state->kvm_context;
>  
>  	kvm_context->fd = fd;
> -	kvm_context->vm_fd = -1;
> +	kvm_state->vmfd = -1;
>  	kvm_context->opaque = cpu_single_env;
>  	kvm_context->dirty_pages_log_all = 0;
>  	kvm_context->no_irqchip_creation = 0;
> @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
>  	vcpu_ctx->kvm = kvm;
>  	vcpu_ctx->id = id;
>  
> -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
>  	if (r == -1) {
>  		fprintf(stderr, "kvm_create_vcpu: %m\n");
>  		goto err;
> @@ -550,7 +550,7 @@ static int kvm_set_boot_vcpu_id(kvm_context_t kvm, uint32_t id)
>  #ifdef KVM_CAP_SET_BOOT_CPU_ID
>      int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_BOOT_CPU_ID);
>      if (r > 0)
> -        return ioctl(kvm->vm_fd, KVM_SET_BOOT_CPU_ID, id);
> +        return kvm_vm_ioctl(kvm_state, KVM_SET_BOOT_CPU_ID, id);
>      return -ENOSYS;
>  #else
>      return -ENOSYS;
> @@ -571,7 +571,7 @@ int kvm_create_vm(kvm_context_t kvm)
>  		fprintf(stderr, "kvm_create_vm: %m\n");
>  		return -1;
>  	}
> -	kvm->vm_fd = fd;
> +	kvm_state->vmfd = fd;
>  	return 0;
>  }
>  
> @@ -594,7 +594,7 @@ int kvm_check_extension(kvm_context_t kvm, int ext)
>  {
>  	int ret;
>  
> -	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, ext);
> +	ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, ext);
>  	if (ret > 0)
>  		return ret;
>  	return 0;
> @@ -609,7 +609,7 @@ void kvm_create_irqchip(kvm_context_t kvm)
>  	if (!kvm->no_irqchip_creation) {
>  		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_IRQCHIP);
>  		if (r > 0) {	/* kernel irqchip supported */
> -			r = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
> +			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_IRQCHIP);
>  			if (r >= 0) {
>  				kvm->irqchip_inject_ioctl = KVM_IRQ_LINE;
>  #if defined(KVM_CAP_IRQ_INJECT_STATUS) && defined(KVM_IRQ_LINE_STATUS)
> @@ -664,7 +664,7 @@ int kvm_register_phys_mem(kvm_context_t kvm,
>  	DPRINTF("memory: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
>  		memory.guest_phys_addr, memory.memory_size,
>  		memory.userspace_addr, memory.slot, memory.flags);
> -	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
> +	r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &memory);
>  	if (r == -1) {
>  		fprintf(stderr, "create_userspace_phys_mem: %s\n", strerror(errno));
>  		return -1;
> @@ -710,7 +710,7 @@ void kvm_destroy_phys_mem(kvm_context_t kvm, unsigned long phys_start,
>  		memory.guest_phys_addr,
>  		memory.memory_size,
>  		memory.flags);
> -	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
> +	r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &memory);
>  	if (r == -1) {
>  		fprintf(stderr, "destroy_userspace_phys_mem: %s",
>  			strerror(errno));
> @@ -741,7 +741,7 @@ static int kvm_get_map(kvm_context_t kvm, int ioctl_num, int slot, void *buf)
>  
>  	log.dirty_bitmap = buf;
>  
> -	r = ioctl(kvm->vm_fd, ioctl_num, &log);
> +	r = kvm_vm_ioctl(kvm_state, ioctl_num, &log);
>  	if (r == -1)
>  		return -errno;
>  	return 0;
> @@ -794,7 +794,7 @@ int kvm_set_irq_level(kvm_context_t kvm, int irq, int level, int *status)
>  		return 0;
>  	event.level = level;
>  	event.irq = irq;
> -	r = ioctl(kvm->vm_fd, kvm->irqchip_inject_ioctl, &event);
> +	r = kvm_vm_ioctl(kvm_state, kvm->irqchip_inject_ioctl, &event);
>  	if (r == -1)
>  		perror("kvm_set_irq_level");
>  
> @@ -816,7 +816,7 @@ int kvm_get_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
>  
>  	if (!kvm->irqchip_in_kernel)
>  		return 0;
> -	r = ioctl(kvm->vm_fd, KVM_GET_IRQCHIP, chip);
> +	r = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
>  	if (r == -1) {
>  		r = -errno;
>  		perror("kvm_get_irqchip\n");
> @@ -830,7 +830,7 @@ int kvm_set_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
>  
>  	if (!kvm->irqchip_in_kernel)
>  		return 0;
> -	r = ioctl(kvm->vm_fd, KVM_SET_IRQCHIP, chip);
> +	r = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip);
>  	if (r == -1) {
>  		r = -errno;
>  		perror("kvm_set_irqchip\n");
> @@ -1230,7 +1230,7 @@ int kvm_coalesce_mmio_region(target_phys_addr_t addr, ram_addr_t size)
>  		zone.addr = addr;
>  		zone.size = size;
>  
> -		r = ioctl(kvm->vm_fd, KVM_REGISTER_COALESCED_MMIO, &zone);
> +		r = kvm_vm_ioctl(kvm_state, KVM_REGISTER_COALESCED_MMIO, &zone);
>  		if (r == -1) {
>  			perror("kvm_register_coalesced_mmio_zone");
>  			return -errno;
> @@ -1253,7 +1253,7 @@ int kvm_uncoalesce_mmio_region(target_phys_addr_t addr, ram_addr_t size)
>  		zone.addr = addr;
>  		zone.size = size;
>  
> -		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_COALESCED_MMIO, &zone);
> +		r = kvm_vm_ioctl(kvm_state, KVM_UNREGISTER_COALESCED_MMIO, &zone);
>  		if (r == -1) {
>  			perror("kvm_unregister_coalesced_mmio_zone");
>  			return -errno;
> @@ -1271,7 +1271,7 @@ int kvm_assign_pci_device(kvm_context_t kvm,
>  {
>  	int ret;
>  
> -	ret = ioctl(kvm->vm_fd, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
> +	ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -1283,7 +1283,7 @@ static int kvm_old_assign_irq(kvm_context_t kvm,
>  {
>  	int ret;
>  
> -	ret = ioctl(kvm->vm_fd, KVM_ASSIGN_IRQ, assigned_irq);
> +	ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_IRQ, assigned_irq);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -1298,7 +1298,7 @@ int kvm_assign_irq(kvm_context_t kvm,
>  
>  	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_ASSIGN_DEV_IRQ);
>  	if (ret > 0) {
> -		ret = ioctl(kvm->vm_fd, KVM_ASSIGN_DEV_IRQ, assigned_irq);
> +		ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_DEV_IRQ, assigned_irq);
>  		if (ret < 0)
>  			return -errno;
>  		return ret;
> @@ -1312,7 +1312,7 @@ int kvm_deassign_irq(kvm_context_t kvm,
>  {
>  	int ret;
>  
> -	ret = ioctl(kvm->vm_fd, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
> +	ret = kvm_vm_ioctl(kvm_state, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
>  	if (ret < 0)
>              return -errno;
>  
> @@ -1333,7 +1333,7 @@ int kvm_deassign_pci_device(kvm_context_t kvm,
>  {
>  	int ret;
>  
> -	ret = ioctl(kvm->vm_fd, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
> +	ret = kvm_vm_ioctl(kvm_state, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -1364,7 +1364,7 @@ int kvm_reinject_control(kvm_context_t kvm, int pit_reinject)
>  
>  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_REINJECT_CONTROL);
>  	if (r > 0) {
> -		r = ioctl(kvm->vm_fd, KVM_REINJECT_CONTROL, &control);
> +		r = kvm_vm_ioctl(kvm_state, KVM_REINJECT_CONTROL, &control);
>  		if (r == -1)
>  			return -errno;
>  		return r;
> @@ -1584,7 +1584,7 @@ int kvm_commit_irq_routes(kvm_context_t kvm)
>  	int r;
>  
>  	kvm->irq_routes->flags = 0;
> -	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, kvm->irq_routes);
> +	r = kvm_vm_ioctl(kvm_state, KVM_SET_GSI_ROUTING, kvm->irq_routes);
>  	if (r == -1)
>  		r = -errno;
>  	return r;
> @@ -1616,7 +1616,7 @@ int kvm_assign_set_msix_nr(kvm_context_t kvm,
>  {
>          int ret;
>  
> -        ret = ioctl(kvm->vm_fd, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
> +        ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
>          if (ret < 0)
>                  return -errno;
>  
> @@ -1628,7 +1628,7 @@ int kvm_assign_set_msix_entry(kvm_context_t kvm,
>  {
>          int ret;
>  
> -        ret = ioctl(kvm->vm_fd, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> +        ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
>          if (ret < 0)
>                  return -errno;
>  
> @@ -1649,7 +1649,7 @@ static int _kvm_irqfd(kvm_context_t kvm, int fd, int gsi, int flags)
>  		.flags = flags,
>  	};
>  
> -	r = ioctl(kvm->vm_fd, KVM_IRQFD, &data);
> +	r = kvm_vm_ioctl(kvm_state, KVM_IRQFD, &data);
>  	if (r == -1)
>  		r = -errno;
>  	return r;
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index d5291a3..f27da7c 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -54,9 +54,6 @@ extern int kvm_abi;
>  struct kvm_context {
>  	/// Filedescriptor to /dev/kvm
>  	int fd;
> -	int vm_fd;
> -	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
> -	struct kvm_callbacks *callbacks;

So you move some information (vm_fd, kvm_callbacks) from kvm_context to
KVMState.

Why not wait until KVMState is capable of fully replacing kvm_context,
and then do a full replacement at once?

I fail to understand the point of the partial convertion, and it seems
to make the qemu-kvm.c code more confusing?

>  	void *opaque;
>  	/// is dirty pages logging enabled for all regions or not
>  	int dirty_pages_log_all;
> @@ -1180,5 +1177,8 @@ typedef struct KVMState
>      struct kvm_context kvm_context;
>  } KVMState;
>  
> +extern KVMState *kvm_state;
> +
> +int kvm_vm_ioctl(KVMState *s, int type, ...);
>  
>  #endif
> -- 
> 1.6.2.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Glauber Costa]

On Fri, Jul 17, 2009 at 10:35:43AM -0300, Marcelo Tosatti wrote:
> On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> > Start using kvm_vm_ioctl's code.
> > For type safety, delete vm_fd from kvm_context entirely, so the
> > compiler can play along with us helping to detect errors I might
> > have made.
> > 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  kvm-all.c      |    2 ++
> >  qemu-kvm-x86.c |   18 +++++++++---------
> >  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
> >  qemu-kvm.h     |    6 +++---
> >  4 files changed, 40 insertions(+), 38 deletions(-)
> > 
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 67908a7..9373d99 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -809,6 +809,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
> >  
> >      return ret;
> >  }
> > +#endif
> >  
> >  int kvm_vm_ioctl(KVMState *s, int type, ...)
> >  {
> > @@ -827,6 +828,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
> >      return ret;
> >  }
> >  
> > +#ifdef KVM_UPSTREAM
> >  int kvm_vcpu_ioctl(CPUState *env, int type, ...)
> >  {
> >      int ret;
> > diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
> > index 350f272..c7393cd 100644
> > --- a/qemu-kvm-x86.c
> > +++ b/qemu-kvm-x86.c
> > @@ -40,7 +40,7 @@ int kvm_set_tss_addr(kvm_context_t kvm, unsigned long addr)
> >  
> >  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_TSS_ADDR);
> >  	if (r > 0) {
> > -		r = ioctl(kvm->vm_fd, KVM_SET_TSS_ADDR, addr);
> > +		r = kvm_vm_ioctl(kvm_state, KVM_SET_TSS_ADDR, addr);
> >  		if (r == -1) {
> >  			fprintf(stderr, "kvm_set_tss_addr: %m\n");
> >  			return -errno;
> > @@ -82,7 +82,7 @@ static int kvm_create_pit(kvm_context_t kvm)
> >  	if (!kvm->no_pit_creation) {
> >  		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_PIT);
> >  		if (r > 0) {
> > -			r = ioctl(kvm->vm_fd, KVM_CREATE_PIT);
> > +			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_PIT);
> >  			if (r >= 0)
> >  				kvm->pit_in_kernel = 1;
> >  			else {
> > @@ -211,7 +211,7 @@ int kvm_create_memory_alias(kvm_context_t kvm,
> >  		.memory_size = len,
> >  		.target_phys_addr = target_phys,
> >  	};
> > -	int fd = kvm->vm_fd;
> > +	int fd = kvm_state->vmfd;
> >  	int r;
> >  	int slot;
> >  
> > @@ -272,7 +272,7 @@ int kvm_get_pit(kvm_context_t kvm, struct kvm_pit_state *s)
> >  	int r;
> >  	if (!kvm->pit_in_kernel)
> >  		return 0;
> > -	r = ioctl(kvm->vm_fd, KVM_GET_PIT, s);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_GET_PIT, s);
> >  	if (r == -1) {
> >  		r = -errno;
> >  		perror("kvm_get_pit");
> > @@ -285,7 +285,7 @@ int kvm_set_pit(kvm_context_t kvm, struct kvm_pit_state *s)
> >  	int r;
> >  	if (!kvm->pit_in_kernel)
> >  		return 0;
> > -	r = ioctl(kvm->vm_fd, KVM_SET_PIT, s);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_SET_PIT, s);
> >  	if (r == -1) {
> >  		r = -errno;
> >  		perror("kvm_set_pit");
> > @@ -299,7 +299,7 @@ int kvm_get_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
> >  	int r;
> >  	if (!kvm->pit_in_kernel)
> >  		return 0;
> > -	r = ioctl(kvm->vm_fd, KVM_GET_PIT2, ps2);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_GET_PIT2, ps2);
> >  	if (r == -1) {
> >  		r = -errno;
> >  		perror("kvm_get_pit2");
> > @@ -312,7 +312,7 @@ int kvm_set_pit2(kvm_context_t kvm, struct kvm_pit_state2 *ps2)
> >  	int r;
> >  	if (!kvm->pit_in_kernel)
> >  		return 0;
> > -	r = ioctl(kvm->vm_fd, KVM_SET_PIT2, ps2);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_SET_PIT2, ps2);
> >  	if (r == -1) {
> >  		r = -errno;
> >  		perror("kvm_set_pit2");
> > @@ -549,7 +549,7 @@ int kvm_set_shadow_pages(kvm_context_t kvm, unsigned int nrshadow_pages)
> >  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
> >  		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
> >  	if (r > 0) {
> > -		r = ioctl(kvm->vm_fd, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
> > +		r = kvm_vm_ioctl(kvm_state, KVM_SET_NR_MMU_PAGES, nrshadow_pages);
> >  		if (r == -1) {
> >  			fprintf(stderr, "kvm_set_shadow_pages: %m\n");
> >  			return -errno;
> > @@ -568,7 +568,7 @@ int kvm_get_shadow_pages(kvm_context_t kvm, unsigned int *nrshadow_pages)
> >  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION,
> >  		  KVM_CAP_MMU_SHADOW_CACHE_CONTROL);
> >  	if (r > 0) {
> > -		*nrshadow_pages = ioctl(kvm->vm_fd, KVM_GET_NR_MMU_PAGES);
> > +		*nrshadow_pages = kvm_vm_ioctl(kvm_state, KVM_GET_NR_MMU_PAGES);
> >  		return 0;
> >  	}
> >  #endif
> > diff --git a/qemu-kvm.c b/qemu-kvm.c
> > index 904e751..f09bcca 100644
> > --- a/qemu-kvm.c
> > +++ b/qemu-kvm.c
> > @@ -44,7 +44,7 @@ int kvm_pit_reinject = 1;
> >  int kvm_nested = 0;
> >  
> >  
> > -static KVMState *kvm_state;
> > +KVMState *kvm_state;
> >  kvm_context_t kvm_context;
> >  
> >  pthread_mutex_t qemu_mutex = PTHREAD_MUTEX_INITIALIZER;
> > @@ -332,7 +332,7 @@ static int kvm_dirty_pages_log_change(kvm_context_t kvm,
> >  			mem.guest_phys_addr,
> >  			mem.memory_size,
> >  			mem.flags);
> > -		r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &mem);
> > +		r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &mem);
> >  		if (r == -1)
> >  			fprintf(stderr, "%s: %m\n", __FUNCTION__);
> >  	}
> > @@ -452,7 +452,7 @@ int kvm_init(int smp_cpus)
> >      kvm_context = &kvm_state->kvm_context;
> >  
> >  	kvm_context->fd = fd;
> > -	kvm_context->vm_fd = -1;
> > +	kvm_state->vmfd = -1;
> >  	kvm_context->opaque = cpu_single_env;
> >  	kvm_context->dirty_pages_log_all = 0;
> >  	kvm_context->no_irqchip_creation = 0;
> > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
> >  	vcpu_ctx->kvm = kvm;
> >  	vcpu_ctx->id = id;
> >  
> > -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
> >  	if (r == -1) {
> >  		fprintf(stderr, "kvm_create_vcpu: %m\n");
> >  		goto err;
> > @@ -550,7 +550,7 @@ static int kvm_set_boot_vcpu_id(kvm_context_t kvm, uint32_t id)
> >  #ifdef KVM_CAP_SET_BOOT_CPU_ID
> >      int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_SET_BOOT_CPU_ID);
> >      if (r > 0)
> > -        return ioctl(kvm->vm_fd, KVM_SET_BOOT_CPU_ID, id);
> > +        return kvm_vm_ioctl(kvm_state, KVM_SET_BOOT_CPU_ID, id);
> >      return -ENOSYS;
> >  #else
> >      return -ENOSYS;
> > @@ -571,7 +571,7 @@ int kvm_create_vm(kvm_context_t kvm)
> >  		fprintf(stderr, "kvm_create_vm: %m\n");
> >  		return -1;
> >  	}
> > -	kvm->vm_fd = fd;
> > +	kvm_state->vmfd = fd;
> >  	return 0;
> >  }
> >  
> > @@ -594,7 +594,7 @@ int kvm_check_extension(kvm_context_t kvm, int ext)
> >  {
> >  	int ret;
> >  
> > -	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, ext);
> > +	ret = kvm_ioctl(kvm_state, KVM_CHECK_EXTENSION, ext);
> >  	if (ret > 0)
> >  		return ret;
> >  	return 0;
> > @@ -609,7 +609,7 @@ void kvm_create_irqchip(kvm_context_t kvm)
> >  	if (!kvm->no_irqchip_creation) {
> >  		r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_IRQCHIP);
> >  		if (r > 0) {	/* kernel irqchip supported */
> > -			r = ioctl(kvm->vm_fd, KVM_CREATE_IRQCHIP);
> > +			r = kvm_vm_ioctl(kvm_state, KVM_CREATE_IRQCHIP);
> >  			if (r >= 0) {
> >  				kvm->irqchip_inject_ioctl = KVM_IRQ_LINE;
> >  #if defined(KVM_CAP_IRQ_INJECT_STATUS) && defined(KVM_IRQ_LINE_STATUS)
> > @@ -664,7 +664,7 @@ int kvm_register_phys_mem(kvm_context_t kvm,
> >  	DPRINTF("memory: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
> >  		memory.guest_phys_addr, memory.memory_size,
> >  		memory.userspace_addr, memory.slot, memory.flags);
> > -	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &memory);
> >  	if (r == -1) {
> >  		fprintf(stderr, "create_userspace_phys_mem: %s\n", strerror(errno));
> >  		return -1;
> > @@ -710,7 +710,7 @@ void kvm_destroy_phys_mem(kvm_context_t kvm, unsigned long phys_start,
> >  		memory.guest_phys_addr,
> >  		memory.memory_size,
> >  		memory.flags);
> > -	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_SET_USER_MEMORY_REGION, &memory);
> >  	if (r == -1) {
> >  		fprintf(stderr, "destroy_userspace_phys_mem: %s",
> >  			strerror(errno));
> > @@ -741,7 +741,7 @@ static int kvm_get_map(kvm_context_t kvm, int ioctl_num, int slot, void *buf)
> >  
> >  	log.dirty_bitmap = buf;
> >  
> > -	r = ioctl(kvm->vm_fd, ioctl_num, &log);
> > +	r = kvm_vm_ioctl(kvm_state, ioctl_num, &log);
> >  	if (r == -1)
> >  		return -errno;
> >  	return 0;
> > @@ -794,7 +794,7 @@ int kvm_set_irq_level(kvm_context_t kvm, int irq, int level, int *status)
> >  		return 0;
> >  	event.level = level;
> >  	event.irq = irq;
> > -	r = ioctl(kvm->vm_fd, kvm->irqchip_inject_ioctl, &event);
> > +	r = kvm_vm_ioctl(kvm_state, kvm->irqchip_inject_ioctl, &event);
> >  	if (r == -1)
> >  		perror("kvm_set_irq_level");
> >  
> > @@ -816,7 +816,7 @@ int kvm_get_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
> >  
> >  	if (!kvm->irqchip_in_kernel)
> >  		return 0;
> > -	r = ioctl(kvm->vm_fd, KVM_GET_IRQCHIP, chip);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, chip);
> >  	if (r == -1) {
> >  		r = -errno;
> >  		perror("kvm_get_irqchip\n");
> > @@ -830,7 +830,7 @@ int kvm_set_irqchip(kvm_context_t kvm, struct kvm_irqchip *chip)
> >  
> >  	if (!kvm->irqchip_in_kernel)
> >  		return 0;
> > -	r = ioctl(kvm->vm_fd, KVM_SET_IRQCHIP, chip);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, chip);
> >  	if (r == -1) {
> >  		r = -errno;
> >  		perror("kvm_set_irqchip\n");
> > @@ -1230,7 +1230,7 @@ int kvm_coalesce_mmio_region(target_phys_addr_t addr, ram_addr_t size)
> >  		zone.addr = addr;
> >  		zone.size = size;
> >  
> > -		r = ioctl(kvm->vm_fd, KVM_REGISTER_COALESCED_MMIO, &zone);
> > +		r = kvm_vm_ioctl(kvm_state, KVM_REGISTER_COALESCED_MMIO, &zone);
> >  		if (r == -1) {
> >  			perror("kvm_register_coalesced_mmio_zone");
> >  			return -errno;
> > @@ -1253,7 +1253,7 @@ int kvm_uncoalesce_mmio_region(target_phys_addr_t addr, ram_addr_t size)
> >  		zone.addr = addr;
> >  		zone.size = size;
> >  
> > -		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_COALESCED_MMIO, &zone);
> > +		r = kvm_vm_ioctl(kvm_state, KVM_UNREGISTER_COALESCED_MMIO, &zone);
> >  		if (r == -1) {
> >  			perror("kvm_unregister_coalesced_mmio_zone");
> >  			return -errno;
> > @@ -1271,7 +1271,7 @@ int kvm_assign_pci_device(kvm_context_t kvm,
> >  {
> >  	int ret;
> >  
> > -	ret = ioctl(kvm->vm_fd, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
> > +	ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
> >  	if (ret < 0)
> >  		return -errno;
> >  
> > @@ -1283,7 +1283,7 @@ static int kvm_old_assign_irq(kvm_context_t kvm,
> >  {
> >  	int ret;
> >  
> > -	ret = ioctl(kvm->vm_fd, KVM_ASSIGN_IRQ, assigned_irq);
> > +	ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_IRQ, assigned_irq);
> >  	if (ret < 0)
> >  		return -errno;
> >  
> > @@ -1298,7 +1298,7 @@ int kvm_assign_irq(kvm_context_t kvm,
> >  
> >  	ret = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_ASSIGN_DEV_IRQ);
> >  	if (ret > 0) {
> > -		ret = ioctl(kvm->vm_fd, KVM_ASSIGN_DEV_IRQ, assigned_irq);
> > +		ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_DEV_IRQ, assigned_irq);
> >  		if (ret < 0)
> >  			return -errno;
> >  		return ret;
> > @@ -1312,7 +1312,7 @@ int kvm_deassign_irq(kvm_context_t kvm,
> >  {
> >  	int ret;
> >  
> > -	ret = ioctl(kvm->vm_fd, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
> > +	ret = kvm_vm_ioctl(kvm_state, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
> >  	if (ret < 0)
> >              return -errno;
> >  
> > @@ -1333,7 +1333,7 @@ int kvm_deassign_pci_device(kvm_context_t kvm,
> >  {
> >  	int ret;
> >  
> > -	ret = ioctl(kvm->vm_fd, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
> > +	ret = kvm_vm_ioctl(kvm_state, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
> >  	if (ret < 0)
> >  		return -errno;
> >  
> > @@ -1364,7 +1364,7 @@ int kvm_reinject_control(kvm_context_t kvm, int pit_reinject)
> >  
> >  	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_REINJECT_CONTROL);
> >  	if (r > 0) {
> > -		r = ioctl(kvm->vm_fd, KVM_REINJECT_CONTROL, &control);
> > +		r = kvm_vm_ioctl(kvm_state, KVM_REINJECT_CONTROL, &control);
> >  		if (r == -1)
> >  			return -errno;
> >  		return r;
> > @@ -1584,7 +1584,7 @@ int kvm_commit_irq_routes(kvm_context_t kvm)
> >  	int r;
> >  
> >  	kvm->irq_routes->flags = 0;
> > -	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, kvm->irq_routes);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_SET_GSI_ROUTING, kvm->irq_routes);
> >  	if (r == -1)
> >  		r = -errno;
> >  	return r;
> > @@ -1616,7 +1616,7 @@ int kvm_assign_set_msix_nr(kvm_context_t kvm,
> >  {
> >          int ret;
> >  
> > -        ret = ioctl(kvm->vm_fd, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
> > +        ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
> >          if (ret < 0)
> >                  return -errno;
> >  
> > @@ -1628,7 +1628,7 @@ int kvm_assign_set_msix_entry(kvm_context_t kvm,
> >  {
> >          int ret;
> >  
> > -        ret = ioctl(kvm->vm_fd, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> > +        ret = kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> >          if (ret < 0)
> >                  return -errno;
> >  
> > @@ -1649,7 +1649,7 @@ static int _kvm_irqfd(kvm_context_t kvm, int fd, int gsi, int flags)
> >  		.flags = flags,
> >  	};
> >  
> > -	r = ioctl(kvm->vm_fd, KVM_IRQFD, &data);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_IRQFD, &data);
> >  	if (r == -1)
> >  		r = -errno;
> >  	return r;
> > diff --git a/qemu-kvm.h b/qemu-kvm.h
> > index d5291a3..f27da7c 100644
> > --- a/qemu-kvm.h
> > +++ b/qemu-kvm.h
> > @@ -54,9 +54,6 @@ extern int kvm_abi;
> >  struct kvm_context {
> >  	/// Filedescriptor to /dev/kvm
> >  	int fd;
> > -	int vm_fd;
> > -	/// Callbacks that KVM uses to emulate various unvirtualizable functionality
> > -	struct kvm_callbacks *callbacks;
> 
> So you move some information (vm_fd, kvm_callbacks) from kvm_context to
> KVMState.
> 
> Why not wait until KVMState is capable of fully replacing kvm_context,
> and then do a full replacement at once?
> 
> I fail to understand the point of the partial convertion, and it seems
> to make the qemu-kvm.c code more confusing?
> 
First: incremental steps.
Second: It is not placing in two structures. kvm_context is now _inside_
KVMState. So it is really all inside KVMState, just part of it, in a substructure.

Note that, we have our own copy of KVMState, which is slightly different from upstream.
(it includes kvm_context!). A reasonable next step, is to fold our data of kvm_context
directly inside KVMState.

I wanted, however, to re-use the fields KVMState already have, otherwise the patch
could get quite big.

Also, you have already merged a patch that does exactly this: reuses the breakpoints
structure from KVMState.

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Marcelo Tosatti]

On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> Start using kvm_vm_ioctl's code.
> For type safety, delete vm_fd from kvm_context entirely, so the
> compiler can play along with us helping to detect errors I might
> have made.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  kvm-all.c      |    2 ++
>  qemu-kvm-x86.c |   18 +++++++++---------
>  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
>  qemu-kvm.h     |    6 +++---
>  4 files changed, 40 insertions(+), 38 deletions(-)

<snip>

> @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
>  	vcpu_ctx->kvm = kvm;
>  	vcpu_ctx->id = id;
>  
> -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
>  	if (r == -1) {
>  		fprintf(stderr, "kvm_create_vcpu: %m\n");


int kvm_vm_ioctl(KVMState *s, int type, ...)
{
    ...
    if (ret == -1)
        ret = -errno;
}

Is that fine?

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Marcelo Tosatti]

On Fri, Jul 17, 2009 at 12:49:04PM -0300, Glauber Costa wrote:
> On Fri, Jul 17, 2009 at 12:39:17PM -0300, Marcelo Tosatti wrote:
> > On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> > > Start using kvm_vm_ioctl's code.
> > > For type safety, delete vm_fd from kvm_context entirely, so the
> > > compiler can play along with us helping to detect errors I might
> > > have made.
> > > 
> > > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > > ---
> > >  kvm-all.c      |    2 ++
> > >  qemu-kvm-x86.c |   18 +++++++++---------
> > >  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
> > >  qemu-kvm.h     |    6 +++---
> > >  4 files changed, 40 insertions(+), 38 deletions(-)
> > 
> > <snip>
> > 
> > > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
> > >  	vcpu_ctx->kvm = kvm;
> > >  	vcpu_ctx->id = id;
> > >  
> > > -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> > > +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
> > >  	if (r == -1) {
> > >  		fprintf(stderr, "kvm_create_vcpu: %m\n");
> > 
> > 
> > int kvm_vm_ioctl(KVMState *s, int type, ...)
> > {
> >     ...
> >     if (ret == -1)
> >         ret = -errno;
> > }
> > 
> > Is that fine?
> I don't see a problem with that.

Problem is the sites in qemu-kvm.c kvm_vm_ioctl to return -1 
on error, but kvm_vm_ioctl converts -1 to -errno, no?

> But even if there is, the way to change is it to convince anthony of
> the contrary. If he changes it (or anything else), we'll gain it in
> the next merge.

Or make a temporary wrapper around kvm_vm_ioctl to convert -errno back
to -1 ?

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Glauber Costa]

On Fri, Jul 17, 2009 at 12:39:17PM -0300, Marcelo Tosatti wrote:
> On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> > Start using kvm_vm_ioctl's code.
> > For type safety, delete vm_fd from kvm_context entirely, so the
> > compiler can play along with us helping to detect errors I might
> > have made.
> > 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  kvm-all.c      |    2 ++
> >  qemu-kvm-x86.c |   18 +++++++++---------
> >  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
> >  qemu-kvm.h     |    6 +++---
> >  4 files changed, 40 insertions(+), 38 deletions(-)
> 
> <snip>
> 
> > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
> >  	vcpu_ctx->kvm = kvm;
> >  	vcpu_ctx->id = id;
> >  
> > -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> > +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
> >  	if (r == -1) {
> >  		fprintf(stderr, "kvm_create_vcpu: %m\n");
> 
> 
> int kvm_vm_ioctl(KVMState *s, int type, ...)
> {
>     ...
>     if (ret == -1)
>         ret = -errno;
> }
> 
> Is that fine?
I don't see a problem with that.
But even if there is, the way to change is it to convince anthony of the
contrary. If he changes it (or anything else), we'll gain it in the next merge.

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Marcelo Tosatti]

On Fri, Jul 17, 2009 at 12:46:23PM -0300, Marcelo Tosatti wrote:
> On Fri, Jul 17, 2009 at 12:49:04PM -0300, Glauber Costa wrote:
> > On Fri, Jul 17, 2009 at 12:39:17PM -0300, Marcelo Tosatti wrote:
> > > On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> > > > Start using kvm_vm_ioctl's code.
> > > > For type safety, delete vm_fd from kvm_context entirely, so the
> > > > compiler can play along with us helping to detect errors I might
> > > > have made.
> > > > 
> > > > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > > > ---
> > > >  kvm-all.c      |    2 ++
> > > >  qemu-kvm-x86.c |   18 +++++++++---------
> > > >  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
> > > >  qemu-kvm.h     |    6 +++---
> > > >  4 files changed, 40 insertions(+), 38 deletions(-)
> > > 
> > > <snip>
> > > 
> > > > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
> > > >  	vcpu_ctx->kvm = kvm;
> > > >  	vcpu_ctx->id = id;
> > > >  
> > > > -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> > > > +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
> > > >  	if (r == -1) {
> > > >  		fprintf(stderr, "kvm_create_vcpu: %m\n");
> > > 
> > > 
> > > int kvm_vm_ioctl(KVMState *s, int type, ...)
> > > {
> > >     ...
> > >     if (ret == -1)
> > >         ret = -errno;
> > > }
> > > 
> > > Is that fine?
> > I don't see a problem with that.
> 
> Problem is the sites in qemu-kvm.c kvm_vm_ioctl to return -1 
> on error, but kvm_vm_ioctl converts -1 to -errno, no?

BTW kvm-all.c:

        if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
            dprintf("ioctl failed %d\n", errno);
            ret = -1;
            break;
        }

> > But even if there is, the way to change is it to convince anthony of
> > the contrary. If he changes it (or anything else), we'll gain it in
> > the next merge.
> 
> Or make a temporary wrapper around kvm_vm_ioctl to convert -errno back
> to -1 ?

Re: [PATCH 2/5] reuse kvm_vm_ioctl

[Glauber Costa]

On Fri, Jul 17, 2009 at 12:57:38PM -0300, Marcelo Tosatti wrote:
> On Fri, Jul 17, 2009 at 12:46:23PM -0300, Marcelo Tosatti wrote:
> > On Fri, Jul 17, 2009 at 12:49:04PM -0300, Glauber Costa wrote:
> > > On Fri, Jul 17, 2009 at 12:39:17PM -0300, Marcelo Tosatti wrote:
> > > > On Wed, Jul 15, 2009 at 12:31:40PM -0400, Glauber Costa wrote:
> > > > > Start using kvm_vm_ioctl's code.
> > > > > For type safety, delete vm_fd from kvm_context entirely, so the
> > > > > compiler can play along with us helping to detect errors I might
> > > > > have made.
> > > > > 
> > > > > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > > > > ---
> > > > >  kvm-all.c      |    2 ++
> > > > >  qemu-kvm-x86.c |   18 +++++++++---------
> > > > >  qemu-kvm.c     |   52 ++++++++++++++++++++++++++--------------------------
> > > > >  qemu-kvm.h     |    6 +++---
> > > > >  4 files changed, 40 insertions(+), 38 deletions(-)
> > > > 
> > > > <snip>
> > > > 
> > > > > @@ -516,7 +516,7 @@ kvm_vcpu_context_t kvm_create_vcpu(CPUState *env, int id)
> > > > >  	vcpu_ctx->kvm = kvm;
> > > > >  	vcpu_ctx->id = id;
> > > > >  
> > > > > -	r = ioctl(kvm->vm_fd, KVM_CREATE_VCPU, id);
> > > > > +	r = kvm_vm_ioctl(kvm_state, KVM_CREATE_VCPU, id);
> > > > >  	if (r == -1) {
> > > > >  		fprintf(stderr, "kvm_create_vcpu: %m\n");
> > > > 
> > > > 
> > > > int kvm_vm_ioctl(KVMState *s, int type, ...)
> > > > {
> > > >     ...
> > > >     if (ret == -1)
> > > >         ret = -errno;
> > > > }
> > > > 
> > > > Is that fine?
> > > I don't see a problem with that.
> > 
> > Problem is the sites in qemu-kvm.c kvm_vm_ioctl to return -1 
> > on error, but kvm_vm_ioctl converts -1 to -errno, no?
> 
> BTW kvm-all.c:
> 
>         if (kvm_vm_ioctl(s, KVM_GET_DIRTY_LOG, &d) == -1) {
>             dprintf("ioctl failed %d\n", errno);
>             ret = -1;
>             break;
>         }
gee, that's a real bug.

Maybe we should in fact change kvm-all to stop doing this altogether.
Anthony?