[PATCH 1/7] kvm tools: add HOME env var to hostfs

[PATCH 1/7] kvm tools: add HOME env var to hostfs

[Sasha Levin]

Add a HOME env var when booting a hostfs guest. This will point out to a home
dir within the given guest name.

This will make several apps happier when being run under hostfs.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/guest/init.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/guest/init.c b/tools/kvm/guest/init.c
index e1f377f..9655bdf 100644
--- a/tools/kvm/guest/init.c
+++ b/tools/kvm/guest/init.c
@@ -12,7 +12,10 @@
 static int run_process(char *filename)
 {
 	char *new_argv[] = { filename, NULL };
-	char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0", NULL };
+	char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0",
+				"HOME=/virt/home", NULL };
+
+	mkdir("/virt/home", 0755);
 
 	return execve(filename, new_argv, new_env);
 }
-- 
1.7.12

[PATCH 2/7] kvm tools: clean garbage from ioeventfd code

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/ioeventfd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/kvm/ioeventfd.c b/tools/kvm/ioeventfd.c
index 226876f..742b008 100644
--- a/tools/kvm/ioeventfd.c
+++ b/tools/kvm/ioeventfd.c
@@ -44,7 +44,6 @@ static void *ioeventfd__thread(void *param)
 	}
 
 done:
-	tmp = 1;
 	tmp = write(epoll_stop_fd, &tmp, sizeof(tmp));
 
 	return NULL;
-- 
1.7.12

[PATCH 3/7] kvm tools: Use the new KVM_SIGNAL_MSI ioctl to inject interrupts directly.

[Sasha Levin]

We still create GSIs and keep them for two reasons:

 - They're required by virtio-* devices.
 - There's not much overhead since we just create them when starting the
guest, they don't use anything when the guest is running.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio-pci.h |  3 +++
 tools/kvm/virtio/pci.c             | 25 +++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-pci.h b/tools/kvm/include/kvm/virtio-pci.h
index 9e036b2..44130e0c 100644
--- a/tools/kvm/include/kvm/virtio-pci.h
+++ b/tools/kvm/include/kvm/virtio-pci.h
@@ -15,6 +15,8 @@ struct virtio_pci_ioevent_param {
 	u32			vq;
 };
 
+#define VIRTIO_PCI_F_SIGNAL_MSI (1 << 0)
+
 struct virtio_pci {
 	struct pci_device_header pci_hdr;
 	void			*dev;
@@ -22,6 +24,7 @@ struct virtio_pci {
 	u16			base_addr;
 	u8			status;
 	u8			isr;
+	u32			features;
 
 	/* MSI-X */
 	u16			config_vector;
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index f17cd8a..4dc7916 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -7,6 +7,7 @@
 #include "kvm/virtio.h"
 #include "kvm/ioeventfd.h"
 
+#include <sys/ioctl.h>
 #include <linux/virtio_pci.h>
 #include <linux/byteorder.h>
 #include <string.h>
@@ -236,6 +237,17 @@ static void virtio_pci__mmio_callback(u64 addr, u8 *data, u32 len, u8 is_write,
 		memcpy(data, table + addr - offset, len);
 }
 
+static void virtio_pci__signal_msi(struct kvm *kvm, struct virtio_pci *vpci, int vec)
+{
+	struct kvm_msi msi = {
+		.address_lo = vpci->msix_table[vec].msg.address_lo,
+		.address_hi = vpci->msix_table[vec].msg.address_hi,
+		.data = vpci->msix_table[vec].msg.data,
+	};
+
+	ioctl(kvm->vm_fd, KVM_SIGNAL_MSI, &msi);
+}
+
 int virtio_pci__signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq)
 {
 	struct virtio_pci *vpci = vdev->virtio;
@@ -249,7 +261,10 @@ int virtio_pci__signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq)
 			return 0;
 		}
 
-		kvm__irq_trigger(kvm, vpci->gsis[vq]);
+		if (vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
+			virtio_pci__signal_msi(kvm, vpci, vpci->vq_vector[vq]);
+		else
+			kvm__irq_trigger(kvm, vpci->gsis[vq]);
 	} else {
 		vpci->isr = VIRTIO_IRQ_HIGH;
 		kvm__irq_trigger(kvm, vpci->pci_hdr.irq_line);
@@ -270,7 +285,10 @@ int virtio_pci__signal_config(struct kvm *kvm, struct virtio_device *vdev)
 			return 0;
 		}
 
