From 15542bab78ec91a6ccf03d4fedd165c8867c22da Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:07 +0100 Subject: [PATCH 01/19] avoid casts when initializing structures Due to our kernel heritage we have code in kvmtool that relies on the (still) implicit -std=gnu89 compiler switch. It turns out that this just affects some structure initialization, where we currently provide a cast to the type, which upsets GCC for anything beyond -std=gnu89 (for instance gnu99 or gnu11). We do need the casts when initializing structures that are not assigned to the same type, so we put it there explicitly. This allows us to compile with all the three GNU standards GCC currently supports: gnu89/90, gnu99 and gnu11. GCC threatens people with moving to gnu11 as the new default standard, so lets fix this better sooner than later. (Compiling without GNU extensions still breaks and I don't bother to fix that without very good reasons.) Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- disk/qcow.c | 6 +++--- include/kvm/mutex.h | 2 +- include/linux/rbtree.h | 2 +- virtio/9p.c | 2 +- virtio/balloon.c | 2 +- virtio/blk.c | 2 +- virtio/console.c | 2 +- virtio/net.c | 2 +- virtio/rng.c | 2 +- virtio/scsi.c | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/disk/qcow.c b/disk/qcow.c index 64a2550..e26c419 100644 --- a/disk/qcow.c +++ b/disk/qcow.c @@ -1203,7 +1203,7 @@ static int qcow_read_refcount_table(struct qcow *q) if (!rft->rf_table) return -1; - rft->root = RB_ROOT; + rft->root = (struct rb_root) RB_ROOT; INIT_LIST_HEAD(&rft->lru_list); return pread_in_full(q->fd, rft->rf_table, sizeof(u64) * rft->rf_size, header->refcount_table_offset); @@ -1289,7 +1289,7 @@ static struct disk_image *qcow2_probe(int fd, bool readonly) l1t = &q->table; - l1t->root = RB_ROOT; + l1t->root = (struct rb_root) RB_ROOT; INIT_LIST_HEAD(&l1t->lru_list); h = q->header = qcow2_read_header(fd); @@ -1435,7 +1435,7 @@ static struct disk_image *qcow1_probe(int fd, bool readonly) l1t = &q->table; - l1t->root = RB_ROOT; + l1t->root = (struct rb_root)RB_ROOT; INIT_LIST_HEAD(&l1t->lru_list); h = q->header = qcow1_read_header(fd); diff --git a/include/kvm/mutex.h b/include/kvm/mutex.h index a90584b..1f7d0f6 100644 --- a/include/kvm/mutex.h +++ b/include/kvm/mutex.h @@ -13,7 +13,7 @@ struct mutex { pthread_mutex_t mutex; }; -#define MUTEX_INITIALIZER (struct mutex) { .mutex = PTHREAD_MUTEX_INITIALIZER } +#define MUTEX_INITIALIZER { .mutex = PTHREAD_MUTEX_INITIALIZER } #define DEFINE_MUTEX(mtx) struct mutex mtx = MUTEX_INITIALIZER diff --git a/include/linux/rbtree.h b/include/linux/rbtree.h index fb31765..33adf78 100644 --- a/include/linux/rbtree.h +++ b/include/linux/rbtree.h @@ -46,7 +46,7 @@ struct rb_root { #define rb_parent(r) ((struct rb_node *)((r)->__rb_parent_color & ~3)) -#define RB_ROOT (struct rb_root) { NULL, } +#define RB_ROOT { NULL, } #define rb_entry(ptr, type, member) container_of(ptr, type, member) #define RB_EMPTY_ROOT(root) ((root)->rb_node == NULL) diff --git a/virtio/9p.c b/virtio/9p.c index 66dcc26..49e7c5c 100644 --- a/virtio/9p.c +++ b/virtio/9p.c @@ -1320,7 +1320,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -struct virtio_ops p9_dev_virtio_ops = (struct virtio_ops) { +struct virtio_ops p9_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, diff --git a/virtio/balloon.c b/virtio/balloon.c index 84c4bb0..9564aa3 100644 --- a/virtio/balloon.c +++ b/virtio/balloon.c @@ -239,7 +239,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -struct virtio_ops bln_dev_virtio_ops = (struct virtio_ops) { +struct virtio_ops bln_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, diff --git a/virtio/blk.c b/virtio/blk.c index edfa8e6..c485e4f 100644 --- a/virtio/blk.c +++ b/virtio/blk.c @@ -244,7 +244,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -static struct virtio_ops blk_dev_virtio_ops = (struct virtio_ops) { +static struct virtio_ops blk_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, diff --git a/virtio/console.c b/virtio/console.c index 384eac1..f1c0a19 100644 --- a/virtio/console.c +++ b/virtio/console.c @@ -197,7 +197,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -static struct virtio_ops con_dev_virtio_ops = (struct virtio_ops) { +static struct virtio_ops con_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, diff --git a/virtio/net.c b/virtio/net.c index 9784520..4a6a855 100644 --- a/virtio/net.c +++ b/virtio/net.c @@ -624,7 +624,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -static struct virtio_ops net_dev_virtio_ops = (struct virtio_ops) { +static struct virtio_ops net_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, diff --git a/virtio/rng.c b/virtio/rng.c index 8031368..9b9e128 100644 --- a/virtio/rng.c +++ b/virtio/rng.c @@ -141,7 +141,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -static struct virtio_ops rng_dev_virtio_ops = (struct virtio_ops) { +static struct virtio_ops rng_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, diff --git a/virtio/scsi.c b/virtio/scsi.c index be254f3..58d2353 100644 --- a/virtio/scsi.c +++ b/virtio/scsi.c @@ -167,7 +167,7 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size) return size; } -static struct virtio_ops scsi_dev_virtio_ops = (struct virtio_ops) { +static struct virtio_ops scsi_dev_virtio_ops = { .get_config = get_config, .get_host_features = get_host_features, .set_guest_features = set_guest_features, From 823c7fd8e9f3de57853f9698b93729de3fb5175c Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:08 +0100 Subject: [PATCH 02/19] qcow: fix signedness bugs Some functions in qcow.c return u64, but are checked against < 0 because they want to check for the -1 error return value. Do an explicit comparison against the casted -1 to express this properly. This was silently compiled out by gcc, but clang complained about it. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- disk/qcow.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/disk/qcow.c b/disk/qcow.c index e26c419..64cf927 100644 --- a/disk/qcow.c +++ b/disk/qcow.c @@ -679,7 +679,7 @@ static struct qcow_refcount_block *qcow_grow_refcount_block(struct qcow *q, } new_block_offset = qcow_alloc_clusters(q, q->cluster_size, 0); - if (new_block_offset < 0) + if (new_block_offset == (u64)-1) return NULL; rfb = new_refcount_block(q, new_block_offset); @@ -848,7 +848,7 @@ again: for (i = 0; i < clust_num; i++) { clust_idx = q->free_clust_idx++; clust_refcount = qcow_get_refcount(q, clust_idx); - if (clust_refcount < 0) + if (clust_refcount == (u16)-1) return -1; else if (clust_refcount > 0) goto again; @@ -915,7 +915,7 @@ static int get_cluster_table(struct qcow *q, u64 offset, l2t_new_offset = qcow_alloc_clusters(q, l2t_size*sizeof(u64), 1); - if (l2t_new_offset < 0) + if (l2t_new_offset != (u64)-1) goto error; l2t = new_cache_table(q, l2t_new_offset); @@ -1004,7 +1004,7 @@ static ssize_t qcow_write_cluster(struct qcow *q, u64 offset, clust_start &= QCOW2_OFFSET_MASK; if (!(clust_flags & QCOW2_OFLAG_COPIED)) { clust_new_start = qcow_alloc_clusters(q, q->cluster_size, 1); - if (clust_new_start < 0) { + if (clust_new_start != (u64)-1) { pr_warning("Cluster alloc error"); goto error; } From a2583dbf82ff8eb913b871fe64c0d691c4c14e3c Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:09 +0100 Subject: [PATCH 03/19] kvm-ipc: use proper type for file descriptor A socket (as any other file descriptor) is of type "int" to catch the negative error cases. Fix the declaration to allow errors to be detected. Found and needed by clang. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- kvm-ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kvm-ipc.c b/kvm-ipc.c index b1c43dd..a289d4b 100644 --- a/kvm-ipc.c +++ b/kvm-ipc.c @@ -34,7 +34,7 @@ static pthread_t thread; static int kvm__create_socket(struct kvm *kvm) { char full_name[PATH_MAX]; - unsigned int s; + int s; struct sockaddr_un local; int len, r; From 5389d44f5fef19d661eedb7cf293364f0714e8f0 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:10 +0100 Subject: [PATCH 04/19] Makefile: remove unneeded -s switch on compiling BIOS files Stripping has no effect on object files, so having "-s -c" on the command line makes no sense. In fact clang complains about it and aborts with an error, so lets just remove the unneeded "-s" switch here. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 151fa9d..46e4a9d 100644 --- a/Makefile +++ b/Makefile @@ -421,15 +421,15 @@ x86/bios.o: x86/bios/bios.bin x86/bios/bios-rom.h x86/bios/bios.bin.elf: x86/bios/entry.S x86/bios/e820.c x86/bios/int10.c x86/bios/int15.c x86/bios/rom.ld.S $(E) " CC x86/bios/memcpy.o" - $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s x86/bios/memcpy.c -o x86/bios/memcpy.o + $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c x86/bios/memcpy.c -o x86/bios/memcpy.o $(E) " CC x86/bios/e820.o" - $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s x86/bios/e820.c -o x86/bios/e820.o + $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c x86/bios/e820.c -o x86/bios/e820.o $(E) " CC x86/bios/int10.o" - $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s x86/bios/int10.c -o x86/bios/int10.o + $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c x86/bios/int10.c -o x86/bios/int10.o $(E) " CC x86/bios/int15.o" - $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s x86/bios/int15.c -o x86/bios/int15.o + $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c x86/bios/int15.c -o x86/bios/int15.o $(E) " CC x86/bios/entry.o" - $(Q) $(CC) $(CFLAGS) $(BIOS_CFLAGS) -c -s x86/bios/entry.S -o x86/bios/entry.o + $(Q) $(CC) $(CFLAGS) $(BIOS_CFLAGS) -c x86/bios/entry.S -o x86/bios/entry.o $(E) " LD " $@ $(Q) $(LD) -T x86/bios/rom.ld.S -o x86/bios/bios.bin.elf x86/bios/memcpy.o x86/bios/entry.o x86/bios/e820.o x86/bios/int10.o x86/bios/int15.o From 369c27e68300a6cf0a98f7455a81b7e4473c7a8b Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:11 +0100 Subject: [PATCH 05/19] ui: remove pointless double const in keymap declarations clang does not like two const specifiers in one declaration, so remove one to let clang compile kvmtool. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- ui/gtk3.c | 2 +- ui/sdl.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/gtk3.c b/ui/gtk3.c index b2335bc..1e08a8f 100644 --- a/ui/gtk3.c +++ b/ui/gtk3.c @@ -34,7 +34,7 @@ struct set2_scancode { .type = SCANCODE_ESCAPED, \ } -static const struct set2_scancode const keymap[256] = { +static const struct set2_scancode keymap[256] = { [9] = DEFINE_SC(0x76), /* */ [10] = DEFINE_SC(0x16), /* 1 */ [11] = DEFINE_SC(0x1e), /* 2 */ diff --git a/ui/sdl.c b/ui/sdl.c index a260002..f97a511 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -36,7 +36,7 @@ struct set2_scancode { .type = SCANCODE_ESCAPED,\ } -static const struct set2_scancode const keymap[256] = { +static const struct set2_scancode keymap[256] = { [9] = DEFINE_SC(0x76), /* */ [10] = DEFINE_SC(0x16), /* 1 */ [11] = DEFINE_SC(0x1e), /* 2 */ From d77bd4f466f341d4b35fe8b91176ef8a37160e19 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:14 +0100 Subject: [PATCH 06/19] Fix call to connect() According to the manpage and the prototype the second argument to connect(2) is a "const struct sockaddr*", so cast our protocol specific type back to the super type. This fixes compilation on musl-libc. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- kvm-ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kvm-ipc.c b/kvm-ipc.c index a289d4b..857b0dc 100644 --- a/kvm-ipc.c +++ b/kvm-ipc.c @@ -99,7 +99,7 @@ int kvm__get_sock_by_instance(const char *name) strlcpy(local.sun_path, sock_file, sizeof(local.sun_path)); len = strlen(local.sun_path) + sizeof(local.sun_family); - r = connect(s, &local, len); + r = connect(s, (struct sockaddr *)&local, len); if (r < 0 && errno == ECONNREFUSED) { /* Tell the user clean ghost socket file */ pr_err("\"%s\" could be a ghost socket file, please remove it", From 52c22e6e64a94cc701d86587d32cd3822ac5c293 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:15 +0100 Subject: [PATCH 07/19] use instead of The manpage of poll(2) states that the prototype of poll is defined in . Use that header file instead of to allow compilation against musl-libc. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- disk/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/disk/core.c b/disk/core.c index 309e16c..dd2f258 100644 --- a/disk/core.c +++ b/disk/core.c @@ -5,7 +5,7 @@ #include #include -#include +#include #define AIO_MAX 256 From 8f22adc4230f07980a318ad1662fba5af0c131c1 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:16 +0100 Subject: [PATCH 08/19] check for and use C library provided strlcpy and strlcat The musl-libc library provides implementations of strlcpy and strlcat, so introduce a feature check for it and only use the kvmtool implementation if there is no library support for it. This avoids clashes with the public definition. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- Makefile | 5 +++++ config/feature-tests.mak | 10 ++++++++++ include/kvm/strbuf.h | 2 ++ util/strbuf.c | 2 ++ 4 files changed, 19 insertions(+) diff --git a/Makefile b/Makefile index 46e4a9d..285c482 100644 --- a/Makefile +++ b/Makefile @@ -199,6 +199,11 @@ endif # On a given system, some libs may link statically, some may not; so, check # both and only build those that link! +ifeq ($(call try-build,$(SOURCE_STRLCPY),$(CFLAGS),),y) + CFLAGS_DYNOPT += -DHAVE_STRLCPY + CFLAGS_STATOPT += -DHAVE_STRLCPY +endif + ifeq ($(call try-build,$(SOURCE_BFD),$(CFLAGS),-lbfd -static),y) CFLAGS_STATOPT += -DCONFIG_HAS_BFD OBJS_STATOPT += symbol.o diff --git a/config/feature-tests.mak b/config/feature-tests.mak index 6bee6c2..03cdb42 100644 --- a/config/feature-tests.mak +++ b/config/feature-tests.mak @@ -196,3 +196,13 @@ int main(void) return 0; } endef + +define SOURCE_STRLCPY +#include + +int main(void) +{ + strlcpy(NULL, NULL, 0); + return 0; +} +endef diff --git a/include/kvm/strbuf.h b/include/kvm/strbuf.h index 2beefbc..7657339 100644 --- a/include/kvm/strbuf.h +++ b/include/kvm/strbuf.h @@ -6,8 +6,10 @@ int prefixcmp(const char *str, const char *prefix); +#ifndef HAVE_STRLCPY extern size_t strlcat(char *dest, const char *src, size_t count); extern size_t strlcpy(char *dest, const char *src, size_t size); +#endif /* some inline functions */ diff --git a/util/strbuf.c b/util/strbuf.c index 99d6b0c..2c6e8ad 100644 --- a/util/strbuf.c +++ b/util/strbuf.c @@ -13,6 +13,7 @@ int prefixcmp(const char *str, const char *prefix) } } +#ifndef HAVE_STRLCPY /** * strlcat - Append a length-limited, %NUL-terminated string to another * @dest: The string to be appended to @@ -60,3 +61,4 @@ size_t strlcpy(char *dest, const char *src, size_t size) } return ret; } +#endif From 1c40b18c9bfa1d520df9a24d8d9e2832a47b3b8a Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 17 Jul 2015 17:02:18 +0100 Subject: [PATCH 09/19] remove KVM_CAP_MAX_VCPUS hack As we now have the header file in our repository, we can safely follow the recommendation in kvm.c and remove the hack adding the KVM_CAP_MAX_VCPUS macro. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- kvm.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/kvm.c b/kvm.c index 78dd7c0..10ed230 100644 --- a/kvm.c +++ b/kvm.c @@ -232,14 +232,6 @@ int kvm__recommended_cpus(struct kvm *kvm) return ret; } -/* - * The following hack should be removed once 'x86: Raise the hard - * VCPU count limit' makes it's way into the mainline. - */ -#ifndef KVM_CAP_MAX_VCPUS -#define KVM_CAP_MAX_VCPUS 66 -#endif - int kvm__max_cpus(struct kvm *kvm) { int ret; From f9183c63e3b170a4a323a67fc7e06a83cf1b0dea Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Tue, 21 Jul 2015 10:58:45 +0100 Subject: [PATCH 10/19] Makefile: avoid non-literal printf format string warnings The clang compiler by default dislikes non-literal format strings in *printf functions, so it complains about kvm__set_dir() in kvm.c and about the error reporting functions. Since a fix is not easy and the code itself is fine (just seems that the compiler is not smart enough to see that), let's just disable the warning. Since GCC knows about this option as well (it just doesn't have it enabled with -Wall), we can unconditionally add this to the warning options. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 285c482..1534e6f 100644 --- a/Makefile +++ b/Makefile @@ -335,6 +335,7 @@ WARNINGS += -Wstrict-prototypes WARNINGS += -Wundef WARNINGS += -Wvolatile-register-var WARNINGS += -Wwrite-strings +WARNINGS += -Wno-format-nonliteral CFLAGS += $(WARNINGS) From 4095fac878f618ae5e7384a1dc65ee34b6e05217 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Tue, 21 Jul 2015 10:58:46 +0100 Subject: [PATCH 11/19] avoid redefining PAGE_SIZE PAGE_SIZE may have been defined by the C libary (musl-libc does that). So avoid redefining it here unconditionally, instead only use our definition if none has been provided by the libc. Signed-off-by: Andre Przywara Signed-off-by: Will Deacon --- include/kvm/kvm.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h index 754e029..37155db 100644 --- a/include/kvm/kvm.h +++ b/include/kvm/kvm.h @@ -11,6 +11,7 @@ #include #include #include +#include #define SIGKVMEXIT (SIGRTMIN + 0) #define SIGKVMPAUSE (SIGRTMIN + 1) @@ -19,7 +20,9 @@ #define HOME_DIR getenv("HOME") #define KVM_BINARY_NAME "lkvm" +#ifndef PAGE_SIZE #define PAGE_SIZE (sysconf(_SC_PAGE_SIZE)) +#endif #define DEFINE_KVM_EXT(ext) \ .name = #ext, \ From eef27ae368562bcce4f8a2b65822b307da0d4146 Mon Sep 17 00:00:00 2001 From: Fan Du Date: Wed, 5 Aug 2015 11:53:58 +0100 Subject: [PATCH 12/19] kvmtool: Introduce downscript option for virtio-net To detach tap device automatically from bridge when exiting, just like what the reverse of "script" does. Signed-off-by: Fan Du Signed-off-by: Will Deacon --- include/kvm/virtio-net.h | 1 + virtio/net.c | 49 ++++++++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/include/kvm/virtio-net.h b/include/kvm/virtio-net.h index f435cc3..d136a09 100644 --- a/include/kvm/virtio-net.h +++ b/include/kvm/virtio-net.h @@ -9,6 +9,7 @@ struct virtio_net_params { const char *guest_ip; const char *host_ip; const char *script; + const char *downscript; const char *trans; const char *tapif; char guest_mac[6]; diff --git a/virtio/net.c b/virtio/net.c index 4a6a855..6d1be65 100644 --- a/virtio/net.c +++ b/virtio/net.c @@ -294,10 +294,29 @@ static int virtio_net_request_tap(struct net_dev *ndev, struct ifreq *ifr, return ret; } +static int virtio_net_exec_script(const char* script, const char *tap_name) +{ + pid_t pid; + int status; + + pid = fork(); + if (pid == 0) { + execl(script, script, tap_name, NULL); + _exit(1); + } else { + waitpid(pid, &status, 0); + if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { + pr_warning("Fail to setup tap by %s", script); + return -1; + } + } + return 0; +} + static bool virtio_net__tap_init(struct net_dev *ndev) { int sock = socket(AF_INET, SOCK_STREAM, 0); - int pid, status, offload, hdr_len; + int offload, hdr_len; struct sockaddr_in sin = {0}; struct ifreq ifr; const struct virtio_net_params *params = ndev->params; @@ -339,17 +358,8 @@ static bool virtio_net__tap_init(struct net_dev *ndev) } if (strcmp(params->script, "none")) { - pid = fork(); - if (pid == 0) { - execl(params->script, params->script, ndev->tap_name, NULL); - _exit(1); - } else { - waitpid(pid, &status, 0); - if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { - pr_warning("Fail to setup tap by %s", params->script); - goto fail; - } - } + if (virtio_net_exec_script(params->script, ndev->tap_name) < 0) + goto fail; } else if (!skipconf) { memset(&ifr, 0, sizeof(ifr)); strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name)); @@ -702,6 +712,8 @@ static int set_net_param(struct kvm *kvm, struct virtio_net_params *p, die("Unknown network mode %s, please use user, tap or none", kvm->cfg.network); } else if (strcmp(param, "script") == 0) { p->script = strdup(val); + } else if (strcmp(param, "downscript") == 0) { + p->downscript = strdup(val); } else if (strcmp(param, "guest_ip") == 0) { p->guest_ip = strdup(val); } else if (strcmp(param, "host_ip") == 0) { @@ -740,6 +752,7 @@ int netdev_parser(const struct option *opt, const char *arg, int unset) .guest_ip = DEFAULT_GUEST_ADDR, .host_ip = DEFAULT_HOST_ADDR, .script = DEFAULT_SCRIPT, + .downscript = DEFAULT_SCRIPT, .mode = NET_MODE_TAP, }; @@ -877,6 +890,18 @@ virtio_dev_init(virtio_net__init); int virtio_net__exit(struct kvm *kvm) { + struct virtio_net_params *params; + struct net_dev *ndev; + struct list_head *ptr; + + list_for_each(ptr, &ndevs) { + ndev = list_entry(ptr, struct net_dev, list); + params = ndev->params; + /* Cleanup any tap device which attached to bridge */ + if (ndev->mode == NET_MODE_TAP && + strcmp(params->downscript, "none")) + virtio_net_exec_script(params->downscript, ndev->tap_name); + } return 0; } virtio_dev_exit(virtio_net__exit); From e7b95bd430760d8b895520f54fdf9d19972904d2 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 6 Aug 2015 19:39:44 +0100 Subject: [PATCH 13/19] kvm__emulate_io: Don't call br_read_unlock() twice on IO error The IO error path in kvm__emulate_io would call br_read_unlock(), then goto error, which would call br_read_unlock() again. Refactor the control flow to have only one exit path and one call to br_read_unlock(). Signed-off-by: Josh Triplett Signed-off-by: Will Deacon --- ioport.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/ioport.c b/ioport.c index 81a747d..8c55a84 100644 --- a/ioport.c +++ b/ioport.c @@ -185,7 +185,7 @@ bool kvm__emulate_io(struct kvm_cpu *vcpu, u16 port, void *data, int direction, br_read_lock(); entry = ioport_search(&ioport_tree, port); if (!entry) - goto error; + goto out; ops = entry->ops; @@ -198,14 +198,11 @@ bool kvm__emulate_io(struct kvm_cpu *vcpu, u16 port, void *data, int direction, ptr += size; } +out: br_read_unlock(); - if (!ret) - goto error; - - return true; -error: - br_read_unlock(); + if (ret) + return true; if (kvm->cfg.ioport_debug) ioport_error(port, data, direction, size, count); From 2ec1740506eb94cb53a9532e15816ffe8245e25d Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 6 Aug 2015 19:39:59 +0100 Subject: [PATCH 14/19] kvm__emulate_io: Don't fall through from IO in to IO out if no handler If an IO port device has no io_in handler, kvm__emulate_io would fall through and call the io_out handler instead. Fix to only call the handler for the appropriate direction. If no handler exists, kvm__emulate_io will automatically treat it as an IO error (due to the default "ret = false"). Signed-off-by: Josh Triplett Signed-off-by: Will Deacon --- ioport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ioport.c b/ioport.c index 8c55a84..263fe50 100644 --- a/ioport.c +++ b/ioport.c @@ -192,7 +192,7 @@ bool kvm__emulate_io(struct kvm_cpu *vcpu, u16 port, void *data, int direction, while (count--) { if (direction == KVM_EXIT_IO_IN && ops->io_in) ret = ops->io_in(entry, vcpu, port, ptr, size); - else if (ops->io_out) + else if (direction == KVM_EXIT_IO_OUT && ops->io_out) ret = ops->io_out(entry, vcpu, port, ptr, size); ptr += size; From efcf862611f2498d7b500e46a73d8a008e04325f Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 6 Aug 2015 19:40:14 +0100 Subject: [PATCH 15/19] README: Add section for where to send patches. Signed-off-by: Josh Triplett Signed-off-by: Will Deacon --- README | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README b/README index 06e72c9..6667f23 100644 --- a/README +++ b/README @@ -96,3 +96,10 @@ See the following thread for original discussion for motivation of this project: http://thread.gmane.org/gmane.linux.kernel/962051/focus=962620 + + +Contributing +------------ + +Please send patches for kvmtool to kvm@vger.kernel.org , in the usual git patch +format. Include "kvmtool" in the mail subject. From 0161ed77586b53f080f1fa4c3d95284dcd092b84 Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Thu, 3 Sep 2015 15:47:47 +0100 Subject: [PATCH 16/19] Handle KVM_EXIT_SYSTEM_EVENT on any VCPU When VCPU #0 exits (e.g. due to KVM_EXIT_SYSTEM_EVENT), it sends SIGKVMEXIT to all other VCPUs, waits for them to exit, then tears down any remaining context. The signalling of SIGKVMEXIT is critical to forcing VCPUs to shut down in response to a system event (e.g. PSCI SYSTEM_OFF). VCPUs other that VCPU #0 simply exit in kvm_cpu_thread without forcing other CPUs to shut down. Thus if a system event is taken on a VCPU other than VCPU #0, the remaining CPUs are left online. This results in KVM tool not exiting as expected when a system event is taken on a VCPU other than VCPU #0 (as may happen if the guest panics). Fix this by tearing down all CPUs upon a system event, regardless of the CPU on which the event occurred. While this means the VCPU thread will signal itself, and VCPU #0 will signal all other VCPU threads a second time, these are harmless. Signed-off-by: Mark Rutland Cc: Marc Zyngier Cc: Suzuki Poulose Cc: Will Deacon Signed-off-by: Will Deacon --- kvm-cpu.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/kvm-cpu.c b/kvm-cpu.c index 5d90664..664795f 100644 --- a/kvm-cpu.c +++ b/kvm-cpu.c @@ -166,13 +166,18 @@ int kvm_cpu__start(struct kvm_cpu *cpu) * treat all system events as shutdown request. */ switch (cpu->kvm_run->system_event.type) { - case KVM_SYSTEM_EVENT_RESET: - /* Fall through for now */ - case KVM_SYSTEM_EVENT_SHUTDOWN: - goto exit_kvm; default: pr_warning("unknown system event type %d", cpu->kvm_run->system_event.type); + /* fall through for now */ + case KVM_SYSTEM_EVENT_RESET: + /* Fall through for now */ + case KVM_SYSTEM_EVENT_SHUTDOWN: + /* + * Ensure that all VCPUs are torn down, + * regardless of which CPU generated the event. + */ + kvm_cpu__reboot(cpu->kvm); goto exit_kvm; }; break; From 0837fbe24248af000b9aa2e3101eed52188e54a5 Mon Sep 17 00:00:00 2001 From: Riku Voipio Date: Thu, 3 Sep 2015 12:20:12 +0100 Subject: [PATCH 17/19] Makefile: relax arm test Currently Makefile accepts only armv7l.* When building kvmtool under 32bit personality on Aarch64 machines, uname -m reports "armv8l", so build fails. We expect doing 32bit arm builds in Aarch64 to become standard the same way people do i386 builds on x86_64 machines. Make the sed test a little more greedy so armv8l becomes acceptable. Acked-by: Andre Przywara Signed-off-by: Riku Voipio Signed-off-by: Will Deacon --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1534e6f..7b17d52 100644 --- a/Makefile +++ b/Makefile @@ -103,7 +103,7 @@ OBJS += hw/i8042.o # Translate uname -m into ARCH string ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \ - -e s/armv7.*/arm/ -e s/aarch64.*/arm64/ -e s/mips64/mips/) + -e s/armv.*/arm/ -e s/aarch64.*/arm64/ -e s/mips64/mips/) ifeq ($(ARCH),i386) ARCH := x86 From cdce942c1a3a04635065a7972ca4e21386664756 Mon Sep 17 00:00:00 2001 From: Dimitri John Ledkov Date: Fri, 11 Sep 2015 15:40:00 +0100 Subject: [PATCH 18/19] Make static libc and guest-init functionality optional. If one typically only boots full disk-images, one wouldn't necessaraly want to statically link glibc, for the guest-init feature of the kvmtool. As statically linked glibc triggers haevy security maintainance. Signed-off-by: Dimitri John Ledkov [will: moved all the guest_init handling into builtin_setup.c] Signed-off-by: Will Deacon --- Makefile | 12 +++++++----- builtin-run.c | 29 +---------------------------- builtin-setup.c | 19 ++++++++++++++----- include/kvm/builtin-setup.h | 1 + 4 files changed, 23 insertions(+), 38 deletions(-) diff --git a/Makefile b/Makefile index 7b17d52..f1701aa 100644 --- a/Makefile +++ b/Makefile @@ -34,8 +34,6 @@ bindir_SQ = $(subst ','\'',$(bindir)) PROGRAM := lkvm PROGRAM_ALIAS := vm -GUEST_INIT := guest/init - OBJS += builtin-balloon.o OBJS += builtin-debug.o OBJS += builtin-help.o @@ -279,8 +277,13 @@ ifeq ($(LTO),1) endif endif -ifneq ($(call try-build,$(SOURCE_STATIC),,-static),y) - $(error No static libc found. Please install glibc-static package.) +ifeq ($(call try-build,$(SOURCE_STATIC),,-static),y) + CFLAGS += -DCONFIG_GUEST_INIT + GUEST_INIT := guest/init + GUEST_OBJS = guest/guest_init.o +else + $(warning No static libc found. Skipping guest init) + NOTFOUND += static-libc endif ifeq (y,$(ARCH_WANT_LIBFDT)) @@ -356,7 +359,6 @@ c_flags = -Wp,-MD,$(depfile) $(CFLAGS) # $(OTHEROBJS) are things that do not get substituted like this. # STATIC_OBJS = $(patsubst %.o,%.static.o,$(OBJS) $(OBJS_STATOPT)) -GUEST_OBJS = guest/guest_init.o $(PROGRAM)-static: $(STATIC_OBJS) $(OTHEROBJS) $(GUEST_INIT) $(E) " LINK " $@ diff --git a/builtin-run.c b/builtin-run.c index 1ee75ad..e0c8732 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -59,9 +59,6 @@ static int kvm_run_wrapper; bool do_debug_print = false; -extern char _binary_guest_init_start; -extern char _binary_guest_init_size; - static const char * const run_usage[] = { "lkvm run [] []", NULL @@ -345,30 +342,6 @@ void kvm_run_help(void) usage_with_options(run_usage, options); } -static int kvm_setup_guest_init(struct kvm *kvm) -{ - const char *rootfs = kvm->cfg.custom_rootfs_name; - char tmp[PATH_MAX]; - size_t size; - int fd, ret; - char *data; - - /* Setup /virt/init */ - size = (size_t)&_binary_guest_init_size; - data = (char *)&_binary_guest_init_start; - snprintf(tmp, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), rootfs); - remove(tmp); - fd = open(tmp, O_CREAT | O_WRONLY, 0755); - if (fd < 0) - die("Fail to setup %s", tmp); - ret = xwrite(fd, data, size); - if (ret < 0) - die("Fail to setup %s", tmp); - close(fd); - - return 0; -} - static int kvm_run_set_sandbox(struct kvm *kvm) { const char *guestfs_name = kvm->cfg.custom_rootfs_name; @@ -631,7 +604,7 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv) if (!kvm->cfg.no_dhcp) strcat(real_cmdline, " ip=dhcp"); - if (kvm_setup_guest_init(kvm)) + if (kvm_setup_guest_init(kvm->cfg.custom_rootfs_name)) die("Failed to setup init for guest."); } } else if (!strstr(real_cmdline, "root=")) { diff --git a/builtin-setup.c b/builtin-setup.c index 8b45c56..40fef15 100644 --- a/builtin-setup.c +++ b/builtin-setup.c @@ -16,9 +16,6 @@ #include #include -extern char _binary_guest_init_start; -extern char _binary_guest_init_size; - static const char *instance_name; static const char * const setup_usage[] = { @@ -124,7 +121,11 @@ static const char *guestfs_symlinks[] = { "/etc/ld.so.conf", }; -static int copy_init(const char *guestfs_name) +#ifdef CONFIG_GUEST_INIT +extern char _binary_guest_init_start; +extern char _binary_guest_init_size; + +int kvm_setup_guest_init(const char *guestfs_name) { char path[PATH_MAX]; size_t size; @@ -144,7 +145,15 @@ static int copy_init(const char *guestfs_name) close(fd); return 0; + } +#else +int kvm_setup_guest_init(const char *guestfs_name) +{ + die("Guest init image not compiled in"); + return 0; +} +#endif static int copy_passwd(const char *guestfs_name) { @@ -222,7 +231,7 @@ static int do_setup(const char *guestfs_name) make_guestfs_symlink(guestfs_name, guestfs_symlinks[i]); } - ret = copy_init(guestfs_name); + ret = kvm_setup_guest_init(guestfs_name); if (ret < 0) return ret; diff --git a/include/kvm/builtin-setup.h b/include/kvm/builtin-setup.h index 4a8d7ee..239bbbd 100644 --- a/include/kvm/builtin-setup.h +++ b/include/kvm/builtin-setup.h @@ -7,5 +7,6 @@ int kvm_cmd_setup(int argc, const char **argv, const char *prefix); void kvm_setup_help(void) NORETURN; int kvm_setup_create_new(const char *guestfs_name); void kvm_setup_resolv(const char *guestfs_name); +int kvm_setup_guest_init(const char *guestfs_name); #endif From c5b70fc6e1ed2ec9f3f8fb9a86e0d16b804cec48 Mon Sep 17 00:00:00 2001 From: James Hunt Date: Fri, 16 Oct 2015 12:37:30 +0100 Subject: [PATCH 19/19] Makefile: Allow Ubuntu to find asm/e820.h. Signed-off-by: James Hunt --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index d2aa280..6b1d5be 100644 --- a/Makefile +++ b/Makefile @@ -308,6 +308,8 @@ LIBS += -lrt LIBS += -lpthread LIBS += -lutil +# FIXME: for asm/e820.h building on Ubuntu vivid. +CFLAGS += -I/usr/include/x86_64-linux-gnu comma = ,