[RFC 00/12] Overhaul of error handling and module init/uninit

[RFC 00/12] Overhaul of error handling and module init/uninit

[Sasha Levin]

This patch series is really a work in progress, but is mature enough to
cover most of the different modules we have in the tool, and receive
comments regarding the work done so far and any future work.

There were three main goals while doing this work:
1. Less die(), more clean exits. We are not interested in hitting a die()
during regular program flow. die() should be reserved to extreme cases where
we are not able to report the error back.

2. Adding actual error handling where it was missing. Starting from simple
things like checking malloc()s, through making functions report their failures
(this meant switching lots of functions from 'void' to 'int' since they can
actually fail), and all the way to being able to handle errors by
uninitializing and exiting gracefully.

3. Getting initialization and uninitialization of modules somewhat
standard. The main purpose here is to be able to either add __init functions
or to be able to call all initialization functions from a single place without
code duplication.

Sasha Levin (12):
  kvm tools: Split kvm_cmd_run into init, work and uninit
  kvm tools: Fixes for symbol resolving module
  kvm tools: Fixes for IRQ module
  kvm tools: Fixes for UI modules
  kvm tools: Fixes for ioport module
  kvm tools: Fixes for ioeventfd module
  kvm tools: Fixes for serial module
  kvm tools: Fixes for mptable module
  kvm tools: Fixes for ioeventfd module
  kvm tools: Fixes for disk image module
  kvm tools: Fixes for rtc module
  kvm tools: Fixes for ioeventfd module

 tools/kvm/builtin-run.c                 |  197 +++++++++++++++++++++++++------
 tools/kvm/disk/blk.c                    |   13 ++-
 tools/kvm/disk/core.c                   |   76 ++++++++-----
 tools/kvm/disk/qcow.c                   |    2 +-
 tools/kvm/disk/raw.c                    |    9 +-
 tools/kvm/framebuffer.c                 |    9 ++-
 tools/kvm/hw/pci-shmem.c                |    7 +-
 tools/kvm/hw/rtc.c                      |   27 ++++-
 tools/kvm/hw/serial.c                   |   43 ++++++-
 tools/kvm/hw/vesa.c                     |   17 ++-
 tools/kvm/include/kvm/8250-serial.h     |    3 +-
 tools/kvm/include/kvm/disk-image.h      |    2 +-
 tools/kvm/include/kvm/framebuffer.h     |    1 +
 tools/kvm/include/kvm/ioeventfd.h       |    8 +-
 tools/kvm/include/kvm/ioport.h          |    5 +-
 tools/kvm/include/kvm/irq.h             |    3 +-
 tools/kvm/include/kvm/kvm.h             |    9 +-
 tools/kvm/include/kvm/pci.h             |    5 +-
 tools/kvm/include/kvm/rbtree-interval.h |    1 +
 tools/kvm/include/kvm/rtc.h             |    5 +-
 tools/kvm/include/kvm/sdl.h             |    7 +-
 tools/kvm/include/kvm/symbol.h          |    7 +-
 tools/kvm/include/kvm/virtio-blk.h      |    5 +-
 tools/kvm/include/kvm/virtio-pci.h      |    1 +
 tools/kvm/include/kvm/virtio-rng.h      |    4 +-
 tools/kvm/include/kvm/virtio-trans.h    |    1 +
 tools/kvm/include/kvm/vnc.h             |   10 ++-
 tools/kvm/ioeventfd.c                   |  158 +++++++++++++++++--------
 tools/kvm/ioport.c                      |   69 ++++++++++-
 tools/kvm/kvm.c                         |  101 +++++++++++-----
 tools/kvm/mmio.c                        |    8 +-
 tools/kvm/pci.c                         |   58 +++++++---
 tools/kvm/powerpc/kvm.c                 |    9 ++-
 tools/kvm/symbol.c                      |   59 +++++++--
 tools/kvm/ui/sdl.c                      |   31 ++++-
 tools/kvm/ui/vnc.c                      |   22 +++-
 tools/kvm/util/rbtree-interval.c        |   11 +-
 tools/kvm/virtio/blk.c                  |   37 ++++--
 tools/kvm/virtio/pci.c                  |   46 ++++++-
 tools/kvm/virtio/rng.c                  |   27 ++++-
 tools/kvm/virtio/trans.c                |    4 +-
 tools/kvm/x86/include/kvm/mptable.h     |    3 +-
 tools/kvm/x86/irq.c                     |   52 ++++++--
 tools/kvm/x86/kvm-cpu.c                 |   10 +-
 tools/kvm/x86/kvm.c                     |   18 +++-
 tools/kvm/x86/mptable.c                 |   20 +++-
 46 files changed, 926 insertions(+), 294 deletions(-)

-- 
1.7.8

[RFC 01/12] kvm tools: Split kvm_cmd_run into init, work and uninit

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c |   38 ++++++++++++++++++++++++++++++--------
 1 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 524ca16..0234879 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -845,15 +845,13 @@ static void kvm_run_write_sandbox_cmd(const char **argv, int argc)
 	close(fd);
 }
 
-int kvm_cmd_run(int argc, const char **argv, const char *prefix)
+static int kvm_cmd_run_init(int argc, const char **argv)
 {
 	static char real_cmdline[2048], default_name[20];
 	struct framebuffer *fb = NULL;
 	unsigned int nr_online_cpus;
-	int exit_code = 0;
 	int max_cpus, recommended_cpus;
 	int i;
-	void *ret;
 
 	signal(SIGALRM, handle_sigalrm);
 	kvm_ipc__register_handler(KVM_IPC_DEBUG, handle_debug);
@@ -1132,6 +1130,14 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	thread_pool__init(nr_online_cpus);
 	ioeventfd__start();
 
+	return 0;
+}
+
+static int kvm_cmd_run_work(void)
+{
+	int i, r = -1;
+	void *ret = NULL;
+
 	for (i = 0; i < nrcpus; i++) {
 		if (pthread_create(&kvm_cpus[i]->thread, NULL, kvm_cpu_thread, kvm_cpus[i]) != 0)
 			die("unable to create KVM VCPU thread");
@@ -1139,7 +1145,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 
 	/* Only VCPU #0 is going to exit by itself when shutting down */
 	if (pthread_join(kvm_cpus[0]->thread, &ret) != 0)
-		exit_code = 1;
+		r = 0;
 
 	for (i = 1; i < nrcpus; i++) {
 		if (kvm_cpus[i]->is_running) {
@@ -1147,10 +1153,15 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 			if (pthread_join(kvm_cpus[i]->thread, &ret) != 0)
 				die("pthread_join");
 		}
-		if (ret != NULL)
-			exit_code = 1;
+		if (ret == NULL)
+			r = 0;
 	}
 
+	return r;
+}
+
+static int kvm_cmd_run_uninit(int guest_ret)
+{
 	compat__print_all_messages();
 
 	fb__stop();
@@ -1161,8 +1172,19 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	disk_image__close_all(kvm->disks, image_count);
 	kvm__delete(kvm);
 
-	if (!exit_code)
+	if (guest_ret == 0)
 		printf("\n  # KVM session ended normally.\n");
 
-	return exit_code;
+	return 0;
+}
+
+int kvm_cmd_run(int argc, const char **argv, const char *prefix)
+{
+	int r, ret;
+
+	r = kvm_cmd_run_init(argc, argv);
+	ret = kvm_cmd_run_work();
+	r = kvm_cmd_run_uninit(ret);
+
+	return ret;
 }
-- 
1.7.8

[RFC 02/12] kvm tools: Fixes for symbol resolving module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c        |   31 ++++++++++++++-------
 tools/kvm/include/kvm/symbol.h |    7 +++-
 tools/kvm/symbol.c             |   59 +++++++++++++++++++++++++++++++---------
 tools/kvm/x86/kvm-cpu.c        |   10 +++++--
 4 files changed, 79 insertions(+), 28 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 0234879..3d046d7 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -851,7 +851,7 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	struct framebuffer *fb = NULL;
 	unsigned int nr_online_cpus;
 	int max_cpus, recommended_cpus;
-	int i;
+	int i, r;
 
 	signal(SIGALRM, handle_sigalrm);
 	kvm_ipc__register_handler(KVM_IPC_DEBUG, handle_debug);
@@ -946,8 +946,6 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	if (!script)
 		script = DEFAULT_SCRIPT;
 
-	symbol__init(vmlinux_filename);
-
 	term_init();
 
 	if (!guest_name) {
@@ -1047,7 +1045,12 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 				real_cmdline, vidmode))
 		die("unable to load kernel %s", kernel_filename);
 
-	kvm->vmlinux		= vmlinux_filename;
+	kvm->vmlinux = vmlinux_filename;
+	r = symbol__init(kvm);
+	if (r < 0) {
+		pr_error("symbol__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	ioport__setup_arch();
 
@@ -1130,7 +1133,8 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	thread_pool__init(nr_online_cpus);
 	ioeventfd__start();
 
-	return 0;
+fail:
+	return r;
 }
 
 static int kvm_cmd_run_work(void)
@@ -1160,10 +1164,16 @@ static int kvm_cmd_run_work(void)
 	return r;
 }
 
-static int kvm_cmd_run_uninit(int guest_ret)
+static void kvm_cmd_run_uninit(int guest_ret)
 {
+	int r = 0;
+
 	compat__print_all_messages();
 
+	r = symbol__uninit(kvm);
+	if (r < 0)
+		pr_warning("symbol__uninit() failed with error %d\n", r);
+
 	fb__stop();
 
 	virtio_blk__delete_all(kvm);
@@ -1174,17 +1184,18 @@ static int kvm_cmd_run_uninit(int guest_ret)
 
 	if (guest_ret == 0)
 		printf("\n  # KVM session ended normally.\n");
-
-	return 0;
 }
 
 int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 {
-	int r, ret;
+	int r, ret = -EFAULT;
 
 	r = kvm_cmd_run_init(argc, argv);
+	if (r < 0)
+		goto fail;
 	ret = kvm_cmd_run_work();
-	r = kvm_cmd_run_uninit(ret);
+	kvm_cmd_run_uninit(ret);
 
+fail:
 	return ret;
 }
diff --git a/tools/kvm/include/kvm/symbol.h b/tools/kvm/include/kvm/symbol.h
index 5bc2221..062f411 100644
--- a/tools/kvm/include/kvm/symbol.h
+++ b/tools/kvm/include/kvm/symbol.h
@@ -6,14 +6,17 @@
 
 struct kvm;
 
+#define SYMBOL_DEFAULT_UNKNOWN "<unknown>"
+
 #ifdef CONFIG_HAS_BFD
-void symbol__init(const char *vmlinux);
+int symbol__init(struct kvm *kvm);
+int symbol__uninit(struct kvm *kvm);
 char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size);
 #else
 static inline void symbol__init(const char *vmlinux) { }
 static inline char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size)
 {
-	char *s = strncpy(sym, "<unknown>", size);
+	char *s = strncpy(sym, SYMBOL_DEFAULT_UNKNOWN, size);
 	sym[size - 1] = '\0';
 	return s;
 }
diff --git a/tools/kvm/symbol.c b/tools/kvm/symbol.c
index a2b1e67..54691a4 100644
--- a/tools/kvm/symbol.c
+++ b/tools/kvm/symbol.c
@@ -2,6 +2,7 @@
 
 #include "kvm/kvm.h"
 
+#include <linux/err.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
@@ -9,19 +10,40 @@
 
 static bfd *abfd;
 
-void symbol__init(const char *vmlinux)
+int symbol__init(struct kvm *kvm)
 {
-	if (!vmlinux)
-		return;
+	int r = 0;
+
+	if (!kvm->vmlinux)
+		return -EINVAL;
 
 	bfd_init();
 
-	abfd = bfd_openr(vmlinux, NULL);
+	abfd = bfd_openr(kvm->vmlinux, NULL);
+	if (abfd == NULL) {
+		bfd_error_type err = bfd_get_error();
+
+		switch(err) {
+		case bfd_error_no_memory:
+			r = -ENOMEM;
+			break;
+		case bfd_error_invalid_target:
+			r = -EINVAL;
+			break;
+		default:
+			r = -EFAULT;
+			break;
+		}
+	}
+
+	return r;
 }
 
 static asymbol *lookup(asymbol **symbols, int nr_symbols, const char *symbol_name)
 {
-	int i;
+	int i, r;
+
+	r = -ENOENT;
 
 	for (i = 0; i < nr_symbols; i++) {
 		asymbol *symbol = symbols[i];
@@ -30,7 +52,7 @@ static asymbol *lookup(asymbol **symbols, int nr_symbols, const char *symbol_nam
 			return symbol;
 	}
 
-	return NULL;
+	return ERR_PTR(r);
 }
 
 char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size)
@@ -44,9 +66,9 @@ char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size
 	long symtab_size;
 	asymbol *symbol;
 	asymbol **syms;
-	int nr_syms;
-	char *s;
+	int nr_syms, r;
 
+	r = -ENOENT;
 	if (!abfd)
 		goto not_found;
 
@@ -57,13 +79,15 @@ char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size
 	if (!symtab_size)
 		goto not_found;
 
+	r = -ENOMEM;
 	syms = malloc(symtab_size);
 	if (!syms)
 		goto not_found;
 
 	nr_syms = bfd_canonicalize_symtab(abfd, syms);
 
-	section = bfd_get_section_by_name(abfd, ".debug_aranges");
+	r = -ENOENT;
+	section= bfd_get_section_by_name(abfd, ".debug_aranges");
 	if (!section)
 		goto not_found;
 
@@ -74,7 +98,7 @@ char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size
 		goto not_found;
 
 	symbol = lookup(syms, nr_syms, func);
-	if (!symbol)
+	if (IS_ERR(symbol))
 		goto not_found;
 
 	sym_start = bfd_asymbol_value(symbol);
@@ -90,9 +114,18 @@ char *symbol__lookup(struct kvm *kvm, unsigned long addr, char *sym, size_t size
 	return sym;
 
 not_found:
-	s = strncpy(sym, "<unknown>", size);
+	return ERR_PTR(r);
+}
 