-		kvm__irq_trigger(kvm, vpci->config_gsi);
+		if (vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
+			virtio_pci__signal_msi(kvm, vpci, vpci->vq_vector[vpci->config_vector]);
+		else
+			kvm__irq_trigger(kvm, vpci->config_gsi);
 	} else {
 		vpci->isr = VIRTIO_PCI_ISR_CONFIG;
 		kvm__irq_trigger(kvm, vpci->pci_hdr.irq_line);
@@ -347,6 +365,9 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 	if (r < 0)
 		goto free_mmio;
 
+	if (kvm__supports_extension(kvm, KVM_CAP_SIGNAL_MSI))
+		vpci->features |= VIRTIO_PCI_F_SIGNAL_MSI;
+
 	vpci->pci_hdr.irq_pin	= pin;
 	vpci->pci_hdr.irq_line	= line;
 	r = pci__register(&vpci->pci_hdr, ndev);
-- 
1.7.12

[PATCH 4/7] kvm tools: fix warnings in virtio-blk

[Sasha Levin]

Fix up warnings related to not checking return value of read/write by actually
handling errors there.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/blk.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index 740442a..d988365 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -184,9 +184,12 @@ static void *virtio_blk_thread(void *dev)
 {
 	struct blk_dev *bdev = dev;
 	u64 data;
+	int r;
 
 	while (1) {
-		read(bdev->io_efd, &data, sizeof(u64));
+		r = read(bdev->io_efd, &data, sizeof(u64));
+		if (r < 0)
+			continue;
 		virtio_blk_do_io(bdev->kvm, &bdev->vqs[0], bdev);
 	}
 
@@ -198,8 +201,11 @@ static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	struct blk_dev *bdev = dev;
 	u64 data = 1;
+	int r;
 
-	write(bdev->io_efd, &data, sizeof(data));
+	r = write(bdev->io_efd, &data, sizeof(data));
+	if (r < 0)
+		return r;
 
 	return 0;
 }
-- 
1.7.12

[PATCH 5/7] kvm tools: enable LTO

[Sasha Levin]

Build with -flto set, which should enable link-time-optimizations.

I'm not sure if it provides a significant performance increase, but
it's probably just worth it for catching issues which it may cause.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 8aa0125..0e2fa66 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -243,7 +243,7 @@ DEFINES	+= -DKVMTOOLS_VERSION='"$(KVMTOOLS_VERSION)"'
 DEFINES	+= -DBUILD_ARCH='"$(ARCH)"'
 
 KVM_INCLUDE := include
-CFLAGS	+= $(CPPFLAGS) $(DEFINES) -I$(KVM_INCLUDE) -I$(ARCH_INCLUDE) -I$(KINCL_PATH)/include -I$(KINCL_PATH)/arch/$(ARCH)/include/ -O2 -fno-strict-aliasing -g
+CFLAGS	+= $(CPPFLAGS) $(DEFINES) -I$(KVM_INCLUDE) -I$(ARCH_INCLUDE) -I$(KINCL_PATH)/include -I$(KINCL_PATH)/arch/$(ARCH)/include/ -O2 -fno-strict-aliasing -g -flto
 
 WARNINGS += -Wall
 WARNINGS += -Wcast-align
-- 
1.7.12

[PATCH 6/7] kvm tools: 9p don't nuke fids on attach

[Sasha Levin]

We're not supposed to kill all fids when a new attach request
arrives. This used to cause issues when the guest would send
multiple attach requests.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/9p.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 27ef57b..d668106 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -117,26 +117,6 @@ static void close_fid(struct p9_dev *p9dev, u32 fid)
 	free(pfid);
 }
 
-static void clear_all_fids(struct p9_dev *p9dev)
-{
-	struct rb_node *node = rb_first(&p9dev->fids);
-
-	while (node) {
-		struct p9_fid *fid = rb_entry(node, struct p9_fid, node);
-
-		if (fid->fd > 0)
-			close(fid->fd);
-
-		if (fid->dir)
-			closedir(fid->dir);
-
-		rb_erase(&fid->node, &p9dev->fids);
-		free(fid);
-
-		node = rb_first(&p9dev->fids);
-	}
-}
-
 static void virtio_p9_set_reply_header(struct p9_pdu *pdu, u32 size)
 {
 	u8 cmd;
@@ -443,8 +423,6 @@ static void virtio_p9_attach(struct p9_dev *p9dev,
 	free(uname);
 	free(aname);
 
-	clear_all_fids(p9dev);
-
 	if (lstat(p9dev->root_dir, &st) < 0)
 		goto err_out;
 
-- 
1.7.12

[PATCH 7/7] kvm tools: simplify virtio config handling

[Sasha Levin]

Instead of a get/set for config values, just request the address of the
config region, and handle that by simply reading directly from that region.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio.h |  3 +--
 tools/kvm/virtio/9p.c          | 12 ++----------
 tools/kvm/virtio/balloon.c     | 12 ++----------
 tools/kvm/virtio/blk.c         | 12 ++----------
 tools/kvm/virtio/console.c     | 12 ++----------
 tools/kvm/virtio/mmio.c        |  6 +++---
 tools/kvm/virtio/net.c         | 12 ++----------
 tools/kvm/virtio/pci.c         |  4 ++--
 tools/kvm/virtio/rng.c         |  8 +-------
 tools/kvm/virtio/scsi.c        | 12 ++----------
 10 files changed, 19 insertions(+), 74 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index 71b6bad..5dc2544 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -78,8 +78,7 @@ struct virtio_device {
 };
 
 struct virtio_ops {
-	void (*set_config)(struct kvm *kvm, void *dev, u8 data, u32 offset);
-	u8 (*get_config)(struct kvm *kvm, void *dev, u32 offset);
+	u8 *(*get_config)(struct kvm *kvm, void *dev);
 	u32 (*get_host_features)(struct kvm *kvm, void *dev);
 	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features);
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 pfn);
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index d668106..c3f5280 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -1250,18 +1250,11 @@ static void virtio_p9_do_io(struct kvm *kvm, void *param)
 	}
 }
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	struct p9_dev *p9dev = dev;
 
-	((u8 *)(p9dev->config))[offset] = data;
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
-{
-	struct p9_dev *p9dev = dev;
-
-	return ((u8 *)(p9dev->config))[offset];
+	return ((u8 *)(p9dev->config));
 }
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
@@ -1323,7 +1316,6 @@ static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 }
 
 struct virtio_ops p9_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c
index a838ff4..ea64fd4 100644
--- a/tools/kvm/virtio/balloon.c
+++ b/tools/kvm/virtio/balloon.c
@@ -175,18 +175,11 @@ static void handle_mem(int fd, u32 type, u32 len, u8 *msg)
 	bdev.vdev.ops->signal_config(kvm, &bdev.vdev);
 }
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	struct bln_dev *bdev = dev;
 
-	((u8 *)(&bdev->config))[offset] = data;
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
-{
-	struct bln_dev *bdev = dev;
-
-	return ((u8 *)(&bdev->config))[offset];
+	return ((u8 *)(&bdev->config));
 }
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
@@ -241,7 +234,6 @@ static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 }
 
 struct virtio_ops bln_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index d988365..98f17a2 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -134,18 +134,11 @@ static void virtio_blk_do_io(struct kvm *kvm, struct virt_queue *vq, struct blk_
 	}
 }
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	struct blk_dev *bdev = dev;
 
-	((u8 *)(&bdev->blk_config))[offset] = data;
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
-{
-	struct blk_dev *bdev = dev;
-
-	return ((u8 *)(&bdev->blk_config))[offset];
+	return ((u8 *)(&bdev->blk_config));
 }
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
@@ -230,7 +223,6 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 }
 
 static struct virtio_ops blk_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index 4bb1365..e925a54 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -105,18 +105,11 @@ static void virtio_console_handle_callback(struct kvm *kvm, void *param)
 
 }
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	struct con_dev *cdev = dev;
 