-	sym[size - 1] = '\0';
+int symbol__uninit(struct kvm *kvm)
+{
+	bfd_boolean r = TRUE;
+
+	if (abfd)
+		r = bfd_close(abfd);
+
+	if (r == TRUE)
+		return 0;
 
-	return s;
+	return -EFAULT;
 }
diff --git a/tools/kvm/x86/kvm-cpu.c b/tools/kvm/x86/kvm-cpu.c
index 051699f..4aa1b9a 100644
--- a/tools/kvm/x86/kvm-cpu.c
+++ b/tools/kvm/x86/kvm-cpu.c
@@ -6,7 +6,7 @@
 
 #include <asm/msr-index.h>
 #include <asm/apicdef.h>
-
+#include <linux/err.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <signal.h>
@@ -324,7 +324,7 @@ void kvm_cpu__show_code(struct kvm_cpu *vcpu)
 	unsigned int code_bytes = 64;
 	unsigned int code_prologue = 43;
 	unsigned int code_len = code_bytes;
-	char sym[MAX_SYM_LEN];
+	char sym[MAX_SYM_LEN] = SYMBOL_DEFAULT_UNKNOWN, *psym;
 	unsigned char c;
 	unsigned int i;
 	u8 *ip;
@@ -340,7 +340,11 @@ void kvm_cpu__show_code(struct kvm_cpu *vcpu)
 	dprintf(debug_fd, "\n Code:\n");
 	dprintf(debug_fd,   " -----\n");
 
-	symbol__lookup(vcpu->kvm, vcpu->regs.rip, sym, MAX_SYM_LEN);
+	psym = symbol__lookup(vcpu->kvm, vcpu->regs.rip, sym, MAX_SYM_LEN);
+	if (IS_ERR(psym))
+		dprintf(debug_fd,
+			"Warning: symbol__lookup() failed to find symbol "
+			"with error: %ld\n", PTR_ERR(psym));
 
 	dprintf(debug_fd, " rip: [<%016lx>] %s\n\n", (unsigned long) vcpu->regs.rip, sym);
 
-- 
1.7.8

[RFC 03/12] kvm tools: Fixes for IRQ module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c     |   10 +++++++++-
 tools/kvm/include/kvm/irq.h |    3 ++-
 tools/kvm/x86/irq.c         |   35 ++++++++++++++++++++++++-----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 3d046d7..e40d90b 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -977,7 +977,11 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	if (!kvm_cpus)
 		die("Couldn't allocate array for %d CPUs", nrcpus);
 
-	irq__init(kvm);
+	r = irq__init(kvm);
+	if (r < 0) {
+		pr_error("irq__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	pci__init();
 
@@ -1174,6 +1178,10 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("symbol__uninit() failed with error %d\n", r);
 
+	r = irq__uninit(kvm);
+	if (r < 0)
+		pr_warning("irq__uninit() failed with error %d\n", r);
+
 	fb__stop();
 
 	virtio_blk__delete_all(kvm);
diff --git a/tools/kvm/include/kvm/irq.h b/tools/kvm/include/kvm/irq.h
index 61f593d..755d7fe 100644
--- a/tools/kvm/include/kvm/irq.h
+++ b/tools/kvm/include/kvm/irq.h
@@ -25,7 +25,8 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line);
 
 struct rb_node *irq__get_pci_tree(void);
 
-void irq__init(struct kvm *kvm);
+int irq__init(struct kvm *kvm);
+int irq__uninit(struct kvm *kvm);
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg);
 
 #endif
diff --git a/tools/kvm/x86/irq.c b/tools/kvm/x86/irq.c
index b8a7257..3683cb4 100644
--- a/tools/kvm/x86/irq.c
+++ b/tools/kvm/x86/irq.c
@@ -76,19 +76,20 @@ static int insert(struct rb_root *root, struct pci_dev *data)
 		else if (result > 0)
 			new = &((*new)->rb_right);
 		else
-			return 0;
+			return -EEXIST;
 	}
 
 	/* Add new node and rebalance tree. */
 	rb_link_node(&data->node, parent, new);
 	rb_insert_color(&data->node, root);
 
-	return 1;
+	return 0;
 }
 
 int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
 {
 	struct pci_dev *node;
+	int r;
 
 	node = search(&pci_tree, dev);
 
@@ -96,7 +97,7 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
 		/* We haven't found a node - First device of it's kind */
 		node = malloc(sizeof(*node));
 		if (node == NULL)
-			return -1;
+			return -ENOMEM;
 
 		*node = (struct pci_dev) {
 			.id	= dev,
@@ -111,9 +112,10 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
 
 		INIT_LIST_HEAD(&node->lines);
 
-		if (insert(&pci_tree, node) != 1) {
+		r = insert(&pci_tree, node);
+		if (r) {
 			free(node);
-			return -1;
+			return r;
 		}
 	}
 
@@ -121,7 +123,7 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
 		/* This device already has a pin assigned, give out a new line and device id */
 		struct irq_line *new = malloc(sizeof(*new));
 		if (new == NULL)
-			return -1;
+			return -ENOMEM;
 
 		new->line	= next_line++;
 		*line		= new->line;
@@ -133,17 +135,17 @@ int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
 		return 0;
 	}
 
-	return -1;
+	return -EFAULT;
 }
 
-void irq__init(struct kvm *kvm)
+int irq__init(struct kvm *kvm)
 {
 	int i, r;
 
 	irq_routing = calloc(sizeof(struct kvm_irq_routing) +
 			IRQ_MAX_GSI * sizeof(struct kvm_irq_routing_entry), 1);
 	if (irq_routing == NULL)
-		die("Failed allocating space for GSI table");
+		return -ENOMEM;
 
 	/* Hook first 8 GSIs to master IRQCHIP */
 	for (i = 0; i < 8; i++)
@@ -163,8 +165,19 @@ void irq__init(struct kvm *kvm)
 	}
 
 	r = ioctl(kvm->vm_fd, KVM_SET_GSI_ROUTING, irq_routing);
-	if (r)
-		die("Failed setting GSI routes");
+	if (r) {
+		free(irq_routing);
+		return errno;
+	}
+
+	return 0;
+}
+
+int irq__uninit(struct kvm *kvm)
+{
+	free(irq_routing);
+
+	return 0;
 }
 
 int irq__add_msix_route(struct kvm *kvm, struct msi_msg *msg)
-- 
1.7.8

[RFC 04/12] kvm tools: Fixes for UI modules

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c             |   35 ++++++++++++++++++++++++++---------
 tools/kvm/framebuffer.c             |    9 ++++++++-
 tools/kvm/hw/vesa.c                 |   11 +++++++----
 tools/kvm/include/kvm/framebuffer.h |    1 +
 tools/kvm/include/kvm/sdl.h         |    7 ++++++-
 tools/kvm/include/kvm/vnc.h         |   10 ++++++++--
 tools/kvm/ui/sdl.c                  |   31 +++++++++++++++++++++++++++----
 tools/kvm/ui/vnc.c                  |   22 +++++++++++++++++-----
 8 files changed, 100 insertions(+), 26 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index e40d90b..37bc3f2 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -35,6 +35,7 @@
 #include "kvm/builtin-debug.h"
 
 #include <linux/types.h>
+#include <linux/err.h>
 
 #include <sys/utsname.h>
 #include <sys/types.h>
@@ -992,8 +993,9 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	if (vnc || sdl) {
 		if (vidmode == -1)
 			vidmode = 0x312;
-	} else
+	} else {
 		vidmode = 0;
+	}
 
 	memset(real_cmdline, 0, sizeof(real_cmdline));
 	kvm__arch_set_cmdline(real_cmdline, vnc || sdl);
@@ -1105,20 +1107,35 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	pci_shmem__init(kvm);
 
-	if (vnc || sdl)
+	if (vnc || sdl) {
 		fb = vesa__init(kvm);
+		if (IS_ERR(fb)) {
+			pr_error("vesa__init() failed with error %ld\n", PTR_ERR(fb));
+			goto fail;
+		}
+	}
 