-	((u8 *)(&cdev->config))[offset] = data;
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
-{
-	struct con_dev *cdev = dev;
-
-	return ((u8 *)(&cdev->config))[offset];
+	return ((u8 *)(&cdev->config));
 }
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
@@ -174,7 +167,6 @@ static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 }
 
 static struct virtio_ops con_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c
index 1cf0815..5971922 100644
--- a/tools/kvm/virtio/mmio.c
+++ b/tools/kvm/virtio/mmio.c
@@ -94,11 +94,11 @@ static void virtio_mmio_device_specific(u64 addr, u8 *data, u32 len,
 
 	for (i = 0; i < len; i++) {
 		if (is_write)
-			vdev->ops->set_config(vmmio->kvm, vmmio->dev,
-					      *(u8 *)data + i, addr + i);
+			vdev->ops->get_config(vmmio->kvm, vmmio->dev)[addr + i] = 
+					      *(u8 *)data + i;
 		else
 			data[i] = vdev->ops->get_config(vmmio->kvm,
-							vmmio->dev, addr + i);
+							vmmio->dev)[addr + i];
 	}
 }
 
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 8f3735b..25bc3a4 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -290,18 +290,11 @@ static struct net_dev_operations uip_ops = {
 	.tx	= uip_ops_tx,
 };
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	struct net_dev *ndev = dev;
 
-	((u8 *)(&ndev->config))[offset] = data;
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
-{
-	struct net_dev *ndev = dev;
-
-	return ((u8 *)(&ndev->config))[offset];
+	return ((u8 *)(&ndev->config));
 }
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
@@ -448,7 +441,6 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 }
 
 static struct virtio_ops net_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index 4dc7916..81f95ae 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -86,7 +86,7 @@ static bool virtio_pci__specific_io_in(struct kvm *kvm, struct virtio_device *vd
 	} else if (type == VIRTIO_PCI_O_CONFIG) {
 		u8 cfg;
 
-		cfg = vdev->ops->get_config(kvm, vpci->dev, config_offset);
+		cfg = vdev->ops->get_config(kvm, vpci->dev)[config_offset];
 		ioport__write8(data, cfg);
 		return true;
 	}
@@ -164,7 +164,7 @@ static bool virtio_pci__specific_io_out(struct kvm *kvm, struct virtio_device *v
 
 		return true;
 	} else if (type == VIRTIO_PCI_O_CONFIG) {
-		vdev->ops->set_config(kvm, vpci->dev, *(u8 *)data, config_offset);
+		vdev->ops->get_config(kvm, vpci->dev)[config_offset] = *(u8 *)data;
 
 		return true;
 	}
diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c
index 5aa632d..2b1ab39 100644
--- a/tools/kvm/virtio/rng.c
+++ b/tools/kvm/virtio/rng.c
@@ -41,12 +41,7 @@ struct rng_dev {
 static LIST_HEAD(rdevs);
 static int compat_id = -1;
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
-{
-	/* Unused */
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	/* Unused */
 	return 0;
@@ -138,7 +133,6 @@ static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 }
 
 static struct virtio_ops rng_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
diff --git a/tools/kvm/virtio/scsi.c b/tools/kvm/virtio/scsi.c
index 8276fb0..c445f08 100644
--- a/tools/kvm/virtio/scsi.c
+++ b/tools/kvm/virtio/scsi.c
@@ -29,18 +29,11 @@ struct scsi_dev {
 	struct kvm			*kvm;
 };
 
-static void set_config(struct kvm *kvm, void *dev, u8 data, u32 offset)
+static u8 *get_config(struct kvm *kvm, void *dev)
 {
 	struct scsi_dev *sdev = dev;
 
-	((u8 *)(&sdev->config))[offset] = data;
-}
-
-static u8 get_config(struct kvm *kvm, void *dev, u32 offset)
-{
-	struct scsi_dev *sdev = dev;
-
-	return ((u8 *)(&sdev->config))[offset];
+	return ((u8 *)(&sdev->config));
 }
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
@@ -174,7 +167,6 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 }
 
 static struct virtio_ops scsi_dev_virtio_ops = (struct virtio_ops) {
-	.set_config		= set_config,
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
 	.set_guest_features	= set_guest_features,
-- 
1.7.12

Re: [PATCH 1/7] kvm tools: add HOME env var to hostfs

[Cyrill Gorcunov]

On Thu, Aug 30, 2012 at 09:36:37AM +0200, Sasha Levin wrote:
> +	char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0",
> +				"HOME=/virt/home", NULL };
> +
> +	mkdir("/virt/home", 0755);

Please add check for mkdir error code. Frankly, this is a bad habbit
to assume that mkdir never fails (this could be done on top of this
series I think but should not be leaved without attention).

	Cyrill

Re: [PATCH 5/7] kvm tools: enable LTO

[Pekka Enberg]

On Thu, Aug 30, 2012 at 10:36 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> Build with -flto set, which should enable link-time-optimizations.
>
> I'm not sure if it provides a significant performance increase, but
> it's probably just worth it for catching issues which it may cause.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

Ingo, any objections to this?

> ---
>  tools/kvm/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
> index 8aa0125..0e2fa66 100644
> --- a/tools/kvm/Makefile
> +++ b/tools/kvm/Makefile
> @@ -243,7 +243,7 @@ DEFINES     += -DKVMTOOLS_VERSION='"$(KVMTOOLS_VERSION)"'
>  DEFINES        += -DBUILD_ARCH='"$(ARCH)"'
>
>  KVM_INCLUDE := include
> -CFLAGS += $(CPPFLAGS) $(DEFINES) -I$(KVM_INCLUDE) -I$(ARCH_INCLUDE) -I$(KINCL_PATH)/include -I$(KINCL_PATH)/arch/$(ARCH)/include/ -O2 -fno-strict-aliasing -g
> +CFLAGS += $(CPPFLAGS) $(DEFINES) -I$(KVM_INCLUDE) -I$(ARCH_INCLUDE) -I$(KINCL_PATH)/include -I$(KINCL_PATH)/arch/$(ARCH)/include/ -O2 -fno-strict-aliasing -g -flto
>
>  WARNINGS += -Wall
>  WARNINGS += -Wcast-align
> --
> 1.7.12
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] kvm tools: enable LTO

[Ingo Molnar]

* Pekka Enberg <penberg@kernel.org> wrote:

> On Thu, Aug 30, 2012 at 10:36 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > Build with -flto set, which should enable link-time-optimizations.
> >
> > I'm not sure if it provides a significant performance increase, but
> > it's probably just worth it for catching issues which it may cause.
> >
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> 
> Ingo, any objections to this?

No objections if you can live with a 2x-4x increase in build 
time - at worst it might cause funnies with the BIOS linker 
script and such.

Thanks,

	Ingo

Re: [PATCH 5/7] kvm tools: enable LTO

[Cyrill Gorcunov]

On Thu, Aug 30, 2012 at 10:16:54AM +0200, Ingo Molnar wrote:
> 
> * Pekka Enberg <penberg@kernel.org> wrote:
> 
> > On Thu, Aug 30, 2012 at 10:36 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > > Build with -flto set, which should enable link-time-optimizations.
> > >
> > > I'm not sure if it provides a significant performance increase, but
> > > it's probably just worth it for catching issues which it may cause.
> > >
> > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > 
> > Ingo, any objections to this?
> 
> No objections if you can live with a 2x-4x increase in build 
> time - at worst it might cause funnies with the BIOS linker 
> script and such.

Maybe we could enable it via some make option?
Say make LTO=1 or something?

	Cyrill

Re: [PATCH 5/7] kvm tools: enable LTO

[Sasha Levin]

On 08/30/2012 10:22 AM, Cyrill Gorcunov wrote:
> On Thu, Aug 30, 2012 at 10:16:54AM +0200, Ingo Molnar wrote:
>>
>> * Pekka Enberg <penberg@kernel.org> wrote:
>>
>>> On Thu, Aug 30, 2012 at 10:36 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
>>>> Build with -flto set, which should enable link-time-optimizations.
>>>>
>>>> I'm not sure if it provides a significant performance increase, but
>>>> it's probably just worth it for catching issues which it may cause.
>>>>
>>>> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
>>>
>>> Ingo, any objections to this?
>>
>> No objections if you can live with a 2x-4x increase in build 
>> time - at worst it might cause funnies with the BIOS linker 
>> script and such.
> 
> Maybe we could enable it via some make option?
> Say make LTO=1 or something?