-	if (vnc) {
-		if (fb)
-			vnc__init(fb);
+	if (vnc && fb) {
+		r = vnc__init(fb);
+		if (r < 0) {
+			pr_error("vnc__init() failed with error %d\n", r);
+			goto fail;
+		}
 	}
 
-	if (sdl) {
-		if (fb)
-			sdl__init(fb);
+	if (sdl && fb) {
+		sdl__init(fb);
+		if (r < 0) {
+			pr_error("sdl__init() failed with error %d\n", r);
+			goto fail;
+		}
 	}
 
-	fb__start();
+	r = fb__start();
+	if (r < 0) {
+		pr_error("fb__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	/* Device init all done; firmware init must
 	 * come after this (it may set up device trees etc.)
diff --git a/tools/kvm/framebuffer.c b/tools/kvm/framebuffer.c
index b6eb1ac..e15b717 100644
--- a/tools/kvm/framebuffer.c
+++ b/tools/kvm/framebuffer.c
@@ -4,6 +4,7 @@
 #include <linux/list.h>
 #include <stdlib.h>
 #include <sys/mman.h>
+#include <errno.h>
 
 static LIST_HEAD(framebuffers);
 
@@ -18,7 +19,7 @@ struct framebuffer *fb__register(struct framebuffer *fb)
 int fb__attach(struct framebuffer *fb, struct fb_target_operations *ops)
 {
 	if (fb->nr_targets >= FB_MAX_TARGETS)
-		return -1;
+		return -ENOSPC;
 
 	fb->targets[fb->nr_targets++] = ops;
 
@@ -63,6 +64,12 @@ void fb__stop(void)
 	struct framebuffer *fb;
 
 	list_for_each_entry(fb, &framebuffers, node) {
+		u32 i;
+
+		for (i = 0; i < fb->nr_targets; i++)
+			if (fb->targets[i]->stop)
+				fb->targets[i]->stop(fb);
+
 		munmap(fb->mem, fb->mem_size);
 	}
 }
diff --git a/tools/kvm/hw/vesa.c b/tools/kvm/hw/vesa.c
index 63f1082..2c0101f 100644
--- a/tools/kvm/hw/vesa.c
+++ b/tools/kvm/hw/vesa.c
@@ -8,9 +8,10 @@
 #include "kvm/irq.h"
 #include "kvm/kvm.h"
 #include "kvm/pci.h"
+
 #include <linux/byteorder.h>
 #include <sys/mman.h>
-
+#include <linux/err.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
 #include <inttypes.h>
@@ -50,9 +51,11 @@ struct framebuffer *vesa__init(struct kvm *kvm)
 	u16 vesa_base_addr;
 	u8 dev, line, pin;
 	char *mem;
+	int r;
 
-	if (irq__register_device(PCI_DEVICE_ID_VESA, &dev, &pin, &line) < 0)
-		return NULL;
+	r = irq__register_device(PCI_DEVICE_ID_VESA, &dev, &pin, &line);
+	if (r < 0)
+		return ERR_PTR(r);
 
 	vesa_pci_device.irq_pin		= pin;
 	vesa_pci_device.irq_line	= line;
@@ -62,7 +65,7 @@ struct framebuffer *vesa__init(struct kvm *kvm)
 
 	mem = mmap(NULL, VESA_MEM_SIZE, PROT_RW, MAP_ANON_NORESERVE, -1, 0);
 	if (mem == MAP_FAILED)
-		return NULL;
+		ERR_PTR(-errno);
 
 	kvm__register_mem(kvm, VESA_MEM_ADDR, VESA_MEM_SIZE, mem);
 
diff --git a/tools/kvm/include/kvm/framebuffer.h b/tools/kvm/include/kvm/framebuffer.h
index b66d0ba..dc5022c 100644
--- a/tools/kvm/include/kvm/framebuffer.h
+++ b/tools/kvm/include/kvm/framebuffer.h
@@ -8,6 +8,7 @@ struct framebuffer;
 
 struct fb_target_operations {
 	int (*start)(struct framebuffer *fb);
+	int (*stop)(struct framebuffer *fb);
 };
 
 #define FB_MAX_TARGETS			2
diff --git a/tools/kvm/include/kvm/sdl.h b/tools/kvm/include/kvm/sdl.h
index a5aa411..d0f83e0 100644
--- a/tools/kvm/include/kvm/sdl.h
+++ b/tools/kvm/include/kvm/sdl.h
@@ -6,12 +6,17 @@
 struct framebuffer;
 
 #ifdef CONFIG_HAS_SDL
-void sdl__init(struct framebuffer *fb);
+int sdl__init(struct framebuffer *fb);
+int sdl__uninit(struct framebuffer *fb);
 #else
 static inline void sdl__init(struct framebuffer *fb)
 {
 	die("SDL support not compiled in. (install the SDL-dev[el] package)");
 }
+static inline void sdl__uninit(struct framebuffer *fb)
+{
+	die("SDL support not compiled in. (install the SDL-dev[el] package)");
+}
 #endif
 
 #endif /* KVM__SDL_H */
diff --git a/tools/kvm/include/kvm/vnc.h b/tools/kvm/include/kvm/vnc.h
index da2f635..7f1df5d 100644
--- a/tools/kvm/include/kvm/vnc.h
+++ b/tools/kvm/include/kvm/vnc.h
@@ -4,10 +4,16 @@
 struct framebuffer;
 
 #ifdef CONFIG_HAS_VNCSERVER
-void vnc__init(struct framebuffer *fb);
+int vnc__init(struct framebuffer *fb);
+int vnc__uninit(struct framebuffer *fb);
 #else
-static inline void vnc__init(struct framebuffer *fb)
+static inline int vnc__init(struct framebuffer *fb)
 {
+	return 0;
+}
+static inline int vnc__uninit(struct framebuffer *fb)
+{
+	return 0;
 }
 #endif
 
diff --git a/tools/kvm/ui/sdl.c b/tools/kvm/ui/sdl.c
index 5bf11fa..36e46fa 100644
--- a/tools/kvm/ui/sdl.c
+++ b/tools/kvm/ui/sdl.c
@@ -137,6 +137,7 @@ static const struct set2_scancode const keymap[255] = {
 	[118]	= DEFINE_ESC(0x70),	/* <ins> */
 	[119]	= DEFINE_ESC(0x71),	/* <delete> */
 };
+static bool running, done;
 
 static const struct set2_scancode *to_code(u8 scancode)
 {
@@ -225,7 +226,7 @@ static void *sdl__thread(void *p)
 
 	SDL_EnableKeyRepeat(200, 50);
 
-	for (;;) {
+	while (running) {
 		SDL_BlitSurface(guest_screen, NULL, screen, NULL);
 		SDL_Flip(screen);
 
@@ -254,6 +255,11 @@ static void *sdl__thread(void *p)
 
 		SDL_Delay(1000 / FRAME_RATE);
 	}
+
+	if (running == false && done == false) {
+		done = true;
+		return NULL;
+	}
 exit:
 	kvm_cpu__reboot();
 
@@ -264,17 +270,34 @@ static int sdl__start(struct framebuffer *fb)
 {
 	pthread_t thread;
 
+	running = true;
+
 	if (pthread_create(&thread, NULL, sdl__thread, fb) != 0)
 		return -1;
 
 	return 0;
 }
 
+static int sdl__stop(struct framebuffer *fb)
+{
+	running = false;
+	while (done == false)
+		sleep(0);
+
+	return 0;
+}
+
 static struct fb_target_operations sdl_ops = {
-	.start			= sdl__start,
+	.start	= sdl__start,
+	.start	= sdl__stop,
 };
 
-void sdl__init(struct framebuffer *fb)
+int sdl__init(struct framebuffer *fb)
+{
+	return fb__attach(fb, &sdl_ops);
+}
+
+int sdl__uninit(struct framebuffer *fb)
 {
-	fb__attach(fb, &sdl_ops);
+	return sdl__stop(fb);
 }
diff --git a/tools/kvm/ui/vnc.c b/tools/kvm/ui/vnc.c
index d760492..344e6b7 100644
--- a/tools/kvm/ui/vnc.c
+++ b/tools/kvm/ui/vnc.c
@@ -30,6 +30,7 @@ static char letters[26] = {
 	0x1a,
 };
 
+static rfbScreenInfoPtr server;
 static char num[10] = {
 	0x45, 0x16, 0x1e, 0x26, 0x2e, 0x23, 0x36, 0x3d, 0x3e, 0x46,
 };
@@ -182,8 +183,6 @@ static void *vnc__thread(void *p)
 	char argv[1][1] = {{0}};
 	int argc = 1;
 
-	rfbScreenInfoPtr server;
-
 	server = rfbGetScreen(&argc, (char **) argv, fb->width, fb->height, 8, 3, 4);
 	server->frameBuffer		= fb->mem;
 	server->alwaysShared		= TRUE;
@@ -208,11 +207,24 @@ static int vnc__start(struct framebuffer *fb)
 	return 0;
 }
 
+static int vnc__stop(struct framebuffer *fb)
+{
+	rfbShutdownServer(server, TRUE);
+
+	return 0;
+}
+
 static struct fb_target_operations vnc_ops = {
-	.start			= vnc__start,
+	.start	= vnc__start,
+	.stop	= vnc__stop,
 };
 
-void vnc__init(struct framebuffer *fb)
+int vnc__init(struct framebuffer *fb)
 {
-	fb__attach(fb, &vnc_ops);
+	return fb__attach(fb, &vnc_ops);
 }
+
+int vnc__uninit(struct framebuffer *fb)
+{
+	return vnc__stop(fb);
+}
\ No newline at end of file
-- 
1.7.8

[RFC 05/12] kvm tools: Fixes for ioport module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c                 |   11 ++++++
 tools/kvm/hw/pci-shmem.c                |    7 +++-
 tools/kvm/hw/vesa.c                     |    6 +++-
 tools/kvm/include/kvm/ioport.h          |    5 ++-
 tools/kvm/include/kvm/rbtree-interval.h |    1 +
 tools/kvm/ioport.c                      |   60 +++++++++++++++++++++++++++++-
 tools/kvm/util/rbtree-interval.c        |    2 -
 tools/kvm/virtio/pci.c                  |    7 +++-
 8 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 37bc3f2..5388171 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -986,6 +986,12 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	pci__init();
 
+	r = ioport__init(kvm);
+	if (r < 0) {
+		pr_error("ioport__init() failed with error %d\n", r);
+		goto fail;
+	}
+
 	/*
 	 * vidmode should be either specified
 	 * either set by default
@@ -1205,6 +1211,11 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	virtio_rng__delete_all(kvm);
 
 	disk_image__close_all(kvm->disks, image_count);
+
+	r = ioport__uninit(kvm);
+	if (r < 0)
+		pr_warning("ioport__uninit() failed with error %d\n", r);
+
 	kvm__delete(kvm);
 
 	if (guest_ret == 0)
diff --git a/tools/kvm/hw/pci-shmem.c b/tools/kvm/hw/pci-shmem.c
index 6130131..8bac151 100644
--- a/tools/kvm/hw/pci-shmem.c
+++ b/tools/kvm/hw/pci-shmem.c
@@ -219,6 +219,7 @@ int pci_shmem__init(struct kvm *kvm)
 {
 	u8 dev, line, pin;
 	char *mem;
+	int r;
 
 	if (shmem_region == 0)
 		return 0;
@@ -231,7 +232,11 @@ int pci_shmem__init(struct kvm *kvm)
 	pci_shmem_pci_device.irq_line = line;
 
 	/* Register MMIO space for MSI-X */
-	ivshmem_registers = ioport__register(IOPORT_EMPTY, &shmem_pci__io_ops, IOPORT_SIZE, NULL);
+	r = ioport__register(IOPORT_EMPTY, &shmem_pci__io_ops, IOPORT_SIZE, NULL);
+	if (r < 0)
+		return r;
+	ivshmem_registers = (u16)r;
+
 	msix_block = pci_get_io_space_block(0x1010);
 	kvm__register_mmio(kvm, msix_block, 0x1010, false, callback_mmio_msix, NULL);
 
diff --git a/tools/kvm/hw/vesa.c b/tools/kvm/hw/vesa.c
index 2c0101f..757f0a2 100644
--- a/tools/kvm/hw/vesa.c
+++ b/tools/kvm/hw/vesa.c
@@ -57,9 +57,13 @@ struct framebuffer *vesa__init(struct kvm *kvm)
 	if (r < 0)
 		return ERR_PTR(r);
 
+	r = ioport__register(IOPORT_EMPTY, &vesa_io_ops, IOPORT_SIZE, NULL);
+	if (r < 0)
+		return ERR_PTR(r);
+
 	vesa_pci_device.irq_pin		= pin;
 	vesa_pci_device.irq_line	= line;
-	vesa_base_addr			= ioport__register(IOPORT_EMPTY, &vesa_io_ops, IOPORT_SIZE, NULL);
+	vesa_base_addr			= (u16)r;
 	vesa_pci_device.bar[0]		= cpu_to_le32(vesa_base_addr | PCI_BASE_ADDRESS_SPACE_IO);
 	pci__register(&vesa_pci_device, dev);
 
diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 09bf876..f47b347 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -31,7 +31,10 @@ struct ioport_operations {
 
 void ioport__setup_arch(void);
 
-u16 ioport__register(u16 port, struct ioport_operations *ops, int count, void *param);
+int ioport__register(u16 port, struct ioport_operations *ops, int count, void *param);
+int ioport__unregister(u16 port);
+int ioport__init(struct kvm *kvm);
+int ioport__uninit(struct kvm *kvm);
 
 static inline u8 ioport__read8(u8 *data)
 {
diff --git a/tools/kvm/include/kvm/rbtree-interval.h b/tools/kvm/include/kvm/rbtree-interval.h
index a6688c4..13245ba 100644
--- a/tools/kvm/include/kvm/rbtree-interval.h
+++ b/tools/kvm/include/kvm/rbtree-interval.h
@@ -5,6 +5,7 @@
 #include <linux/types.h>
 
 #define RB_INT_INIT(l, h) (struct rb_int_node){.low = l, .high = h}
+#define rb_int(n) rb_entry(n, struct rb_int_node, node)
 
 struct rb_int_node {
 	struct rb_node	node;
diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c
index b417942..19caf6c 100644
--- a/tools/kvm/ioport.c
+++ b/tools/kvm/ioport.c
@@ -52,7 +52,12 @@ static int ioport_insert(struct rb_root *root, struct ioport *data)
 	return rb_int_insert(root, &data->node);
 }
 
-u16 ioport__register(u16 port, struct ioport_operations *ops, int count, void *param)
+static void ioport_remove(struct rb_root *root, struct ioport *data)
+{
+	rb_int_erase(root, &data->node);
+}
+
+int ioport__register(u16 port, struct ioport_operations *ops, int count, void *param)
 {
 	struct ioport *entry;
 
@@ -68,7 +73,7 @@ u16 ioport__register(u16 port, struct ioport_operations *ops, int count, void *p
 
 	entry = malloc(sizeof(*entry));
 	if (entry == NULL)
-		die("Failed allocating new ioport entry");
+		return -ENOMEM;
 
 	*entry = (struct ioport) {
 		.node	= RB_INT_INIT(port, port + count),
@@ -83,6 +88,46 @@ u16 ioport__register(u16 port, struct ioport_operations *ops, int count, void *p
 	return port;
 }
 
+int ioport__unregister(u16 port)
+{
+	struct ioport *entry;
+	int r;
+
+	br_write_lock();
+
+	r = -ENOENT;
+	entry = ioport_search(&ioport_tree, port);
+	if (!entry)
+		goto done;
+
+	ioport_remove(&ioport_tree, entry);
+
+	free(entry);
+
+	r = 0;
+
+done:
+	br_write_unlock();
+
+	return r;
+}
+
+static void ioport__unregister_all(void)
+{
+	struct ioport *entry;
+	struct rb_node *rb;
+	struct rb_int_node *rb_node;
+
+	rb = rb_first(&ioport_tree);
+	while (rb) {
+		rb_node = rb_int(rb);
+		entry = ioport_node(rb_node);
+		ioport_remove(&ioport_tree, entry);
+		free(entry);
+		rb = rb_first(&ioport_tree);
+	}
+}
+
 static const char *to_direction(int direction)
 {
 	if (direction == KVM_EXIT_IO_IN)
@@ -133,3 +178,14 @@ error:
 
 	return !ioport_debug;
 }
+
+int ioport__init(struct kvm *kvm)
+{
+	return 0;
+}
+
+int ioport__uninit(struct kvm *kvm)
+{
+	ioport__unregister_all();
+	return 0;
+}
\ No newline at end of file
diff --git a/tools/kvm/util/rbtree-interval.c b/tools/kvm/util/rbtree-interval.c
index d02fbf0..edc140d 100644
--- a/tools/kvm/util/rbtree-interval.c
+++ b/tools/kvm/util/rbtree-interval.c
@@ -1,8 +1,6 @@
 #include <kvm/rbtree-interval.h>
 #include <stddef.h>
 
-#define rb_int(n) rb_entry(n, struct rb_int_node, node)
-
 struct rb_int_node *rb_int_search_single(struct rb_root *root, u64 point)
 {
 	struct rb_node *node = root->rb_node;
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index 3051314..2643d8a 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -284,11 +284,16 @@ int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev,
 {
 	struct virtio_pci *vpci = vtrans->virtio;
 	u8 pin, line, ndev;
+	int r;
 
 	vpci->dev = dev;
 	vpci->msix_io_block = pci_get_io_space_block(PCI_IO_SIZE * 2);
 
-	vpci->base_addr = ioport__register(IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vtrans);
+	r = ioport__register(IOPORT_EMPTY, &virtio_pci__io_ops, IOPORT_SIZE, vtrans);
+	if (r < 0)
+		return r;
+
+	vpci->base_addr = (u16)r;
 	kvm__register_mmio(kvm, vpci->msix_io_block, PCI_IO_SIZE, false, callback_mmio_table, vpci);
 
 	vpci->pci_hdr = (struct pci_device_header) {
-- 
1.7.8

[RFC 06/12] kvm tools: Fixes for ioeventfd module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c           |   12 ++-
 tools/kvm/include/kvm/ioeventfd.h |    8 +-
 tools/kvm/ioeventfd.c             |  158 ++++++++++++++++++++++++++-----------
 tools/kvm/virtio/pci.c            |    5 +-
 4 files changed, 128 insertions(+), 55 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 5388171..b7e7977 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -958,7 +958,11 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	kvm->single_step = single_step;
 
-	ioeventfd__init(kvm);
+	r = ioeventfd__init(kvm);
+	if (r < 0) {
+		pr_error("ioeventfd__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	max_cpus = kvm__max_cpus(kvm);
 	recommended_cpus = kvm__recommended_cpus(kvm);
@@ -1158,8 +1162,6 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	}
 
 	thread_pool__init(nr_online_cpus);
-	ioeventfd__start();
-
 fail:
 	return r;
 }
@@ -1216,6 +1218,10 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("ioport__uninit() failed with error %d\n", r);
 
+	r = ioeventfd__uninit(kvm);
+	if (r < 0)
+		pr_warning("ioeventfd__uninit() failed with error %d\n", r);
+
 	kvm__delete(kvm);
 
 	if (guest_ret == 0)
diff --git a/tools/kvm/include/kvm/ioeventfd.h b/tools/kvm/include/kvm/ioeventfd.h
index 3a95788..cb8a21b 100644
--- a/tools/kvm/include/kvm/ioeventfd.h
+++ b/tools/kvm/include/kvm/ioeventfd.h
@@ -19,9 +19,9 @@ struct ioevent {
 	struct list_head	list;
 };
 
-void ioeventfd__init(struct kvm *kvm);
-void ioeventfd__start(void);
-void ioeventfd__add_event(struct ioevent *ioevent);
-void ioeventfd__del_event(u64 addr, u64 datamatch);
+int ioeventfd__init(struct kvm *kvm);
+int ioeventfd__uninit(struct kvm *kvm);
+int ioeventfd__add_event(struct ioevent *ioevent);
+int ioeventfd__del_event(u64 addr, u64 datamatch);
 
 #endif
diff --git a/tools/kvm/ioeventfd.c b/tools/kvm/ioeventfd.c
index 1d6a7ac..bb248d7 100644
--- a/tools/kvm/ioeventfd.c
+++ b/tools/kvm/ioeventfd.c
@@ -16,34 +16,117 @@
 #define IOEVENTFD_MAX_EVENTS	20
 
 static struct	epoll_event events[IOEVENTFD_MAX_EVENTS];
-static int	epoll_fd;
+static int	epoll_fd, epoll_stop_fd;
 static LIST_HEAD(used_ioevents);
 static bool	ioeventfd_avail;
 
-void ioeventfd__init(struct kvm *kvm)
+static void *ioeventfd__thread(void *param)
 {
+	u64 tmp = 1;
+
+	for (;;) {
+		int nfds, i;
+
+		nfds = epoll_wait(epoll_fd, events, IOEVENTFD_MAX_EVENTS, -1);
+		for (i = 0; i < nfds; i++) {
+			struct ioevent *ioevent;
+
+			if (events[i].data.fd == epoll_stop_fd)
+				goto done;
+
+			ioevent = events[i].data.ptr;
+
+			if (read(ioevent->fd, &tmp, sizeof(tmp)) < 0)
+				die("Failed reading event");
+
+			ioevent->fn(ioevent->fn_kvm, ioevent->fn_ptr);
+		}
+	}
+
+done:
+	tmp = 1;
+	tmp = write(epoll_stop_fd, &tmp, sizeof(tmp));
+
+	return NULL;
+}
+
+static int ioeventfd__start(void)
+{
+	pthread_t thread;
+
+	if (!ioeventfd_avail)
+		return -ENOSYS;
+
+	return pthread_create(&thread, NULL, ioeventfd__thread, NULL);
+}
+
+int ioeventfd__init(struct kvm *kvm)
+{
+	struct epoll_event epoll_event = {.events = EPOLLIN};
+	int r;
+
 	ioeventfd_avail = kvm__supports_extension(kvm, KVM_CAP_IOEVENTFD);
 	if (!ioeventfd_avail)
-		return;
+		return -ENOSYS;
 
 	epoll_fd = epoll_create(IOEVENTFD_MAX_EVENTS);
 	if (epoll_fd < 0)
-		die("Failed creating epoll fd");
+		return -errno;
+
+	epoll_stop_fd = eventfd(0, 0);
+	epoll_event.data.fd = epoll_stop_fd;
+
+	r = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, epoll_stop_fd, &epoll_event);
+	if (r < 0)
+		goto cleanup;
+
+	r = ioeventfd__start();
+	if (r < 0)
+		goto cleanup;
+
+	r = 0;
+
+	return r;
+
+cleanup:
+	close(epoll_stop_fd);
+	close(epoll_fd);
+
+	return r;
 }
 
-void ioeventfd__add_event(struct ioevent *ioevent)
+int ioeventfd__uninit(struct kvm *kvm)
+{
+	u64 tmp = 1;
+	int r;
+
+	r = write(epoll_stop_fd, &tmp, sizeof(tmp));
+	if (r < 0)
+		return r;
+
+	r = read(epoll_stop_fd, &tmp, sizeof(tmp));
+	if (r < 0)
+		return r;
+
+	close(epoll_fd);
+	close(epoll_stop_fd);
+
+	return 0;
+}
+
+int ioeventfd__add_event(struct ioevent *ioevent)
 {
 	struct kvm_ioeventfd kvm_ioevent;
 	struct epoll_event epoll_event;
 	struct ioevent *new_ioevent;
-	int event;
+	int event, r;
 
 	if (!ioeventfd_avail)
-		return;
+		return -ENOSYS;
 
 	new_ioevent = malloc(sizeof(*new_ioevent));
 	if (new_ioevent == NULL)
-		die("Failed allocating memory for new ioevent");
+		return -ENOMEM;
 
 	*new_ioevent = *ioevent;
 	event = new_ioevent->fd;
@@ -56,28 +139,40 @@ void ioeventfd__add_event(struct ioevent *ioevent)
 		.flags			= KVM_IOEVENTFD_FLAG_PIO | KVM_IOEVENTFD_FLAG_DATAMATCH,
 	};
 
-	if (ioctl(ioevent->fn_kvm->vm_fd, KVM_IOEVENTFD, &kvm_ioevent) != 0)
-		die("Failed creating new ioeventfd");
+	r = ioctl(ioevent->fn_kvm->vm_fd, KVM_IOEVENTFD, &kvm_ioevent);
+	if (r) {
+		r = -errno;
+		goto cleanup;
+	}
 
 	epoll_event = (struct epoll_event) {
 		.events			= EPOLLIN,
 		.data.ptr		= new_ioevent,
 	};
 
-	if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, event, &epoll_event) != 0)
-		die("Failed assigning new event to the epoll fd");
+	r = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, event, &epoll_event);
+	if (r) {
+		r = -errno;
+		goto cleanup;
+	}
 
 	list_add_tail(&new_ioevent->list, &used_ioevents);
+
+	return 0;
+
+cleanup:
+	free(new_ioevent);
+	return r;
 }
 
-void ioeventfd__del_event(u64 addr, u64 datamatch)
+int ioeventfd__del_event(u64 addr, u64 datamatch)
 {
 	struct kvm_ioeventfd kvm_ioevent;
 	struct ioevent *ioevent;
 	u8 found = 0;
 
 	if (!ioeventfd_avail)
-		return;
+		return -ENOSYS;
 
 	list_for_each_entry(ioevent, &used_ioevents, list) {
 		if (ioevent->io_addr == addr) {
@@ -87,7 +182,7 @@ void ioeventfd__del_event(u64 addr, u64 datamatch)
 	}
 
 	if (found == 0 || ioevent == NULL)
-		return;
+		return -ENOENT;
 
 	kvm_ioevent = (struct kvm_ioeventfd) {
 		.addr			= ioevent->io_addr,
@@ -106,37 +201,6 @@ void ioeventfd__del_event(u64 addr, u64 datamatch)
 
 	close(ioevent->fd);
 	free(ioevent);
-}
-
-static void *ioeventfd__thread(void *param)
-{
-	for (;;) {
-		int nfds, i;
-
-		nfds = epoll_wait(epoll_fd, events, IOEVENTFD_MAX_EVENTS, -1);
-		for (i = 0; i < nfds; i++) {
-			u64 tmp;
-			struct ioevent *ioevent;
-
-			ioevent = events[i].data.ptr;
-
-			if (read(ioevent->fd, &tmp, sizeof(tmp)) < 0)
-				die("Failed reading event");
-
-			ioevent->fn(ioevent->fn_kvm, ioevent->fn_ptr);
-		}
-	}
-
-	return NULL;
-}
-
-void ioeventfd__start(void)
-{
-	pthread_t thread;
-
-	if (!ioeventfd_avail)
-		return;
 
-	if (pthread_create(&thread, NULL, ioeventfd__thread, NULL) != 0)
-		die("Failed starting ioeventfd thread");
+	return 0;
 }
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index 2643d8a..7cc014e 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -34,6 +34,7 @@ static int virtio_pci__init_ioeventfd(struct kvm *kvm, struct virtio_trans *vtra
 {
 	struct ioevent ioevent;
 	struct virtio_pci *vpci = vtrans->virtio;
+	int r;
 
 	vpci->ioeventfds[vq] = (struct virtio_pci_ioevent_param) {
 		.vtrans		= vtrans,
@@ -50,7 +51,9 @@ static int virtio_pci__init_ioeventfd(struct kvm *kvm, struct virtio_trans *vtra
 		.fd		= eventfd(0, 0),
 	};
 
-	ioeventfd__add_event(&ioevent);
+	r = ioeventfd__add_event(&ioevent);
+	if (r)
+		return r;
 
 	if (vtrans->virtio_ops->notify_vq_eventfd)
 		vtrans->virtio_ops->notify_vq_eventfd(kvm, vpci->dev, vq, ioevent.fd);
-- 
1.7.8

[RFC 07/12] kvm tools: Fixes for serial module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c             |   11 ++++++++-
 tools/kvm/hw/serial.c               |   43 ++++++++++++++++++++++++++++++----
 tools/kvm/include/kvm/8250-serial.h |    3 +-
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index b7e7977..eef3f80 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1072,7 +1072,11 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	rtc__init();
 
-	serial8250__init(kvm);
+	r = serial8250__init(kvm);
+	if (r < 0) {
+		pr_error("serial__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	if (active_console == CONSOLE_VIRTIO)
 		virtio_console__init(kvm);
@@ -1180,6 +1184,7 @@ static int kvm_cmd_run_work(void)
 	if (pthread_join(kvm_cpus[0]->thread, &ret) != 0)
 		r = 0;
 
+	kvm_cpus[0] = NULL;
 	for (i = 1; i < nrcpus; i++) {
 		if (kvm_cpus[i]->is_running) {
 			pthread_kill(kvm_cpus[i]->thread, SIGKVMEXIT);
@@ -1214,6 +1219,10 @@ static void kvm_cmd_run_uninit(int guest_ret)
 
 	disk_image__close_all(kvm->disks, image_count);
 
+	r = serial8250__uninit(kvm);
+	if (r < 0)
+		pr_warning("serial8250__uninit() failed with error %d\n", r);
+
 	r = ioport__uninit(kvm);
 	if (r < 0)
 		pr_warning("ioport__uninit() failed with error %d\n", r);
diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
index c0f6970..561fc72 100644
--- a/tools/kvm/hw/serial.c
+++ b/tools/kvm/hw/serial.c
@@ -339,19 +339,52 @@ static struct ioport_operations serial8250_ops = {
 	.io_out		= serial8250_out,
 };
 
-static void serial8250__device_init(struct kvm *kvm, struct serial8250_device *dev)
+static int serial8250__device_init(struct kvm *kvm, struct serial8250_device *dev)
 {
-	ioport__register(dev->iobase, &serial8250_ops, 8, NULL);
+	int r;
+
+	r = ioport__register(dev->iobase, &serial8250_ops, 8, NULL);
 	kvm__irq_line(kvm, dev->irq, 0);
+
+	return r;
 }
 
-void serial8250__init(struct kvm *kvm)
+int serial8250__init(struct kvm *kvm)
 {
-	unsigned int i;
+	unsigned int i, j;
+	int r = 0;
 
 	for (i = 0; i < ARRAY_SIZE(devices); i++) {
 		struct serial8250_device *dev = &devices[i];
 
-		serial8250__device_init(kvm, dev);
+		r = serial8250__device_init(kvm, dev);
+		if (r < 0)
+			goto cleanup;
+	}
+
+	return r;
+cleanup:
+	for (j = 0; j <= i; j++) {
+		struct serial8250_device *dev = &devices[j];
+
+		ioport__unregister(dev->iobase);
 	}
+
+	return r;
 }
+
+int serial8250__uninit(struct kvm *kvm)
+{
+	unsigned int i;
+	int r;
+
+	for (i = 0; i < ARRAY_SIZE(devices); i++) {
+		struct serial8250_device *dev = &devices[i];
+
+		r = ioport__unregister(dev->iobase);
+		if (r < 0)
+			return r;
+	}
+
+	return 0;
+}
\ No newline at end of file
diff --git a/tools/kvm/include/kvm/8250-serial.h b/tools/kvm/include/kvm/8250-serial.h
index 8bd8798..1800cbf 100644
--- a/tools/kvm/include/kvm/8250-serial.h
+++ b/tools/kvm/include/kvm/8250-serial.h
@@ -3,7 +3,8 @@
 
 struct kvm;
 
-void serial8250__init(struct kvm *kvm);
+int serial8250__init(struct kvm *kvm);
+int serial8250__uninit(struct kvm *kvm);
 void serial8250__update_consoles(struct kvm *kvm);
 void serial8250__inject_sysrq(struct kvm *kvm);
 
-- 
1.7.8

[RFC 08/12] kvm tools: Fixes for mptable module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c             |    8 ++++++++
 tools/kvm/include/kvm/kvm.h         |    3 ++-
 tools/kvm/powerpc/kvm.c             |    9 ++++++++-
 tools/kvm/x86/include/kvm/mptable.h |    3 ++-
 tools/kvm/x86/kvm.c                 |   18 ++++++++++++++++--
 tools/kvm/x86/mptable.c             |   20 +++++++++++++++-----
 6 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index eef3f80..2be3949 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1158,6 +1158,10 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 	kvm__start_timer(kvm);
 
 	kvm__arch_setup_firmware(kvm);
+	if (r < 0) {
+		pr_error("kvm__arch_setup_firmware() failed with error %d\n", r);
+		goto fail;
+	}
 
 	for (i = 0; i < nrcpus; i++) {
 		kvm_cpus[i] = kvm_cpu__init(kvm, i);
@@ -1223,6 +1227,10 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("serial8250__uninit() failed with error %d\n", r);
 
+	r = kvm__arch_free_firmware(kvm);
+	if (r < 0)
+		pr_warning("kvm__arch_free_firmware() failed with error %d\n", r);
+
 	r = ioport__uninit(kvm);
 	if (r < 0)
 		pr_warning("ioport__uninit() failed with error %d\n", r);
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 8a0c01d..74f46c1 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -55,7 +55,8 @@ void kvm__remove_socket(const char *name);
 
 void kvm__arch_set_cmdline(char *cmdline, bool video);
 void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char *hugetlbfs_path, u64 ram_size, const char *name);
-void kvm__arch_setup_firmware(struct kvm *kvm);
+int kvm__arch_setup_firmware(struct kvm *kvm);
+int kvm__arch_free_firmware(struct kvm *kvm);
 bool kvm__arch_cpu_supports_vm(void);
 void kvm__arch_periodic_poll(struct kvm *kvm);
 
diff --git a/tools/kvm/powerpc/kvm.c b/tools/kvm/powerpc/kvm.c
index f838a8f..67d042c 100644
--- a/tools/kvm/powerpc/kvm.c
+++ b/tools/kvm/powerpc/kvm.c
@@ -176,7 +176,7 @@ static void setup_fdt(struct kvm *kvm)
 /**
  * kvm__arch_setup_firmware
  */
-void kvm__arch_setup_firmware(struct kvm *kvm)
+int kvm__arch_setup_firmware(struct kvm *kvm)
 {
 	/* Load RTAS */
 
@@ -184,4 +184,11 @@ void kvm__arch_setup_firmware(struct kvm *kvm)
 
 	/* Init FDT */
 	setup_fdt(kvm);
+
+	return 0;
+}
+
+int kvm__arch_free_firmware(struct kvm *kvm)
+{
+	return 0;
 }
diff --git a/tools/kvm/x86/include/kvm/mptable.h b/tools/kvm/x86/include/kvm/mptable.h
index 8557ae8..cfc1c72 100644
--- a/tools/kvm/x86/include/kvm/mptable.h
+++ b/tools/kvm/x86/include/kvm/mptable.h
@@ -3,6 +3,7 @@
 
 struct kvm;
 
-void mptable_setup(struct kvm *kvm, unsigned int ncpus);
+int mptable__init(struct kvm *kvm);
+int mptable__uninit(struct kvm *kvm);
 
 #endif /* KVM_MPTABLE_H_ */
diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index d2fbbe2..1ecadad 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -349,15 +349,29 @@ bool load_bzimage(struct kvm *kvm, int fd_kernel,
  * This function is a main routine where we poke guest memory
  * and install BIOS there.
  */
-void kvm__arch_setup_firmware(struct kvm *kvm)
+int kvm__arch_setup_firmware(struct kvm *kvm)
 {
+	int r;
+
 	/* standart minimal configuration */
 	setup_bios(kvm);
 
 	/* FIXME: SMP, ACPI and friends here */
 
 	/* MP table */
-	mptable_setup(kvm, kvm->nrcpus);
+	r = mptable__init(kvm);
+
+	return r;
+}
+
+int kvm__arch_free_firmware(struct kvm *kvm)
+{
+	int r;
+
+	/* MP table */
+	r = mptable__uninit(kvm);
+
+	return r;
 }
 
 void kvm__arch_periodic_poll(struct kvm *kvm)
diff --git a/tools/kvm/x86/mptable.c b/tools/kvm/x86/mptable.c
index 701605a..d499c51 100644
--- a/tools/kvm/x86/mptable.c
+++ b/tools/kvm/x86/mptable.c
@@ -71,7 +71,7 @@ static void mptable_add_irq_src(struct mpc_intsrc *mpc_intsrc,
 /**
  * mptable_setup - create mptable and fill guest memory with it
  */
-void mptable_setup(struct kvm *kvm, unsigned int ncpus)
+int mptable__init(struct kvm *kvm)
 {
 	unsigned long real_mpc_table, real_mpf_intel, size;
 	struct mpf_intel *mpf_intel;
@@ -85,7 +85,7 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	const int pcibusid = 0;
 	const int isabusid = 1;
 
-	unsigned int i, nentries = 0;
+	unsigned int i, nentries = 0, ncpus = kvm->nrcpus;
 	unsigned int ioapicid;
 	void *last_addr;
 
@@ -100,7 +100,7 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 
 	mpc_table = calloc(1, MPTABLE_MAX_SIZE);
 	if (!mpc_table)
-		die("Out of memory");
+		return -ENOMEM;
 
 	MPTABLE_STRNCPY(mpc_table->signature,	MPC_SIGNATURE);
 	MPTABLE_STRNCPY(mpc_table->oem,		MPTABLE_OEM);
@@ -264,8 +264,11 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	 */
 
 	if (size > (unsigned long)(MB_BIOS_END - bios_rom_size) ||
-	    size > MPTABLE_MAX_SIZE)
-		die("MP table is too big");
+	    size > MPTABLE_MAX_SIZE) {
+		free(mpc_table);
+
+		return -E2BIG;
+	}
 
 	/*
 	 * OK, it is time to move it to guest memory.
@@ -273,4 +276,11 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	memcpy(guest_flat_to_host(kvm, real_mpc_table), mpc_table, size);
 
 	free(mpc_table);
+
+	return 0;
 }
+
+int mptable__uninit(struct kvm *kvm)
+{
+	return 0;
+}
\ No newline at end of file
-- 
1.7.8

[RFC 09/12] kvm tools: Fixes for ioeventfd module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c     |   10 ++++++-
 tools/kvm/include/kvm/pci.h |    5 ++-
 tools/kvm/pci.c             |   58 +++++++++++++++++++++++++++++++------------
 3 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 2be3949..2950ea8 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -988,7 +988,11 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 		goto fail;
 	}
 
-	pci__init();
+	r = pci__init(kvm);
+	if (r < 0) {
+		pr_error("pci__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	r = ioport__init(kvm);
 	if (r < 0) {
@@ -1239,6 +1243,10 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("ioeventfd__uninit() failed with error %d\n", r);
 
+	r = pci__uninit(kvm);
+	if (r < 0)
+		pr_warning("pci__uninit() failed with error %d\n", r);
+
 	kvm__delete(kvm);
 
 	if (guest_ret == 0)
diff --git a/tools/kvm/include/kvm/pci.h b/tools/kvm/include/kvm/pci.h
index 07b5403..f5536c7 100644
--- a/tools/kvm/include/kvm/pci.h
+++ b/tools/kvm/include/kvm/pci.h
@@ -84,8 +84,9 @@ struct pci_device_header {
 	u32		bar_size[6];
 } __attribute__((packed));
 
-void pci__init(void);
-void pci__register(struct pci_device_header *dev, u8 dev_num);
+int pci__init(struct kvm *kvm);
+int pci__uninit(struct kvm *kvm);
+int pci__register(struct pci_device_header *dev, u8 dev_num);
 struct pci_device_header *pci__find_dev(u8 dev_num);
 u32 pci_get_io_space_block(u32 size);
 void pci__config_wr(struct kvm *kvm, union pci_config_address addr, void *data, int size);
diff --git a/tools/kvm/pci.c b/tools/kvm/pci.c
index 06eea0f..3696619 100644
--- a/tools/kvm/pci.c
+++ b/tools/kvm/pci.c
@@ -3,6 +3,7 @@
 #include "kvm/util.h"
 #include "kvm/kvm.h"
 
+#include <linux/err.h>
 #include <assert.h>
 
 #define PCI_BAR_OFFSET(b)		(offsetof(struct pci_device_header, bar[b]))
@@ -31,8 +32,8 @@ static void *pci_config_address_ptr(u16 port)
 	unsigned long offset;
 	void *base;
 
-	offset		= port - PCI_CONFIG_ADDRESS;
-	base		= &pci_config_address;
+	offset	= port - PCI_CONFIG_ADDRESS;
+	base	= &pci_config_address;
 
 	return base + offset;
 }
@@ -56,8 +57,8 @@ static bool pci_config_address_in(struct ioport *ioport, struct kvm *kvm, u16 po
 }
 
 static struct ioport_operations pci_config_address_ops = {
-	.io_in		= pci_config_address_in,
-	.io_out		= pci_config_address_out,
+	.io_in	= pci_config_address_in,
+	.io_out	= pci_config_address_out,
 };
 
 static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_number)
@@ -73,7 +74,7 @@ static bool pci_device_exists(u8 bus_number, u8 device_number, u8 function_numbe
 	if (device_number >= PCI_MAX_DEVICES)
 		return false;
 
-	dev		= pci_devices[device_number];
+	dev = pci_devices[device_number];
 
 	return dev != NULL;
 }
@@ -105,15 +106,15 @@ static bool pci_config_data_in(struct ioport *ioport, struct kvm *kvm, u16 port,
 }
 
 static struct ioport_operations pci_config_data_ops = {
-	.io_in		= pci_config_data_in,
-	.io_out		= pci_config_data_out,
+	.io_in	= pci_config_data_in,
+	.io_out	= pci_config_data_out,
 };
 
 void pci__config_wr(struct kvm *kvm, union pci_config_address addr, void *data, int size)
 {
 	u8 dev_num;
 
-	dev_num		= addr.device_number;
+	dev_num	= addr.device_number;
 
 	if (pci_device_exists(0, dev_num, 0)) {
 		unsigned long offset;
@@ -150,7 +151,7 @@ void pci__config_rd(struct kvm *kvm, union pci_config_address addr, void *data,
 {
 	u8 dev_num;
 
-	dev_num		= addr.device_number;
+	dev_num	= addr.device_number;
 
 	if (pci_device_exists(0, dev_num, 0)) {
 		unsigned long offset;
@@ -168,20 +169,45 @@ void pci__config_rd(struct kvm *kvm, union pci_config_address addr, void *data,
 	}
 }
 
-void pci__register(struct pci_device_header *dev, u8 dev_num)
+int pci__register(struct pci_device_header *dev, u8 dev_num)
 {
-	assert(dev_num < PCI_MAX_DEVICES);
-	pci_devices[dev_num]	= dev;
+	if (dev_num >= PCI_MAX_DEVICES)
+		return -ENOSPC;
+
+	pci_devices[dev_num] = dev;
+
+	return 0;
 }
 
 struct pci_device_header *pci__find_dev(u8 dev_num)
 {
-	assert(dev_num < PCI_MAX_DEVICES);
+	if (dev_num >= PCI_MAX_DEVICES)
+		return ERR_PTR(-EOVERFLOW);
+
 	return pci_devices[dev_num];
 }
 
-void pci__init(void)
+int pci__init(struct kvm *kvm)
 {
-	ioport__register(PCI_CONFIG_DATA + 0, &pci_config_data_ops, 4, NULL);
-	ioport__register(PCI_CONFIG_ADDRESS + 0, &pci_config_address_ops, 4, NULL);
+	int r;
+
+	r = ioport__register(PCI_CONFIG_DATA + 0, &pci_config_data_ops, 4, NULL);
+	if (r < 0)
+		return r;
+
+	r = ioport__register(PCI_CONFIG_ADDRESS + 0, &pci_config_address_ops, 4, NULL);
+	if (r < 0) {
+		ioport__unregister(PCI_CONFIG_DATA);
+		return r;
+	}
+
+	return 0;
+}
+
+int pci__uninit(struct kvm *kvm)
+{
+	ioport__unregister(PCI_CONFIG_DATA);
+	ioport__unregister(PCI_CONFIG_ADDRESS);
+
+	return 0;
 }
-- 
1.7.8

[RFC 10/12] kvm tools: Fixes for disk image module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c            |   27 +++++++++---
 tools/kvm/disk/blk.c               |   13 ++++--
 tools/kvm/disk/core.c              |   76 ++++++++++++++++++++++--------------
 tools/kvm/disk/qcow.c              |    2 +-
 tools/kvm/disk/raw.c               |    9 ++--
 tools/kvm/include/kvm/disk-image.h |    2 +-
 tools/kvm/include/kvm/virtio-blk.h |    5 +-
 tools/kvm/virtio/blk.c             |   37 +++++++++++++-----
 8 files changed, 111 insertions(+), 60 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 2950ea8..0b94ef0 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1052,11 +1052,13 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	if (image_count) {
 		kvm->nr_disks = image_count;
-		kvm->disks    = disk_image__open_all(image_filename, readonly_image, image_count);
-		if (!kvm->disks)
-			die("Unable to load all disk images.");
-
-		virtio_blk__init_all(kvm);
+		kvm->disks = disk_image__open_all(image_filename, readonly_image, image_count);
+		if (IS_ERR(kvm->disks)) {
+			r = PTR_ERR(kvm->disks);
+			pr_error("disk_image__open_all() failed with error %ld\n",
+					PTR_ERR(kvm->disks));
+			goto fail;
+		}
 	}
 
 	printf("  # kvm run -k %s -m %Lu -c %d --name %s\n", kernel_filename, ram_size / 1024 / 1024, nrcpus, guest_name);
@@ -1082,6 +1084,12 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 		goto fail;
 	}
 
+	r = virtio_blk__init(kvm);
+	if (r < 0) {
+		pr_error("virtio_blk__init() failed with error %d\n", r);
+		goto fail;
+	}
+
 	if (active_console == CONSOLE_VIRTIO)
 		virtio_console__init(kvm);
 
@@ -1222,10 +1230,15 @@ static void kvm_cmd_run_uninit(int guest_ret)
 
 	fb__stop();
 
-	virtio_blk__delete_all(kvm);
+	r = virtio_blk__uninit(kvm);
+	if (r < 0)
+		pr_warning("virtio_blk__uninit() failed with error %d\n", r);
+
 	virtio_rng__delete_all(kvm);
 
-	disk_image__close_all(kvm->disks, image_count);
+	r = disk_image__close_all(kvm->disks, image_count);
+	if (r < 0)
+		pr_warning("disk_image__close_all() failed with error %d\n", r);
 
 	r = serial8250__uninit(kvm);
 	if (r < 0)
diff --git a/tools/kvm/disk/blk.c b/tools/kvm/disk/blk.c
index 59294e8..72c1722 100644
--- a/tools/kvm/disk/blk.c
+++ b/tools/kvm/disk/blk.c
@@ -1,5 +1,7 @@
 #include "kvm/disk-image.h"
 
+#include <linux/err.h>
+
 /*
  * raw image and blk dev are similar, so reuse raw image ops.
  */
@@ -12,22 +14,23 @@ static struct disk_image_operations blk_dev_ops = {
 struct disk_image *blkdev__probe(const char *filename, struct stat *st)
 {
 	u64 size;
-	int fd;
+	int fd, r;
 
 	if (!S_ISBLK(st->st_mode))
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	/*
 	 * Be careful! We are opening host block device!
 	 * Open it readonly since we do not want to break user's data on disk.
 	 */
-	fd			= open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY);
 	if (fd < 0)
-		return NULL;
+		return ERR_PTR(fd);
 
 	if (ioctl(fd, BLKGETSIZE64, &size) < 0) {
+		r = -errno;
 		close(fd);
-		return NULL;
+		return ERR_PTR(r);
 	}
 
 	/*
diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 4915efd..fb547ba 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -2,6 +2,7 @@
 #include "kvm/qcow.h"
 #include "kvm/virtio-blk.h"
 
+#include <linux/err.h>
 #include <sys/eventfd.h>
 #include <sys/poll.h>
 
@@ -31,14 +32,17 @@ static void *disk_image__thread(void *param)
 struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops, int use_mmap)
 {
 	struct disk_image *disk;
+	int r;
 
-	disk		= malloc(sizeof *disk);
+	disk = malloc(sizeof *disk);
 	if (!disk)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
-	disk->fd	= fd;
-	disk->size	= size;
-	disk->ops	= ops;
+	*disk = (struct disk_image) {
+		.fd	= fd,
+		.size	= size,
+		.ops	= ops,
+	};
 
 	if (use_mmap == DISK_IMAGE_MMAP) {
 		/*
@@ -46,8 +50,9 @@ struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operation
 		 */
 		disk->priv = mmap(NULL, size, PROT_RW, MAP_PRIVATE | MAP_NORESERVE, fd, 0);
 		if (disk->priv == MAP_FAILED) {
+			r = -errno;
 			free(disk);
-			disk = NULL;
+			return ERR_PTR(r);
 		}
 	}
 
@@ -57,8 +62,12 @@ struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operation
 
 		disk->evt = eventfd(0, 0);
 		io_setup(AIO_MAX, &disk->ctx);
-		if (pthread_create(&thread, NULL, disk_image__thread, disk) != 0)
-			die("Failed starting IO thread");
+		r = pthread_create(&thread, NULL, disk_image__thread, disk);
+		if (r) {
+			r = -errno;
+			free(disk);
+			return ERR_PTR(r);
+		}
 	}
 #endif
 	return disk;
@@ -71,54 +80,58 @@ struct disk_image *disk_image__open(const char *filename, bool readonly)
 	int fd;
 
 	if (stat(filename, &st) < 0)
-		return NULL;
+		return ERR_PTR(-errno);
 
 	/* blk device ?*/
-	disk		= blkdev__probe(filename, &st);
-	if (disk)
+	disk = blkdev__probe(filename, &st);
+	if (!IS_ERR_OR_NULL(disk))
 		return disk;
 
-	fd		= open(filename, readonly ? O_RDONLY : O_RDWR);
+	fd = open(filename, readonly ? O_RDONLY : O_RDWR);
 	if (fd < 0)
-		return NULL;
+		return ERR_PTR(fd);
 
 	/* qcow image ?*/
-	disk		= qcow_probe(fd, true);
-	if (disk) {
+	disk = qcow_probe(fd, true);
+	if (!IS_ERR_OR_NULL(disk)) {
 		pr_warning("Forcing read-only support for QCOW");
 		return disk;
 	}
 
 	/* raw image ?*/
-	disk		= raw_image__probe(fd, &st, readonly);
-	if (disk)
+	disk = raw_image__probe(fd, &st, readonly);
+	if (!IS_ERR_OR_NULL(disk))
 		return disk;
 
 	if (close(fd) < 0)
 		pr_warning("close() failed");
 
-	return NULL;
+	return ERR_PTR(-ENOSYS);
 }
 
 struct disk_image **disk_image__open_all(const char **filenames, bool *readonly, int count)
 {
 	struct disk_image **disks;
 	int i;
+	void *err;
 
-	if (!count || count > MAX_DISK_IMAGES)
-		return NULL;
+	if (!count)
+		return ERR_PTR(-EINVAL);
+	if (count > MAX_DISK_IMAGES)
+		return ERR_PTR(-ENOSPC);
 
 	disks = calloc(count, sizeof(*disks));
 	if (!disks)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	for (i = 0; i < count; i++) {
 		if (!filenames[i])
 			continue;
 
 		disks[i] = disk_image__open(filenames[i], readonly[i]);
-		if (!disks[i]) {
+		if (IS_ERR_OR_NULL(disks[i])) {
 			pr_error("Loading disk image '%s' failed", filenames[i]);
+			err = disks[i];
 			goto error;
 		}
 	}
@@ -126,10 +139,11 @@ struct disk_image **disk_image__open_all(const char **filenames, bool *readonly,
 	return disks;
 error:
 	for (i = 0; i < count; i++)
-		disk_image__close(disks[i]);
+		if (!IS_ERR_OR_NULL(disks[i]))
+			disk_image__close(disks[i]);
 
 	free(disks);
-	return NULL;
+	return err;
 }
 
 int disk_image__flush(struct disk_image *disk)
@@ -157,12 +171,14 @@ int disk_image__close(struct disk_image *disk)
 	return 0;
 }
 
-void disk_image__close_all(struct disk_image **disks, int count)
+int disk_image__close_all(struct disk_image **disks, int count)
 {
 	while (count)
 		disk_image__close(disks[--count]);
 
 	free(disks);
+
+	return 0;
 }
 
 /*
@@ -181,7 +197,7 @@ ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec
 		total = disk->ops->read_sector(disk, sector, iov, iovcount, param);
 		if (total < 0) {
 			pr_info("disk_image__read error: total=%ld\n", (long)total);
-			return -1;
+			return total;
 		}
 	} else {
 		/* Do nothing */
@@ -213,7 +229,7 @@ ssize_t disk_image__write(struct disk_image *disk, u64 sector, const struct iove
 		total = disk->ops->write_sector(disk, sector, iov, iovcount, param);
 		if (total < 0) {
 			pr_info("disk_image__write error: total=%ld\n", (long)total);
-			return -1;
+			return total;
 		}
 	} else {
 		/* Do nothing */
@@ -228,9 +244,11 @@ ssize_t disk_image__write(struct disk_image *disk, u64 sector, const struct iove
 ssize_t disk_image__get_serial(struct disk_image *disk, void *buffer, ssize_t *len)
 {
 	struct stat st;
+	int r;
 
-	if (fstat(disk->fd, &st) != 0)
-		return 0;
+	r = fstat(disk->fd, &st);
+	if (r)
+		return r;
 
 	*len = snprintf(buffer, *len, "%llu%llu%llu", (u64)st.st_dev, (u64)st.st_rdev, (u64)st.st_ino);
 	return *len;
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index 712f811..23f11f2 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -1334,7 +1334,7 @@ static struct disk_image *qcow2_probe(int fd, bool readonly)
 	else
 		disk_image = disk_image__new(fd, h->size, &qcow_disk_ops, DISK_IMAGE_REGULAR);
 
-	if (!disk_image)
+	if (IS_ERR_OR_NULL(disk_image))
 		goto free_refcount_table;
 
 	disk_image->async = 0;
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
index caa023c..d2df814 100644
--- a/tools/kvm/disk/raw.c
+++ b/tools/kvm/disk/raw.c
@@ -1,5 +1,7 @@
 #include "kvm/disk-image.h"
 
+#include <linux/err.h>
+
 #ifdef CONFIG_HAS_AIO
 #include <libaio.h>
 #endif
@@ -116,11 +118,10 @@ struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
 		struct disk_image *disk;
 
 		disk = disk_image__new(fd, st->st_size, &ro_ops, DISK_IMAGE_MMAP);
-		if (disk == NULL) {
-
+		if (IS_ERR_OR_NULL(disk)) {
 			disk = disk_image__new(fd, st->st_size, &ro_ops_nowrite, DISK_IMAGE_REGULAR);
 #ifdef CONFIG_HAS_AIO
-			if (disk)
+			if (!IS_ERR_OR_NULL(disk))
 				disk->async = 1;
 #endif
 		}
@@ -132,7 +133,7 @@ struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
 		 */
 		disk = disk_image__new(fd, st->st_size, &raw_image_regular_ops, DISK_IMAGE_REGULAR);
 #ifdef CONFIG_HAS_AIO
-		if (disk)
+		if (!IS_ERR_OR_NULL(disk))
 			disk->async = 1;
 #endif
 		return disk;
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index 56c08da..a0b61bf 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -57,7 +57,7 @@ struct disk_image *disk_image__open(const char *filename, bool readonly);
 struct disk_image **disk_image__open_all(const char **filenames, bool *readonly, int count);
 struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops, int mmap);
 int disk_image__close(struct disk_image *disk);
-void disk_image__close_all(struct disk_image **disks, int count);
+int disk_image__close_all(struct disk_image **disks, int count);
 int disk_image__flush(struct disk_image *disk);
 ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec *iov,
 				int iovcount, void *param);
diff --git a/tools/kvm/include/kvm/virtio-blk.h b/tools/kvm/include/kvm/virtio-blk.h
index 63731a9..e0c0919 100644
--- a/tools/kvm/include/kvm/virtio-blk.h
+++ b/tools/kvm/include/kvm/virtio-blk.h
@@ -5,9 +5,8 @@
 
 struct kvm;
 
-void virtio_blk__init(struct kvm *kvm, struct disk_image *disk);
-void virtio_blk__init_all(struct kvm *kvm);
-void virtio_blk__delete_all(struct kvm *kvm);
+int virtio_blk__init(struct kvm *kvm);
+int virtio_blk__uninit(struct kvm *kvm);
 void virtio_blk_complete(void *param, long len);
 
 #endif /* KVM__BLK_VIRTIO_H */
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index d1a0197..e9b836a 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -208,17 +208,17 @@ static struct virtio_ops blk_dev_virtio_ops = (struct virtio_ops) {
 	.get_size_vq		= get_size_vq,
 };
 
-void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
+static int virtio_blk__init_one(struct kvm *kvm, struct disk_image *disk)
 {
 	struct blk_dev *bdev;
 	unsigned int i;
 
 	if (!disk)
-		return;
+		return -EINVAL;
 
 	bdev = calloc(1, sizeof(struct blk_dev));
 	if (bdev == NULL)
-		die("Failed allocating bdev");
+		return -ENOMEM;
 
 	*bdev = (struct blk_dev) {
 		.mutex			= PTHREAD_MUTEX_INITIALIZER,
@@ -251,23 +251,40 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 						"Please make sure that the guest kernel was "
 						"compiled with CONFIG_VIRTIO_BLK=y enabled "
 						"in its .config");
+	return 0;
 }
 
-void virtio_blk__init_all(struct kvm *kvm)
+static int virtio_blk__uninit_one(struct kvm *kvm, struct blk_dev *bdev)
 {
-	int i;
+	list_del(&bdev->list);
+	free(bdev);
 
-	for (i = 0; i < kvm->nr_disks; i++)
-		virtio_blk__init(kvm, kvm->disks[i]);
+	return 0;
 }
 
-void virtio_blk__delete_all(struct kvm *kvm)
+int virtio_blk__init(struct kvm *kvm)
+{
+	int i, r = 0;
+
+	for (i = 0; i < kvm->nr_disks; i++) {
+		r = virtio_blk__init_one(kvm, kvm->disks[i]);
+		if (r < 0)
+			goto cleanup;
+	}
+
+	return 0;
+cleanup:
+	return virtio_blk__uninit(kvm);
+}
+
+int virtio_blk__uninit(struct kvm *kvm)
 {
 	while (!list_empty(&bdevs)) {
 		struct blk_dev *bdev;
 
 		bdev = list_first_entry(&bdevs, struct blk_dev, list);
-		list_del(&bdev->list);
-		free(bdev);
+		virtio_blk__uninit_one(kvm, bdev);
 	}
+
+	return 0;
 }
-- 
1.7.8

[RFC 11/12] kvm tools: Fixes for rtc module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c     |   10 +++++++++-
 tools/kvm/hw/rtc.c          |   27 +++++++++++++++++++++++----
 tools/kvm/include/kvm/rtc.h |    5 ++++-
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 0b94ef0..5b198e0 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1076,7 +1076,11 @@ static int kvm_cmd_run_init(int argc, const char **argv)
 
 	ioport__setup_arch();
 
-	rtc__init();
+	r = rtc__init(kvm);
+	if (r < 0) {
+		pr_error("rtc__init() failed with error %d\n", r);
+		goto fail;
+	}
 
 	r = serial8250__init(kvm);
 	if (r < 0) {
@@ -1244,6 +1248,10 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("serial8250__uninit() failed with error %d\n", r);
 
+	r = rtc__uninit(kvm);
+	if (r < 0)
+		pr_warning("rtc__uninit() failed with error %d\n", r);
+
 	r = kvm__arch_free_firmware(kvm);
 	if (r < 0)
 		pr_warning("kvm__arch_free_firmware() failed with error %d\n", r);
diff --git a/tools/kvm/hw/rtc.c b/tools/kvm/hw/rtc.c
index 6d4a05a..affc5ee 100644
--- a/tools/kvm/hw/rtc.c
+++ b/tools/kvm/hw/rtc.c
@@ -100,7 +100,6 @@ static bool cmos_ram_index_out(struct ioport *ioport, struct kvm *kvm, u16 port,
 	u8 value = ioport__read8(data);
 
 	kvm->nmi_disabled	= value & (1UL << 7);
-
 	rtc.cmos_idx		= value & ~(1UL << 7);
 
 	return true;
@@ -110,9 +109,29 @@ static struct ioport_operations cmos_ram_index_ioport_ops = {
 	.io_out		= cmos_ram_index_out,
 };
 
-void rtc__init(void)
+int rtc__init(struct kvm *kvm)
 {
+	int r = 0;
+
 	/* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
-	ioport__register(0x0070, &cmos_ram_index_ioport_ops, 1, NULL);
-	ioport__register(0x0071, &cmos_ram_data_ioport_ops, 1, NULL);
+	r = ioport__register(0x0070, &cmos_ram_index_ioport_ops, 1, NULL);
+	if (r < 0)
+		return r;
+
+	r = ioport__register(0x0071, &cmos_ram_data_ioport_ops, 1, NULL);
+	if (r < 0) {
+		ioport__unregister(0x0071);
+		return r;
+	}
+
+	return r;
 }
+
+int rtc__uninit(struct kvm *kvm)
+{
+	/* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */
+	ioport__unregister(0x0070);
+	ioport__unregister(0x0071);
+
+	return 0;
+}
\ No newline at end of file
diff --git a/tools/kvm/include/kvm/rtc.h b/tools/kvm/include/kvm/rtc.h
index 0b8d9f9..a13db59a 100644
--- a/tools/kvm/include/kvm/rtc.h
+++ b/tools/kvm/include/kvm/rtc.h
@@ -1,6 +1,9 @@
 #ifndef KVM__RTC_H
 #define KVM__RTC_H
 
-void rtc__init(void);
+struct kvm;
+
+int rtc__init(struct kvm *kvm);
+int rtc__uninit(struct kvm *kvm);
 
 #endif /* KVM__RTC_H */
-- 
1.7.8

[RFC 12/12] kvm tools: Fixes for ioeventfd module

[Sasha Levin]

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/builtin-run.c              |    8 ++-
 tools/kvm/include/kvm/kvm.h          |    6 +-
 tools/kvm/include/kvm/virtio-pci.h   |    1 +
 tools/kvm/include/kvm/virtio-rng.h   |    4 +-
 tools/kvm/include/kvm/virtio-trans.h |    1 +
 tools/kvm/ioport.c                   |    9 ++-
 tools/kvm/kvm.c                      |  101 +++++++++++++++++++++++-----------
 tools/kvm/mmio.c                     |    8 ++-
 tools/kvm/util/rbtree-interval.c     |    9 ++-
 tools/kvm/virtio/pci.c               |   34 ++++++++++-
 tools/kvm/virtio/rng.c               |   27 +++++++--
 tools/kvm/virtio/trans.c             |    4 +-
 tools/kvm/x86/irq.c                  |   17 +++++-
 13 files changed, 168 insertions(+), 61 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 5b198e0..defd1b9 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1238,7 +1238,9 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("virtio_blk__uninit() failed with error %d\n", r);
 
-	virtio_rng__delete_all(kvm);
+	r = virtio_rng__uninit(kvm);
+	if (r < 0)
+		pr_warning("virtio_rng__uninit() failed with error %d\n", r);
 
 	r = disk_image__close_all(kvm->disks, image_count);
 	if (r < 0)
@@ -1268,7 +1270,9 @@ static void kvm_cmd_run_uninit(int guest_ret)
 	if (r < 0)
 		pr_warning("pci__uninit() failed with error %d\n", r);
 
-	kvm__delete(kvm);
+	r = kvm__uninit(kvm);
+	if (r < 0)
+		pr_warning("pci__uninit() failed with error %d\n", r);
 
 	if (guest_ret == 0)
 		printf("\n  # KVM session ended normally.\n");
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 74f46c1..e7603a9 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -32,7 +32,7 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s
 int kvm__recommended_cpus(struct kvm *kvm);
 int kvm__max_cpus(struct kvm *kvm);
 void kvm__init_ram(struct kvm *kvm);
-void kvm__delete(struct kvm *kvm);
+int kvm__uninit(struct kvm *kvm);
 bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
 			const char *initrd_filename, const char *kernel_cmdline, u16 vidmode);
 void kvm__start_timer(struct kvm *kvm);
@@ -41,8 +41,8 @@ void kvm__irq_line(struct kvm *kvm, int irq, int level);
 void kvm__irq_trigger(struct kvm *kvm, int irq);
 bool kvm__emulate_io(struct kvm *kvm, u16 port, void *data, int direction, int size, u32 count);
 bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_write);
-void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr);
-bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce,
+int kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr);
+int kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce,
 			void (*mmio_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr),
 			void *ptr);
 bool kvm__deregister_mmio(struct kvm *kvm, u64 phys_addr);
diff --git a/tools/kvm/include/kvm/virtio-pci.h b/tools/kvm/include/kvm/virtio-pci.h
index 73f7486..a2d43e6 100644
--- a/tools/kvm/include/kvm/virtio-pci.h
+++ b/tools/kvm/include/kvm/virtio-pci.h
@@ -40,6 +40,7 @@ struct virtio_pci {
 
 int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev,
 			int device_id, int subsys_id, int class);
+int virtio_pci__uninit(struct kvm *kvm, struct virtio_trans *vtrans);
 int virtio_pci__signal_vq(struct kvm *kvm, struct virtio_trans *vtrans, u32 vq);
 int virtio_pci__signal_config(struct kvm *kvm, struct virtio_trans *vtrans);
 
diff --git a/tools/kvm/include/kvm/virtio-rng.h b/tools/kvm/include/kvm/virtio-rng.h
index c0a413b..983dfbf 100644
--- a/tools/kvm/include/kvm/virtio-rng.h
+++ b/tools/kvm/include/kvm/virtio-rng.h
@@ -3,7 +3,7 @@
 
 struct kvm;
 
-void virtio_rng__init(struct kvm *kvm);
-void virtio_rng__delete_all(struct kvm *kvm);
+int virtio_rng__init(struct kvm *kvm);
+int virtio_rng__uninit(struct kvm *kvm);
 
 #endif /* KVM__RNG_VIRTIO_H */
diff --git a/tools/kvm/include/kvm/virtio-trans.h b/tools/kvm/include/kvm/virtio-trans.h
index e7c186e..f2e433f 100644
--- a/tools/kvm/include/kvm/virtio-trans.h
+++ b/tools/kvm/include/kvm/virtio-trans.h
@@ -27,6 +27,7 @@ struct virtio_ops {
 struct virtio_trans_ops {
 	int (*init)(struct kvm *kvm, struct virtio_trans *vtrans, void *dev, int device_id,
 			int subsys_id, int class);
+	int (*uninit)(struct kvm *kvm, struct virtio_trans *vtrans);
 	int (*signal_vq)(struct kvm *kvm, struct virtio_trans *virtio_trans, u32 queueid);
 	int (*signal_config)(struct kvm *kvm, struct virtio_trans *virtio_trans);
 };
diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c
index 19caf6c..5237ba5 100644
--- a/tools/kvm/ioport.c
+++ b/tools/kvm/ioport.c
@@ -60,6 +60,7 @@ static void ioport_remove(struct rb_root *root, struct ioport *data)
 int ioport__register(u16 port, struct ioport_operations *ops, int count, void *param)
 {
 	struct ioport *entry;
+	int r;
 
 	br_write_lock();
 	if (port == IOPORT_EMPTY)
@@ -81,8 +82,12 @@ int ioport__register(u16 port, struct ioport_operations *ops, int count, void *p
 		.priv	= param,
 	};
 
-	ioport_insert(&ioport_tree, entry);
-
+	r = ioport_insert(&ioport_tree, entry);
+	if (r < 0) {
+		free(entry);
+		br_write_unlock();
+		return r;
+	}
 	br_write_unlock();
 
 	return port;
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 8c9e268..32d05b1 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -6,6 +6,7 @@
 #include "kvm/kvm-ipc.h"
 
 #include <linux/kvm.h>
+#include <linux/err.h>
 
 #include <sys/un.h>
 #include <sys/stat.h>
@@ -62,7 +63,7 @@ extern struct kvm_ext kvm_req_ext[];
 
 static char kvm_dir[PATH_MAX];
 
-static void set_dir(const char *fmt, va_list args)
+static int set_dir(const char *fmt, va_list args)
 {
 	char tmp[PATH_MAX];
 
@@ -71,9 +72,11 @@ static void set_dir(const char *fmt, va_list args)
 	mkdir(tmp, 0777);
 
 	if (!realpath(tmp, kvm_dir))
-		die("Unable to set KVM tool directory");
+		return -errno;
 
 	strcat(kvm_dir, "/");
+
+	return 0;
 }
 
 void kvm__set_dir(const char *fmt, ...)
@@ -103,7 +106,7 @@ bool kvm__supports_extension(struct kvm *kvm, unsigned int extension)
 
 static int kvm__check_extensions(struct kvm *kvm)
 {
-	unsigned int i;
+	int i;
 
 	for (i = 0; ; i++) {
 		if (!kvm_req_ext[i].name)
@@ -111,7 +114,7 @@ static int kvm__check_extensions(struct kvm *kvm)
 		if (!kvm__supports_extension(kvm, kvm_req_ext[i].code)) {
 			pr_error("Unsuppored KVM extension detected: %s",
 				kvm_req_ext[i].name);
-			return (int)-i;
+			return -i;
 		}
 	}
 
@@ -120,10 +123,10 @@ static int kvm__check_extensions(struct kvm *kvm)
 
 static struct kvm *kvm__new(void)
 {
-	struct kvm *kvm = calloc(1, sizeof *kvm);
+	struct kvm *kvm = calloc(1, sizeof(*kvm));
 
 	if (!kvm)
-		die("out of memory");
+		return ERR_PTR(-ENOMEM);
 
 	return kvm;
 }
@@ -136,11 +139,13 @@ static int kvm__create_socket(struct kvm *kvm)
 	int len, r;
 
 	if (!kvm->name)
-		return -1;
+		return -EINVAL;
 
 	sprintf(full_name, "%s/%s.sock", kvm__get_dir(), kvm->name);
-	if (access(full_name, F_OK) == 0)
-		die("Socket file %s already exist", full_name);
+	if (access(full_name, F_OK) == 0) {
+		pr_error("Socket file %s already exist", full_name);
+		return -EEXIST;
+	}
 
 	s = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (s < 0)
@@ -161,7 +166,7 @@ static int kvm__create_socket(struct kvm *kvm)
 
 fail:
 	close(s);
-	return -1;
+	return r;
 }
 
 void kvm__remove_socket(const char *name)
@@ -189,9 +194,9 @@ int kvm__get_sock_by_instance(const char *name)
 	if (r < 0 && errno == ECONNREFUSED) {
 		/* Clean ghost socket file */
 		unlink(sock_file);
-		return -1;
+		return r;
 	} else if (r < 0) {
-		die("Failed connecting to instance");
+		return r;
 	}
 
 	return s;
@@ -206,7 +211,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd))
 
 	dir = opendir(kvm__get_dir());
 	if (!dir)
-		return -1;
+		return -errno;
 
 	for (;;) {
 		readdir_r(dir, &entry, &result);
@@ -229,7 +234,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd))
 	return ret;
 }
 
-void kvm__delete(struct kvm *kvm)
+int kvm__uninit(struct kvm *kvm)
 {
 	kvm__stop_timer(kvm);
 
@@ -237,6 +242,8 @@ void kvm__delete(struct kvm *kvm)
 	kvm_ipc__stop();
 	kvm__remove_socket(kvm->name);
 	free(kvm);
+
+	return 0;
 }
 
 /*
@@ -244,7 +251,7 @@ void kvm__delete(struct kvm *kvm)
  * memory regions to it. Therefore, be careful if you use this function for
  * registering memory regions for emulating hardware.
  */
-void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr)
+int kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr)
 {
 	struct kvm_userspace_memory_region mem;
 	int ret;
@@ -258,7 +265,9 @@ void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspac
 
 	ret = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &mem);
 	if (ret < 0)
-		die_perror("KVM_SET_USER_MEMORY_REGION ioctl");
+		return -errno;
+
+	return 0;
 }
 
 int kvm__recommended_cpus(struct kvm *kvm)
@@ -312,33 +321,53 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s
 	struct kvm *kvm;
 	int ret;
 
-	if (!kvm__arch_cpu_supports_vm())
-		die("Your CPU does not support hardware virtualization");
+	if (!kvm__arch_cpu_supports_vm()) {
+		pr_error("Your CPU does not support hardware virtualization");
+		return ERR_PTR(-ENOSYS);		
+	}
 
 	kvm = kvm__new();
+	if (IS_ERR_OR_NULL(kvm))
+		return kvm;
 
 	kvm->sys_fd = open(kvm_dev, O_RDWR);
 	if (kvm->sys_fd < 0) {
-		if (errno == ENOENT)
-			die("'%s' not found. Please make sure your kernel has CONFIG_KVM enabled and that the KVM modules are loaded.", kvm_dev);
-		if (errno == ENODEV)
-			die("'%s' KVM driver not available.\n  # (If the KVM module is loaded then 'dmesg' may offer further clues about the failure.)", kvm_dev);
-
-		fprintf(stderr, "  Fatal, could not open %s: ", kvm_dev);
-		perror(NULL);
-		exit(1);
+		if (errno == ENOENT) {
+			pr_error("'%s' not found. Please make sure your kernel has CONFIG_KVM "
+				"enabled and that the KVM modules are loaded.", kvm_dev);
+			ret = -errno;
+			goto cleanup;
+		}
+		if (errno == ENODEV) {
+			die("'%s' KVM driver not available.\n  # (If the KVM "
+				"module is loaded then 'dmesg' may offer further clues "
+				"about the failure.)", kvm_dev);
+			ret = -errno;
+			goto cleanup;
+		}
+
+		pr_error("Could not open %s: ", kvm_dev);
+		ret = -errno;
+		goto cleanup;
 	}
 
 	ret = ioctl(kvm->sys_fd, KVM_GET_API_VERSION, 0);
-	if (ret != KVM_API_VERSION)
-		die_perror("KVM_API_VERSION ioctl");
+	if (ret != KVM_API_VERSION) {
+		pr_error("KVM_API_VERSION ioctl");
+		ret = -errno;
+		goto cleanup;
+	}
 
 	kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
-	if (kvm->vm_fd < 0)
-		die_perror("KVM_CREATE_VM ioctl");
-
-	if (kvm__check_extensions(kvm))
-		die("A required KVM extention is not supported by OS");
+	if (kvm->vm_fd < 0) {
+		ret = kvm->vm_fd;
+		goto cleanup;
+	}		
+
+	if (kvm__check_extensions(kvm)) {
+		pr_error("A required KVM extention is not supported by OS");
+		ret = -ENOSYS;
+	}
 
 	kvm__arch_init(kvm, kvm_dev, hugetlbfs_path, ram_size, name);
 
@@ -347,6 +376,12 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s
 	kvm_ipc__start(kvm__create_socket(kvm));
 	kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
 	return kvm;
+cleanup:
+	close(kvm->vm_fd);
+	close(kvm->sys_fd);
+	free(kvm);
+
+	return ERR_PTR(ret);
 }
 
 /* RFC 1952 */
diff --git a/tools/kvm/mmio.c b/tools/kvm/mmio.c
index 6e8d1bb..7760a7f 100644
--- a/tools/kvm/mmio.c
+++ b/tools/kvm/mmio.c
@@ -9,6 +9,8 @@
 #include <linux/kvm.h>
 #include <linux/types.h>
 #include <linux/rbtree.h>
+#include <linux/err.h>
+#include <errno.h>
 
 #define mmio_node(n) rb_entry(n, struct mmio_mapping, node)
 
@@ -56,7 +58,7 @@ static const char *to_direction(u8 is_write)
 	return "read";
 }
 
-bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce,
+int kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce,
 			void (*mmio_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr),
 			void *ptr)
 {
@@ -66,7 +68,7 @@ bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool
 
 	mmio = malloc(sizeof(*mmio));
 	if (mmio == NULL)
-		return false;
+		return -ENOMEM;
 
 	*mmio = (struct mmio_mapping) {
 		.node = RB_INT_INIT(phys_addr, phys_addr + phys_addr_len),
@@ -82,7 +84,7 @@ bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool
 		ret = ioctl(kvm->vm_fd, KVM_REGISTER_COALESCED_MMIO, &zone);
 		if (ret < 0) {
 			free(mmio);
-			return false;
+			return -errno;
 		}
 	}
 	br_write_lock();
diff --git a/tools/kvm/util/rbtree-interval.c b/tools/kvm/util/rbtree-interval.c
index edc140d..f9bf4b8 100644
--- a/tools/kvm/util/rbtree-interval.c
+++ b/tools/kvm/util/rbtree-interval.c
@@ -1,5 +1,6 @@
 #include <kvm/rbtree-interval.h>
 #include <stddef.h>
+#include <errno.h>
 
 struct rb_int_node *rb_int_search_single(struct rb_root *root, u64 point)
 {
@@ -56,10 +57,10 @@ static void update_node_max_high(struct rb_node *node, void *arg)
 
 int rb_int_insert(struct rb_root *root, struct rb_int_node *i_node)
 {
-	struct rb_node **node	= &(root->rb_node), *parent = NULL;
+	struct rb_node **node = &(root->rb_node), *parent = NULL;
 
 	while (*node) {
-		int result	= i_node->low - rb_int(*node)->low;
+		int result = i_node->low - rb_int(*node)->low;
 
 		parent = *node;
 		if (result < 0)
@@ -67,14 +68,14 @@ int rb_int_insert(struct rb_root *root, struct rb_int_node *i_node)
 		else if (result > 0)
 			node	= &((*node)->rb_right);
 		else
-			return 0;
+			return -EEXIST;
 	}
 
 	rb_link_node(&i_node->node, parent, node);
 	rb_insert_color(&i_node->node, root);
 
 	rb_augment_insert(&i_node->node, update_node_max_high, NULL);
-	return 1;
+	return 0;
 }
 
 void rb_int_erase(struct rb_root *root, struct rb_int_node *node)
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index 7cc014e..fb46d5c 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -18,6 +18,7 @@ struct virtio_trans_ops *virtio_pci__get_trans_ops(void)
 		.signal_vq	= virtio_pci__signal_vq,
 		.signal_config	= virtio_pci__signal_config,
 		.init		= virtio_pci__init,
+		.uninit		= virtio_pci__uninit,
 	};
 	return &virtio_pci_trans;
 };
@@ -297,7 +298,9 @@ int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev,
 		return r;
 
 	vpci->base_addr = (u16)r;
-	kvm__register_mmio(kvm, vpci->msix_io_block, PCI_IO_SIZE, false, callback_mmio_table, vpci);
+	r = kvm__register_mmio(kvm, vpci->msix_io_block, PCI_IO_SIZE, false, callback_mmio_table, vpci);
+	if (r < 0)
+		goto free_ioport;
 
 	vpci->pci_hdr = (struct pci_device_header) {
 		.vendor_id		= cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET),
@@ -343,12 +346,35 @@ int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev,
 	vpci->pci_hdr.msix.pba_offset = cpu_to_le32(1 | PCI_IO_SIZE); /* Use BAR 3 */
 	vpci->config_vector = 0;
 
-	if (irq__register_device(subsys_id, &ndev, &pin, &line) < 0)
-		return -1;
+	r = irq__register_device(subsys_id, &ndev, &pin, &line);
+	if (r < 0)
+		goto free_mmio;
 
 	vpci->pci_hdr.irq_pin	= pin;
 	vpci->pci_hdr.irq_line	= line;
-	pci__register(&vpci->pci_hdr, ndev);
+	r = pci__register(&vpci->pci_hdr, ndev);
+	if (r < 0)
+		goto free_ioport;
 
 	return 0;
+
+free_mmio:
+	kvm__deregister_mmio(kvm, vpci->msix_io_block);
+free_ioport:
+	ioport__unregister(vpci->base_addr);
+	return r;
 }
+
+int virtio_pci__uninit(struct kvm *kvm, struct virtio_trans *vtrans)
+{
+	struct virtio_pci *vpci = vtrans->virtio;
+	int i;
+
+	kvm__deregister_mmio(kvm, vpci->msix_io_block);
+	ioport__unregister(vpci->base_addr);
+
+	for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++)
+		ioeventfd__del_event(vpci->base_addr + VIRTIO_PCI_QUEUE_NOTIFY, i);
+
+	return 0;
+}
\ No newline at end of file
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index c9430cb..cbbfbeb 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -149,21 +149,27 @@ static struct virtio_ops rng_dev_virtio_ops = (struct virtio_ops) {
 	.get_size_vq		= get_size_vq,
 };
 
-void virtio_rng__init(struct kvm *kvm)
+int virtio_rng__init(struct kvm *kvm)
 {
 	struct rng_dev *rdev;
+	int r;
 
 	rdev = malloc(sizeof(*rdev));
 	if (rdev == NULL)
-		return;
+		return -ENOMEM;
 
 	rdev->fd = open("/dev/urandom", O_RDONLY);
-	if (rdev->fd < 0)
-		die("Failed initializing RNG");
+	if (rdev->fd < 0) {
+		r = rdev->fd;
+		goto cleanup;
+	}
 
 	virtio_trans_init(&rdev->vtrans, VIRTIO_PCI);
-	rdev->vtrans.trans_ops->init(kvm, &rdev->vtrans, rdev, PCI_DEVICE_ID_VIRTIO_RNG,
+	r = rdev->vtrans.trans_ops->init(kvm, &rdev->vtrans, rdev, PCI_DEVICE_ID_VIRTIO_RNG,
 					VIRTIO_ID_RNG, PCI_CLASS_RNG);
+	if (r < 0)
+		goto cleanup;
+
 	rdev->vtrans.virtio_ops = &rng_dev_virtio_ops;
 
 	list_add_tail(&rdev->list, &rdevs);
@@ -175,14 +181,23 @@ void virtio_rng__init(struct kvm *kvm)
 						"Please make sure that the guest kernel was "
 						"compiled with CONFIG_HW_RANDOM_VIRTIO=y enabled "
 						"in its .config");
+	return 0;
+cleanup:
+	close(rdev->fd);
+	free(rdev);
+
+	return r;
 }
 
-void virtio_rng__delete_all(struct kvm *kvm)
+int virtio_rng__uninit(struct kvm *kvm)
 {
 	struct rng_dev *rdev, *tmp;
 
 	list_for_each_entry_safe(rdev, tmp, &rdevs, list) {
 		list_del(&rdev->list);
+		rdev->vtrans.trans_ops->uninit(kvm, &rdev->vtrans);
 		free(rdev);
 	}
+
+	return 0;
 }
diff --git a/tools/kvm/virtio/trans.c b/tools/kvm/virtio/trans.c
index 50c206d..cd4fc7e 100644
--- a/tools/kvm/virtio/trans.c
+++ b/tools/kvm/virtio/trans.c
@@ -13,10 +13,12 @@ int virtio_trans_init(struct virtio_trans *vtrans, enum virtio_trans_type type)
 	case VIRTIO_PCI:
 		trans = calloc(sizeof(struct virtio_pci), 1);
 		if (!trans)
-			die("Failed allocating virtio transport");
+			return -ENOMEM;
 		vtrans->virtio = trans;
 		vtrans->trans_ops = virtio_pci__get_trans_ops();
 	default:
 		return -1;
 	};
+
+	return 0;
 }
\ No newline at end of file
diff --git a/tools/kvm/x86/irq.c b/tools/kvm/x86/irq.c
index 3683cb4..4ee5e56 100644
--- a/tools/kvm/x86/irq.c
+++ b/tools/kvm/x86/irq.c
@@ -46,7 +46,7 @@ static struct pci_dev *search(struct rb_root *root, u32 id)
 	struct rb_node *node = root->rb_node;
 
 	while (node) {
-		struct pci_dev *data = container_of(node, struct pci_dev, node);
+		struct pci_dev *data = rb_entry(node, struct pci_dev, node);
 		int result;
 
 		result = id - data->id;
@@ -175,8 +175,23 @@ int irq__init(struct kvm *kvm)
 
 int irq__uninit(struct kvm *kvm)
 {
+	struct rb_node *ent;
+
 	free(irq_routing);
 
+	for (ent = rb_first(&pci_tree); ent; ent = rb_next(ent)) {
+		struct pci_dev *dev;
+		struct irq_line *line;
+		struct list_head *node, *tmp;
+
+		dev = rb_entry(ent, struct pci_dev, node);
+		list_for_each_safe(node, tmp, &dev->lines) {
+			line = list_entry(node, struct irq_line, node);
+			free(line);
+		}
+		free(dev);
+	}
+
 	return 0;
 }
 
-- 
1.7.8

Re: [RFC 01/12] kvm tools: Split kvm_cmd_run into init, work and uninit

[Pekka Enberg]

On Mon, 2011-12-19 at 15:58 +0200, Sasha Levin wrote:
> +int kvm_cmd_run(int argc, const char **argv, const char *prefix)
> +{
> +	int r, ret;
> +
> +	r = kvm_cmd_run_init(argc, argv);
> +	ret = kvm_cmd_run_work();
> +	r = kvm_cmd_run_uninit(ret);
> +
> +	return ret;
>  }

What's going on here? Why do you bother saving 'r' if you don't use it
for anything?

			Pekka

Re: [RFC 00/12] Overhaul of error handling and module init/uninit

[Pekka Enberg]

On Mon, Dec 19, 2011 at 3:58 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> This patch series is really a work in progress, but is mature enough to
> cover most of the different modules we have in the tool, and receive
> comments regarding the work done so far and any future work.
>
> There were three main goals while doing this work:
> 1. Less die(), more clean exits. We are not interested in hitting a die()
> during regular program flow. die() should be reserved to extreme cases where
> we are not able to report the error back.
>
> 2. Adding actual error handling where it was missing. Starting from simple
> things like checking malloc()s, through making functions report their failures
> (this meant switching lots of functions from 'void' to 'int' since they can
> actually fail), and all the way to being able to handle errors by
> uninitializing and exiting gracefully.
>
> 3. Getting initialization and uninitialization of modules somewhat
> standard. The main purpose here is to be able to either add __init functions
> or to be able to call all initialization functions from a single place without
> code duplication.

You're mixing up cleanups (especially whitespace ones) and fixes which
is painful to deal with. So how about submitting the cleanups first
without changing code logic to get them out of the way?

                     Pekka

Re: [RFC 01/12] kvm tools: Split kvm_cmd_run into init, work and uninit

[Asias He]

On 12/20/2011 06:09 PM, Sasha Levin wrote:
> On Mon, 2011-12-19 at 23:26 +0200, Pekka Enberg wrote:
>> On Mon, 2011-12-19 at 15:58 +0200, Sasha Levin wrote:
>>> +int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>>> +{
>>> +	int r, ret;
>>> +
>>> +	r = kvm_cmd_run_init(argc, argv);
>>> +	ret = kvm_cmd_run_work();
>>> +	r = kvm_cmd_run_uninit(ret);
>>> +
>>> +	return ret;
>>>  }
>>
>> What's going on here? Why do you bother saving 'r' if you don't use it
>> for anything?
> 
> It was part of my plans to get kvm_cmd_run_{init, uninit} as a simple

Can we have a shorter name for 'uninit', e.g. 'fini', thus we will have
{init, fini}.

> for(;;) through a init/uninit function pointer array, right now it's
> simply meaningless there.
> 


-- 
Asias He

Re: [RFC 01/12] kvm tools: Split kvm_cmd_run into init, work and uninit

[Sasha Levin]

On Mon, 2011-12-19 at 23:26 +0200, Pekka Enberg wrote:
> On Mon, 2011-12-19 at 15:58 +0200, Sasha Levin wrote:
> > +int kvm_cmd_run(int argc, const char **argv, const char *prefix)
> > +{
> > +	int r, ret;
> > +
> > +	r = kvm_cmd_run_init(argc, argv);
> > +	ret = kvm_cmd_run_work();
> > +	r = kvm_cmd_run_uninit(ret);
> > +
> > +	return ret;
> >  }
> 
> What's going on here? Why do you bother saving 'r' if you don't use it
> for anything?

It was part of my plans to get kvm_cmd_run_{init, uninit} as a simple
for(;;) through a init/uninit function pointer array, right now it's
simply meaningless there.

-- 

Sasha.