Build time went from 6 sec to 14, I don't think it's that significant...

Re: [PATCH 5/7] kvm tools: enable LTO

[Cyrill Gorcunov]

On Thu, Aug 30, 2012 at 10:33:21AM +0200, Sasha Levin wrote:
> >>>
> >>> Ingo, any objections to this?
> >>
> >> No objections if you can live with a 2x-4x increase in build 
> >> time - at worst it might cause funnies with the BIOS linker 
> >> script and such.
> > 
> > Maybe we could enable it via some make option?
> > Say make LTO=1 or something?
> 
> Build time went from 6 sec to 14, I don't think it's that significant...

At moment. But I bet lkvm will grow with time and build time increase
as well. Still if you think we better stick with LTO, no problem,
lets do it then ;)

	Cyrill

Re: [PATCH 5/7] kvm tools: enable LTO

[Pekka Enberg]

On Thu, Aug 30, 2012 at 11:22 AM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> Maybe we could enable it via some make option?
> Say make LTO=1 or something?

That is not going to help much in catching LTO issues early.

Re: [PATCH 1/7] kvm tools: add HOME env var to hostfs

[Sasha Levin]

On 08/30/2012 09:50 AM, Cyrill Gorcunov wrote:
> On Thu, Aug 30, 2012 at 09:36:37AM +0200, Sasha Levin wrote:
>> +	char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0",
>> +				"HOME=/virt/home", NULL };
>> +
>> +	mkdir("/virt/home", 0755);
> 
> Please add check for mkdir error code. Frankly, this is a bad habbit
> to assume that mkdir never fails (this could be done on top of this
> series I think but should not be leaved without attention).

This is actually supposed to fail most of the times, since it runs as part of
the init :)

There's not much we can do if it fails though, and even at that point - this
failure isn't serious enough to even justify any further action on our end.

Re: [PATCH 1/7] kvm tools: add HOME env var to hostfs

[Pekka Enberg]

On Thu, Aug 30, 2012 at 12:44 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> On 08/30/2012 09:50 AM, Cyrill Gorcunov wrote:
>> On Thu, Aug 30, 2012 at 09:36:37AM +0200, Sasha Levin wrote:
>>> +    char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0",
>>> +                            "HOME=/virt/home", NULL };
>>> +
>>> +    mkdir("/virt/home", 0755);
>>
>> Please add check for mkdir error code. Frankly, this is a bad habbit
>> to assume that mkdir never fails (this could be done on top of this
>> series I think but should not be leaved without attention).
>
> This is actually supposed to fail most of the times, since it runs as part of
> the init :)

You could check for errno == EEXISTS, no?

On Thu, Aug 30, 2012 at 12:44 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> There's not much we can do if it fails though, and even at that point - this
> failure isn't serious enough to even justify any further action on our end.

I'm pretty sure users would appreciate a friendly warning explaining
that we are not running in "normal state".

Re: [PATCH 1/7] kvm tools: add HOME env var to hostfs

[Sasha Levin]

On 08/30/2012 11:48 AM, Pekka Enberg wrote:
> On Thu, Aug 30, 2012 at 12:44 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
>> On 08/30/2012 09:50 AM, Cyrill Gorcunov wrote:
>>> On Thu, Aug 30, 2012 at 09:36:37AM +0200, Sasha Levin wrote:
>>>> +    char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0",
>>>> +                            "HOME=/virt/home", NULL };
>>>> +
>>>> +    mkdir("/virt/home", 0755);
>>>
>>> Please add check for mkdir error code. Frankly, this is a bad habbit
>>> to assume that mkdir never fails (this could be done on top of this
>>> series I think but should not be leaved without attention).
>>
>> This is actually supposed to fail most of the times, since it runs as part of
>> the init :)
> 
> You could check for errno == EEXISTS, no?
> 
> On Thu, Aug 30, 2012 at 12:44 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
>> There's not much we can do if it fails though, and even at that point - this
>> failure isn't serious enough to even justify any further action on our end.
> 
> I'm pretty sure users would appreciate a friendly warning explaining
> that we are not running in "normal state".

This is how we ran up until now :)

I'm going to move that from init.c into the setup itself then.