[PATCH 0/3 V2] kvm tools: Support for multiple virtio-blk

[PATCH 0/3 V2] kvm tools: Support for multiple virtio-blk

[Sasha Levin]

Following patches add support for multiple virtio-blk devices.
Sample cmdline for testing:
./kvm run --image <img1.raw> --readonly --image2 <img2.raw> --readonly2

Changes in V2:
 - Multiple device support is now fully working.
 - Fixed code styling and formatting.
 - Each virtio-blk gets it's own IRQ line, updated mptable.c.
 - New issue: How do we pass multiple disk params? Currently
   the parser we took from perf doesn't support multiple arguments.

[PATCH 1/3 V2] kvm tools: Move disk_image into virtio-blk

[Sasha Levin]

There may be multiple disk images on a running guest,
each associated with a virtio-blk.

Move disk_image into virtio-blk in preperation for
multiple disk images.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/kvm.h        |    1 -
 tools/kvm/include/kvm/virtio-blk.h |    4 +++-
 tools/kvm/kvm-run.c                |    9 ++++-----
 tools/kvm/virtio-blk.c             |   13 ++++++++-----
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 327a1ab..d239f49 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -16,7 +16,6 @@ struct kvm {
 
 	int			nrcpus;		/* Number of cpus to run */
 
-	struct disk_image	*disk_image;
 	uint64_t		ram_size;
 	void			*ram_start;
 
diff --git a/tools/kvm/include/kvm/virtio-blk.h b/tools/kvm/include/kvm/virtio-blk.h
index f82bbc7..f21a0e4 100644
--- a/tools/kvm/include/kvm/virtio-blk.h
+++ b/tools/kvm/include/kvm/virtio-blk.h
@@ -1,8 +1,10 @@
 #ifndef KVM__BLK_VIRTIO_H
 #define KVM__BLK_VIRTIO_H
 
+#include "kvm/disk-image.h"
+
 struct kvm;
 
-void virtio_blk__init(struct kvm *self);
+void virtio_blk__init(struct kvm *self, struct disk_image *disk);
 
 #endif /* KVM__BLK_VIRTIO_H */
diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index 6fd46ed..b92232d 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -409,9 +409,11 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
 
 	if (image_filename) {
-		kvm->disk_image	= disk_image__open(image_filename, readonly_image);
-		if (!kvm->disk_image)
+		struct disk_image *disk = disk_image__open(image_filename, readonly_image);
+		if (!disk)
 			die("unable to load disk image %s", image_filename);
+
+		virtio_blk__init(kvm, disk);
 	}
 	free(hi);
 
@@ -429,8 +431,6 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 
 	pci__init();
 
-	virtio_blk__init(kvm);
-
 	virtio_console__init(kvm);
 
 	if (virtio_rng)
@@ -487,7 +487,6 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 			exit_code	= 1;
 	}
 
-	disk_image__close(kvm->disk_image);
 	kvm__delete(kvm);
 
 	if (!exit_code)
diff --git a/tools/kvm/virtio-blk.c b/tools/kvm/virtio-blk.c
index 9034abd..d7bda5f 100644
--- a/tools/kvm/virtio-blk.c
+++ b/tools/kvm/virtio-blk.c
@@ -28,6 +28,7 @@ struct blk_device {
 	pthread_mutex_t			mutex;
 
 	struct virtio_blk_config	blk_config;
+	struct disk_image			*disk;
 	uint32_t			host_features;
 	uint32_t			guest_features;
 	uint16_t			config_vector;
@@ -130,11 +131,11 @@ static bool virtio_blk_do_io_request(struct kvm *self, struct virt_queue *queue)
 
 	switch (req->type) {
 	case VIRTIO_BLK_T_IN:
-		block_cnt = disk_image__read_sector_iov(self->disk_image, req->sector, iov + 1, in + out - 2);
+		block_cnt = disk_image__read_sector_iov(blk_device.disk, req->sector, iov + 1, in + out - 2);
 
 		break;
 	case VIRTIO_BLK_T_OUT:
-		block_cnt = disk_image__write_sector_iov(self->disk_image, req->sector, iov + 1, in + out - 2);
+		block_cnt = disk_image__write_sector_iov(blk_device.disk, req->sector, iov + 1, in + out - 2);
 
 		break;
 
@@ -243,12 +244,14 @@ static struct pci_device_header virtio_blk_pci_device = {
 
 #define PCI_VIRTIO_BLK_DEVNUM 1
 
-void virtio_blk__init(struct kvm *self)
+void virtio_blk__init(struct kvm *self, struct disk_image *disk)
 {
-	if (!self->disk_image)
+	if (!disk)
 		return;
 
-	blk_device.blk_config.capacity = self->disk_image->size / SECTOR_SIZE;
+	blk_device.disk = disk;
+
+	blk_device.blk_config.capacity = disk->size / SECTOR_SIZE;
 
 	pci__register(&virtio_blk_pci_device, PCI_VIRTIO_BLK_DEVNUM);
 
-- 
1.7.5.rc3

[PATCH 2/3 V2] kvm tools: Add support for multiple virtio-blk

[Sasha Levin]

Add support for multiple blk_devices by un-globalizing
the current blk_device and allow multiple blk_devices.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/ioport.h |    2 +-
 tools/kvm/mptable.c            |   39 ++++++++-
 tools/kvm/virtio-blk.c         |  193 +++++++++++++++++++++++++---------------
 3 files changed, 162 insertions(+), 72 deletions(-)

diff --git a/tools/kvm/include/kvm/ioport.h b/tools/kvm/include/kvm/ioport.h
index 6020124..98a880f 100644
--- a/tools/kvm/include/kvm/ioport.h
+++ b/tools/kvm/include/kvm/ioport.h
@@ -7,7 +7,7 @@
 /* some ports we reserve for own use */
 #define IOPORT_DBG			0xe0
 #define IOPORT_VIRTIO_BLK		0xc200	/* Virtio block device */
-#define IOPORT_VIRTIO_BLK_SIZE		256
+#define IOPORT_VIRTIO_BLK_SIZE		0x200
 #define IOPORT_VIRTIO_CONSOLE		0xd200	/* Virtio console device */
 #define IOPORT_VIRTIO_CONSOLE_SIZE	256
 #define IOPORT_VIRTIO_NET		0xe200	/* Virtio network device */
diff --git a/tools/kvm/mptable.c b/tools/kvm/mptable.c
index 5bbe7ea..e777f64 100644
--- a/tools/kvm/mptable.c
+++ b/tools/kvm/mptable.c
@@ -181,6 +181,7 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	last_addr = (void *)&mpc_intsrc[1];
 	nentries++;
 
+	/* Currently we define 4 possible virtio-blk devices */
 	mpc_intsrc		= last_addr;
 	mpc_intsrc->type	= MP_INTSRC;
 	mpc_intsrc->irqtype	= mp_INT;
@@ -188,7 +189,43 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus)
 	mpc_intsrc->srcbus	= pcibusid;
 	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
 	mpc_intsrc->dstapic	= ioapicid;
-	mpc_intsrc->dstirq	= 15; /* VIRTIO_BLK_IRQ */
+	mpc_intsrc->dstirq	= 9; /* VIRTIO_BLK_IRQ */
+
+	last_addr = (void *)&mpc_intsrc[1];
+	nentries++;
+
+	mpc_intsrc		= last_addr;
+	mpc_intsrc->type	= MP_INTSRC;
+	mpc_intsrc->irqtype	= mp_INT;
+	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
+	mpc_intsrc->srcbus	= pcibusid;
+	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
+	mpc_intsrc->dstapic	= ioapicid;
+	mpc_intsrc->dstirq	= 10; /* VIRTIO_BLK_IRQ */
+
+	last_addr = (void *)&mpc_intsrc[1];
+	nentries++;
+
+	mpc_intsrc		= last_addr;
+	mpc_intsrc->type	= MP_INTSRC;
+	mpc_intsrc->irqtype	= mp_INT;
+	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
+	mpc_intsrc->srcbus	= pcibusid;
+	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
+	mpc_intsrc->dstapic	= ioapicid;
+	mpc_intsrc->dstirq	= 11; /* VIRTIO_BLK_IRQ */
+
+	last_addr = (void *)&mpc_intsrc[1];
+	nentries++;
+
+	mpc_intsrc		= last_addr;
+	mpc_intsrc->type	= MP_INTSRC;
+	mpc_intsrc->irqtype	= mp_INT;
+	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
+	mpc_intsrc->srcbus	= pcibusid;
+	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
+	mpc_intsrc->dstapic	= ioapicid;
+	mpc_intsrc->dstirq	= 12; /* VIRTIO_BLK_IRQ */
 
 	last_addr = (void *)&mpc_intsrc[1];
 	nentries++;
diff --git a/tools/kvm/virtio-blk.c b/tools/kvm/virtio-blk.c
index d7bda5f..cf6416d 100644
--- a/tools/kvm/virtio-blk.c
+++ b/tools/kvm/virtio-blk.c
@@ -17,51 +17,47 @@
 #include <inttypes.h>
 #include <pthread.h>
 
-#define VIRTIO_BLK_IRQ		15
+#define VIRTIO_BLK_IRQ		9
 #define VIRTIO_BLK_PIN		1
-
+#define VIRTIO_BLK_MAX_DEV	4
 #define NUM_VIRT_QUEUES		1
 
 #define VIRTIO_BLK_QUEUE_SIZE	128
 
+struct blk_device_job {
+	struct virt_queue	*vq;
+	struct blk_device	*blk_device;
+	void			*job_id;
+};
+
 struct blk_device {
 	pthread_mutex_t			mutex;
 
 	struct virtio_blk_config	blk_config;
-	struct disk_image			*disk;
+	struct disk_image		*disk;
 	uint32_t			host_features;
 	uint32_t			guest_features;
 	uint16_t			config_vector;
 	uint8_t				status;
+	u8				idx;
 
 	/* virtio queue */
 	uint16_t			queue_selector;
 
 	struct virt_queue		vqs[NUM_VIRT_QUEUES];
-
-	void			*jobs[NUM_VIRT_QUEUES];
+	struct blk_device_job		jobs[NUM_VIRT_QUEUES];
+	struct pci_device_header	pci_device;
 };
 
-#define DISK_SEG_MAX	126
-
-static struct blk_device blk_device = {
-	.mutex			= PTHREAD_MUTEX_INITIALIZER,
-
-	.blk_config		= (struct virtio_blk_config) {
-		/* VIRTIO_BLK_F_SEG_MAX */
-		.seg_max		= DISK_SEG_MAX,
-	},
-	/*
-	 * Note we don't set VIRTIO_BLK_F_GEOMETRY here so the
-	 * node kernel will compute disk geometry by own, the
-	 * same applies to VIRTIO_BLK_F_BLK_SIZE
-	 */
-	.host_features		= (1UL << VIRTIO_BLK_F_SEG_MAX),
-};
+static struct blk_device *blk_devices[VIRTIO_BLK_MAX_DEV];
 
-static bool virtio_blk_pci_io_device_specific_in(void *data, unsigned long offset, int size, uint32_t count)
+static bool virtio_blk_pci_io_device_specific_in(struct blk_device *blk_device,
+						void *data,
+						unsigned long offset,
+						int size,
+						uint32_t count)
 {
-	uint8_t *config_space = (uint8_t *) &blk_device.blk_config;
+	uint8_t *config_space = (uint8_t *) &blk_device->blk_config;
 
 	if (size != 1 || count != 1)
 		return false;
@@ -71,24 +67,38 @@ static bool virtio_blk_pci_io_device_specific_in(void *data, unsigned long offse
 	return true;
 }
 
+/* Translate port into device id + offset in that device addr space */
+static void virtio_blk_port2dev(u16 port,
+				u16 base,
+				u16 size,
+				u16 *dev_idx,
+				u16 *offset)
+{
+	*dev_idx	= (port - base) / size;
+	*offset		= port - (base + *dev_idx * size);
+}
 static bool virtio_blk_pci_io_in(struct kvm *self, uint16_t port, void *data, int size, uint32_t count)
 {
-	unsigned long offset;
+	u16 offset, dev_idx;
 	bool ret = true;
+	struct blk_device *blk_device;
+
+	virtio_blk_port2dev(port, IOPORT_VIRTIO_BLK, IOPORT_VIRTIO_BLK_SIZE,
+				&dev_idx, &offset);
 
-	mutex_lock(&blk_device.mutex);
+	blk_device = blk_devices[dev_idx];
 
-	offset		= port - IOPORT_VIRTIO_BLK;
+	mutex_lock(&blk_device->mutex);
 
 	switch (offset) {
 	case VIRTIO_PCI_HOST_FEATURES:
-		ioport__write32(data, blk_device.host_features);
+		ioport__write32(data, blk_device->host_features);
 		break;
 	case VIRTIO_PCI_GUEST_FEATURES:
 		ret		= false;
 		break;
 	case VIRTIO_PCI_QUEUE_PFN:
-		ioport__write32(data, blk_device.vqs[blk_device.queue_selector].pfn);
+		ioport__write32(data, blk_device->vqs[blk_device->queue_selector].pfn);
 		break;
 	case VIRTIO_PCI_QUEUE_NUM:
 		ioport__write16(data, VIRTIO_BLK_QUEUE_SIZE);
@@ -98,25 +108,27 @@ static bool virtio_blk_pci_io_in(struct kvm *self, uint16_t port, void *data, in
 		ret		= false;
 		break;
 	case VIRTIO_PCI_STATUS:
-		ioport__write8(data, blk_device.status);
+		ioport__write8(data, blk_device->status);
 		break;
 	case VIRTIO_PCI_ISR:
 		ioport__write8(data, 0x1);
-		kvm__irq_line(self, VIRTIO_BLK_IRQ, 0);
+		kvm__irq_line(self, VIRTIO_BLK_IRQ + blk_device->idx, 0);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
-		ioport__write16(data, blk_device.config_vector);
+		ioport__write16(data, blk_device->config_vector);
 		break;
 	default:
-		ret		= virtio_blk_pci_io_device_specific_in(data, offset, size, count);
+		ret = virtio_blk_pci_io_device_specific_in(blk_device, data, offset, size, count);
 	};
 
-	mutex_unlock(&blk_device.mutex);
+	mutex_unlock(&blk_device->mutex);
 
 	return ret;
 }
 
-static bool virtio_blk_do_io_request(struct kvm *self, struct virt_queue *queue)
+static bool virtio_blk_do_io_request(struct kvm *self,
+					struct blk_device *blk_device,
+					struct virt_queue *queue)
 {
 	struct iovec iov[VIRTIO_BLK_QUEUE_SIZE];
 	struct virtio_blk_outhdr *req;
@@ -131,11 +143,11 @@ static bool virtio_blk_do_io_request(struct kvm *self, struct virt_queue *queue)
 
 	switch (req->type) {
 	case VIRTIO_BLK_T_IN:
-		block_cnt = disk_image__read_sector_iov(blk_device.disk, req->sector, iov + 1, in + out - 2);
+		block_cnt = disk_image__read_sector_iov(blk_device->disk, req->sector, iov + 1, in + out - 2);
 
 		break;
 	case VIRTIO_BLK_T_OUT:
-		block_cnt = disk_image__write_sector_iov(blk_device.disk, req->sector, iov + 1, in + out - 2);
+		block_cnt = disk_image__write_sector_iov(blk_device->disk, req->sector, iov + 1, in + out - 2);
 
 		break;
 
@@ -155,58 +167,69 @@ static bool virtio_blk_do_io_request(struct kvm *self, struct virt_queue *queue)
 
 static void virtio_blk_do_io(struct kvm *kvm, void *param)
 {
-	struct virt_queue *vq = param;
+	struct blk_device_job *job = param;
+	struct virt_queue *vq = job->vq;
+	struct blk_device *blk_device = job->blk_device;
 
 	while (virt_queue__available(vq))
-		virtio_blk_do_io_request(kvm, vq);
+		virtio_blk_do_io_request(kvm, blk_device, vq);
 
-	kvm__irq_line(kvm, VIRTIO_BLK_IRQ, 1);
+	kvm__irq_line(kvm, VIRTIO_BLK_IRQ + blk_device->idx, 1);
 }
 
 static bool virtio_blk_pci_io_out(struct kvm *self, uint16_t port, void *data, int size, uint32_t count)
 {
-	unsigned long offset;
+	u16 offset, dev_idx;
 	bool ret = true;
+	struct blk_device *blk_device;
 
-	mutex_lock(&blk_device.mutex);
+	virtio_blk_port2dev(port, IOPORT_VIRTIO_BLK, IOPORT_VIRTIO_BLK_SIZE,
+						&dev_idx, &offset);
 
-	offset		= port - IOPORT_VIRTIO_BLK;
+	blk_device = blk_devices[dev_idx];
+
+	mutex_lock(&blk_device->mutex);
 
 	switch (offset) {
 	case VIRTIO_PCI_GUEST_FEATURES:
-		blk_device.guest_features	= ioport__read32(data);
+		blk_device->guest_features	= ioport__read32(data);
 		break;
 	case VIRTIO_PCI_QUEUE_PFN: {
 		struct virt_queue *queue;
+		struct blk_device_job *job;
 		void *p;
 
-		queue			= &blk_device.vqs[blk_device.queue_selector];
+		job = &blk_device->jobs[blk_device->queue_selector];
 
+		queue			= &blk_device->vqs[blk_device->queue_selector];
 		queue->pfn		= ioport__read32(data);
-
 		p			= guest_flat_to_host(self, queue->pfn << 12);
 
 		vring_init(&queue->vring, VIRTIO_BLK_QUEUE_SIZE, p, 4096);
 
-		blk_device.jobs[blk_device.queue_selector] =
-			thread_pool__add_job(self, virtio_blk_do_io, queue);
+		*job = (struct blk_device_job) {
+			.vq		= queue,
+			.blk_device	= blk_device,
+		};
+
+		job->job_id = thread_pool__add_job(self, virtio_blk_do_io, job);
 
 		break;
 	}
 	case VIRTIO_PCI_QUEUE_SEL:
-		blk_device.queue_selector	= ioport__read16(data);
+		blk_device->queue_selector	= ioport__read16(data);
 		break;
 	case VIRTIO_PCI_QUEUE_NOTIFY: {
 		uint16_t queue_index;
 		queue_index		= ioport__read16(data);
-		thread_pool__do_job(blk_device.jobs[queue_index]);
+		thread_pool__do_job(blk_device->jobs[queue_index].job_id);
 		break;
 	}
 	case VIRTIO_PCI_STATUS:
-		blk_device.status		= ioport__read8(data);
+		blk_device->status		= ioport__read8(data);
 		break;
 	case VIRTIO_MSI_CONFIG_VECTOR:
-		blk_device.config_vector	= VIRTIO_MSI_NO_VECTOR;
+		blk_device->config_vector	= VIRTIO_MSI_NO_VECTOR;
 		break;
 	case VIRTIO_MSI_QUEUE_VECTOR:
 		break;
@@ -214,7 +237,7 @@ static bool virtio_blk_pci_io_out(struct kvm *self, uint16_t port, void *data, i
 		ret		= false;
 	};
 
-	mutex_unlock(&blk_device.mutex);
+	mutex_unlock(&blk_device->mutex);
 
 	return ret;
 }
@@ -228,32 +251,62 @@ static struct ioport_operations virtio_blk_io_ops = {
 #define PCI_DEVICE_ID_VIRTIO_BLK		0x1001
 #define PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET	0x1af4
 #define PCI_SUBSYSTEM_ID_VIRTIO_BLK		0x0002
+#define PCI_VIRTIO_BLK_DEVNUM 10
 
-static struct pci_device_header virtio_blk_pci_device = {
-	.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
-	.device_id		= PCI_DEVICE_ID_VIRTIO_BLK,
-	.header_type		= PCI_HEADER_TYPE_NORMAL,
-	.revision_id		= 0,
-	.class			= 0x010000,
-	.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
-	.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_BLK,
-	.bar[0]			= IOPORT_VIRTIO_BLK | PCI_BASE_ADDRESS_SPACE_IO,
-	.irq_pin		= VIRTIO_BLK_PIN,
-	.irq_line		= VIRTIO_BLK_IRQ,
-};
+static int virtio_blk_find_empty_dev(void)
+{
+	int i;
 
-#define PCI_VIRTIO_BLK_DEVNUM 1
+	for (i = 0; i < VIRTIO_BLK_MAX_DEV; i++) {
+		if (blk_devices[i] == NULL)
+			return i;
+	}
+
+	return -1;
+}
 
 void virtio_blk__init(struct kvm *self, struct disk_image *disk)
 {
+	int new_dev_idx;
+	u16 blk_dev_base_addr;
+	struct blk_device *blk_device;
+
 	if (!disk)
 		return;
 
-	blk_device.disk = disk;
-
-	blk_device.blk_config.capacity = disk->size / SECTOR_SIZE;
+	new_dev_idx = virtio_blk_find_empty_dev();
+	if (new_dev_idx < 0)
+		die("Could not find an empty block device slot");
+
+	blk_devices[new_dev_idx] = calloc(1, sizeof(struct blk_device));
+	if (blk_devices[new_dev_idx] == NULL)
+		die("Failed allocating blk_device");
+
+	blk_device = blk_devices[new_dev_idx];
+	blk_dev_base_addr = IOPORT_VIRTIO_BLK + new_dev_idx * IOPORT_VIRTIO_BLK_SIZE;
+
+	*blk_device = (struct blk_device) {
+		.mutex			= PTHREAD_MUTEX_INITIALIZER,
+		.disk			= disk,
+		.idx			= new_dev_idx,
+		.blk_config		= (struct virtio_blk_config) {
+			.capacity	= disk->size / SECTOR_SIZE,
+		},
+		.pci_device = (struct pci_device_header) {
+			.vendor_id		= PCI_VENDOR_ID_REDHAT_QUMRANET,
+			.device_id		= PCI_DEVICE_ID_VIRTIO_BLK,
+			.header_type		= PCI_HEADER_TYPE_NORMAL,
+			.revision_id		= 0,
+			.class			= 0x010000,
+			.subsys_vendor_id	= PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET,
+			.subsys_id		= PCI_SUBSYSTEM_ID_VIRTIO_BLK,
+			.bar[0]			= blk_dev_base_addr | PCI_BASE_ADDRESS_SPACE_IO,
+			.irq_pin		= VIRTIO_BLK_PIN,
+			.irq_line		= VIRTIO_BLK_IRQ + new_dev_idx,
+		},
+	};
 
-	pci__register(&virtio_blk_pci_device, PCI_VIRTIO_BLK_DEVNUM);
+	pci__register(&blk_device->pci_device, PCI_VIRTIO_BLK_DEVNUM + new_dev_idx);
 
-	ioport__register(IOPORT_VIRTIO_BLK, &virtio_blk_io_ops, IOPORT_VIRTIO_BLK_SIZE);
+	ioport__register(blk_dev_base_addr, &virtio_blk_io_ops, IOPORT_VIRTIO_BLK_SIZE);
 }
-- 
1.7.5.rc3

Re: [PATCH 2/3 V2] kvm tools: Add support for multiple virtio-blk

[Ingo Molnar]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> Add support for multiple blk_devices by un-globalizing
> the current blk_device and allow multiple blk_devices.

Very nice!

One nit:

> +	mpc_intsrc		= last_addr;
> +	mpc_intsrc->type	= MP_INTSRC;
> +	mpc_intsrc->irqtype	= mp_INT;
> +	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
> +	mpc_intsrc->srcbus	= pcibusid;
> +	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
> +	mpc_intsrc->dstapic	= ioapicid;
> +	mpc_intsrc->dstirq	= 10; /* VIRTIO_BLK_IRQ */
> +
> +	last_addr = (void *)&mpc_intsrc[1];
> +	nentries++;
> +
> +	mpc_intsrc		= last_addr;
> +	mpc_intsrc->type	= MP_INTSRC;
> +	mpc_intsrc->irqtype	= mp_INT;
> +	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
> +	mpc_intsrc->srcbus	= pcibusid;
> +	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
> +	mpc_intsrc->dstapic	= ioapicid;
> +	mpc_intsrc->dstirq	= 11; /* VIRTIO_BLK_IRQ */
> +
> +	last_addr = (void *)&mpc_intsrc[1];
> +	nentries++;
> +
> +	mpc_intsrc		= last_addr;
> +	mpc_intsrc->type	= MP_INTSRC;
> +	mpc_intsrc->irqtype	= mp_INT;
> +	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
> +	mpc_intsrc->srcbus	= pcibusid;
> +	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
> +	mpc_intsrc->dstapic	= ioapicid;
> +	mpc_intsrc->dstirq	= 12; /* VIRTIO_BLK_IRQ */

There should really be a helper function for these initializations - and a loop 
that creates VIRTIO_BLK_MAX_DEV of them, right?

Also, the IRQs used by kvm should be enumerated in an include file in a single 
place, with ranges allocated for specific purposes, otherwise we'll quickly 
lose track of them.

And a pet peeve of mine:

> +struct blk_device_job {
> +	struct virt_queue	*vq;
> +	struct blk_device	*blk_device;
> +	void			*job_id;
> +};
> +
>  struct blk_device {
>  	pthread_mutex_t			mutex;
>  
>  	struct virtio_blk_config	blk_config;
> -	struct disk_image			*disk;
> +	struct disk_image		*disk;
>  	uint32_t			host_features;
>  	uint32_t			guest_features;
>  	uint16_t			config_vector;
>  	uint8_t				status;
> +	u8				idx;

The vertical spacing of those two structures (blk_device_job and blk_device) is 
inconsistent.

>  static bool virtio_blk_pci_io_in(struct kvm *self, uint16_t port, void *data, int size, uint32_t count)
>  {
> -	unsigned long offset;
> +	u16 offset, dev_idx;
>  	bool ret = true;
> +	struct blk_device *blk_device;

Suggestion: please standardize on shorter but still obvious variable names. For 
blk_device a good one could be 'bdev'.

Given how frequently it's used you might even abbreviate 'struct blk_device' to 
'struct blk_dev' - even in that shortr form it's still very obvious what it 
means.

The new, standardized and streamlined naming convention should be propagated to 
all 'struct blk_device' using functions. (in separate patches)

Thanks,

	Ingo

Re: [PATCH 2/3 V2] kvm tools: Add support for multiple virtio-blk

[Avi Kivity]

On 05/04/2011 04:56 PM, Ingo Molnar wrote:
> Also, the IRQs used by kvm should be enumerated in an include file in a single
> place, with ranges allocated for specific purposes, otherwise we'll quickly
> lose track of them.

You'll also quickly run out of interrupts.  To get lots of devices you 
can use level-triggered interrupts (this is really how the device is 
meant to be used, but is slow) or switch to MSI-X (fast but 
significantly more code needed).


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 2/3 V2] kvm tools: Add support for multiple virtio-blk

[Asias He]

On 05/04/2011 10:32 PM, Avi Kivity wrote:
> On 05/04/2011 04:56 PM, Ingo Molnar wrote:
>> Also, the IRQs used by kvm should be enumerated in an include file in
>> a single
>> place, with ranges allocated for specific purposes, otherwise we'll
>> quickly
>> lose track of them.
> 
> You'll also quickly run out of interrupts.  To get lots of devices you
> can use level-triggered interrupts (this is really how the device is
> meant to be used, but is slow) or switch to MSI-X (fast but
> significantly more code needed).

Sure, we will switch to MSI-X soon.

-- 
Best Regards,
Asias He

Re: [PATCH 2/3 V2] kvm tools: Add support for multiple virtio-blk

[Sasha Levin]

On Wed, 2011-05-04 at 15:56 +0200, Ingo Molnar wrote:
> * Sasha Levin <levinsasha928@gmail.com> wrote:
> 
> > Add support for multiple blk_devices by un-globalizing
> > the current blk_device and allow multiple blk_devices.
> 
> Very nice!
> 
> One nit:
> 
> > +	mpc_intsrc		= last_addr;
> > +	mpc_intsrc->type	= MP_INTSRC;
> > +	mpc_intsrc->irqtype	= mp_INT;
> > +	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
> > +	mpc_intsrc->srcbus	= pcibusid;
> > +	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
> > +	mpc_intsrc->dstapic	= ioapicid;
> > +	mpc_intsrc->dstirq	= 10; /* VIRTIO_BLK_IRQ */
> > +
> > +	last_addr = (void *)&mpc_intsrc[1];
> > +	nentries++;
> > +
> > +	mpc_intsrc		= last_addr;
> > +	mpc_intsrc->type	= MP_INTSRC;
> > +	mpc_intsrc->irqtype	= mp_INT;
> > +	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
> > +	mpc_intsrc->srcbus	= pcibusid;
> > +	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
> > +	mpc_intsrc->dstapic	= ioapicid;
> > +	mpc_intsrc->dstirq	= 11; /* VIRTIO_BLK_IRQ */
> > +
> > +	last_addr = (void *)&mpc_intsrc[1];
> > +	nentries++;
> > +
> > +	mpc_intsrc		= last_addr;
> > +	mpc_intsrc->type	= MP_INTSRC;
> > +	mpc_intsrc->irqtype	= mp_INT;
> > +	mpc_intsrc->irqflag	= MP_IRQDIR_DEFAULT;
> > +	mpc_intsrc->srcbus	= pcibusid;
> > +	mpc_intsrc->srcbusirq	= 1; /* virtio block irq pin */
> > +	mpc_intsrc->dstapic	= ioapicid;
> > +	mpc_intsrc->dstirq	= 12; /* VIRTIO_BLK_IRQ */
> 
> There should really be a helper function for these initializations - and a loop 
> that creates VIRTIO_BLK_MAX_DEV of them, right?
> 
> Also, the IRQs used by kvm should be enumerated in an include file in a single 
> place, with ranges allocated for specific purposes, otherwise we'll quickly 
> lose track of them.
> 

The plan is that mptable.c is going to change either way, Cyrill noted
we need a way to manage IRQs earlier in that file. That's also why I
tried not changing it too much.

> And a pet peeve of mine:
> 
> > +struct blk_device_job {
> > +	struct virt_queue	*vq;
> > +	struct blk_device	*blk_device;
> > +	void			*job_id;
> > +};
> > +
> >  struct blk_device {
> >  	pthread_mutex_t			mutex;
> >  
> >  	struct virtio_blk_config	blk_config;
> > -	struct disk_image			*disk;
> > +	struct disk_image		*disk;
> >  	uint32_t			host_features;
> >  	uint32_t			guest_features;
> >  	uint16_t			config_vector;
> >  	uint8_t				status;
> > +	u8				idx;
> 
> The vertical spacing of those two structures (blk_device_job and blk_device) is 
> inconsistent.
> 
Will fix.

> >  static bool virtio_blk_pci_io_in(struct kvm *self, uint16_t port, void *data, int size, uint32_t count)
> >  {
> > -	unsigned long offset;
> > +	u16 offset, dev_idx;
> >  	bool ret = true;
> > +	struct blk_device *blk_device;
> 
> Suggestion: please standardize on shorter but still obvious variable names. For 
> blk_device a good one could be 'bdev'.
> 
> Given how frequently it's used you might even abbreviate 'struct blk_device' to 
> 'struct blk_dev' - even in that shortr form it's still very obvious what it 
> means.
> 
> The new, standardized and streamlined naming convention should be propagated to 
> all 'struct blk_device' using functions. (in separate patches)

Will fix.

-- 

Sasha.

Re: [PATCH 2/3 V2] kvm tools: Add support for multiple virtio-blk

[Cyrill Gorcunov]

On 05/04/2011 06:44 PM, Sasha Levin wrote:
...
>>
>> There should really be a helper function for these initializations - and a loop 
>> that creates VIRTIO_BLK_MAX_DEV of them, right?
>>
>> Also, the IRQs used by kvm should be enumerated in an include file in a single 
>> place, with ranges allocated for specific purposes, otherwise we'll quickly 
>> lose track of them.
>>
> 
> The plan is that mptable.c is going to change either way, Cyrill noted
> we need a way to manage IRQs earlier in that file. That's also why I
> tried not changing it too much.
> 
...

  Sasha, I simply out of time, sorry guys. So if you can -- move irqs into
separate file and make loop for block devices this would be great I guess.

  We need a global collector/manager for pins/irqs so caller would do something
like

	irq_alloc(struct irq_cfg *req)

where

	struct irq_cfg {
		int flags;
		int vector;
		int pin;
	}

or something like that. I don't have a clear picture yet.

  Ingo, on the other hands, maybe open-coded snippets would be fine for
a while?

-- 
Thanks,
  Cyrill

[PATCH 3/3 V2] kvm tools: Add cmdline options for loading multiple images

[Sasha Levin]

This is a simple cmdline addition to allow loading multiple images.
perf's cmdline parser doesn't support having multiple args
with the same name (i.e. --image <img1> --image <img2>), so
we have to choose either to extend the parser, or find a diiferent
way to assign multiple images.

Sample cmdline for loading 2 images:
./kvm run --image=image1.raw --readonly --image2=image2.raw --readonly2

NOTE NOTE NOTE - If testing, Please take notice of the
--readonly param changes so you won't destroy your image/disk.

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

diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index b92232d..5182735 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -42,6 +42,7 @@
 #define MB_SHIFT		(20)
 #define MIN_RAM_SIZE_MB		(64ULL)
 #define MIN_RAM_SIZE_BYTE	(MIN_RAM_SIZE_MB << MB_SHIFT)
+#define MAX_DISK_IMAGES		4
 
 static struct kvm *kvm;
 static struct kvm_cpu *kvm_cpus[KVM_NR_CPUS];
@@ -51,7 +52,7 @@ static u64 ram_size = MIN_RAM_SIZE_MB;
 static const char *kernel_cmdline;
 static const char *kernel_filename;
 static const char *initrd_filename;
-static const char *image_filename;
+static const char *image_filename[MAX_DISK_IMAGES];
 static const char *console;
 static const char *kvm_dev;
 static const char *network;
@@ -59,7 +60,7 @@ static const char *host_ip_addr;
 static const char *guest_mac;
 static const char *script;
 static bool single_step;
-static bool readonly_image;
+static bool readonly_image[MAX_DISK_IMAGES];
 static bool virtio_rng;
 extern bool ioport_debug;
 extern int  active_console;
@@ -75,8 +76,17 @@ static const struct option options[] = {
 	OPT_GROUP("Basic options:"),
 	OPT_INTEGER('\0', "cpus", &nrcpus, "Number of CPUs"),
 	OPT_U64('m', "mem", &ram_size, "Virtual machine memory size in MiB."),
-	OPT_STRING('i', "image", &image_filename, "image", "Disk image"),
-	OPT_BOOLEAN('\0', "readonly", &readonly_image,
+	OPT_STRING('i', "image", &image_filename[0], "image", "Disk image"),
+	OPT_BOOLEAN('\0', "readonly", &readonly_image[0],
+			"Don't write changes back to disk image"),
+	OPT_STRING('\0', "image2", &image_filename[1], "image", "Disk image"),
+	OPT_BOOLEAN('\0', "readonly2", &readonly_image[1],
+			"Don't write changes back to disk image"),
+	OPT_STRING('\0', "image3", &image_filename[2], "image", "Disk image"),
+	OPT_BOOLEAN('\0', "readonly3", &readonly_image[2],
+			"Don't write changes back to disk image"),
+	OPT_STRING('\0', "image4", &image_filename[3], "image", "Disk image"),
+	OPT_BOOLEAN('\0', "readonly4", &readonly_image[3],
 			"Don't write changes back to disk image"),
 	OPT_STRING('c', "console", &console, "serial or virtio",
 			"Console to use"),
@@ -397,23 +407,25 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 		strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
 
 	hi = NULL;
-	if (!image_filename) {
+	if (!image_filename[0]) {
 		hi = host_image(real_cmdline, sizeof(real_cmdline));
 		if (hi) {
-			image_filename = hi;
-			readonly_image = true;
+			image_filename[0] = hi;
+			readonly_image[0] = true;
 		}
 	}
 
 	if (!strstr(real_cmdline, "root="))
 		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
 
-	if (image_filename) {
-		struct disk_image *disk = disk_image__open(image_filename, readonly_image);
-		if (!disk)
-			die("unable to load disk image %s", image_filename);
+	for (i = 0; i < MAX_DISK_IMAGES; i++) {
+		if (image_filename[i]) {
+			struct disk_image *disk = disk_image__open(image_filename[i], readonly_image[i]);
+			if (!disk)
+				die("unable to load disk image %s", image_filename[i]);
 
-		virtio_blk__init(kvm, disk);
+			virtio_blk__init(kvm, disk);
+		}
 	}
 	free(hi);
 
-- 
1.7.5.rc3

Re: [PATCH 3/3 V2] kvm tools: Add cmdline options for loading multiple images

[David Ahern]

On 05/04/11 07:45, Sasha Levin wrote:
> This is a simple cmdline addition to allow loading multiple images.
> perf's cmdline parser doesn't support having multiple args
> with the same name (i.e. --image <img1> --image <img2>), so
> we have to choose either to extend the parser, or find a diiferent
> way to assign multiple images.
> 
> Sample cmdline for loading 2 images:
> ./kvm run --image=image1.raw --readonly --image2=image2.raw --readonly2

syntax is getting a bit unwieldy. Why not use a scheme similar to qemu
and concatenate related arguments into one and handle multiple usages?
e.g., --image=image1.raw,ro --image=image2.raw,ro

David


> 
> NOTE NOTE NOTE - If testing, Please take notice of the
> --readonly param changes so you won't destroy your image/disk.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  tools/kvm/kvm-run.c |   36 ++++++++++++++++++++++++------------
>  1 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
> index b92232d..5182735 100644
> --- a/tools/kvm/kvm-run.c
> +++ b/tools/kvm/kvm-run.c
> @@ -42,6 +42,7 @@
>  #define MB_SHIFT		(20)
>  #define MIN_RAM_SIZE_MB		(64ULL)
>  #define MIN_RAM_SIZE_BYTE	(MIN_RAM_SIZE_MB << MB_SHIFT)
> +#define MAX_DISK_IMAGES		4
>  
>  static struct kvm *kvm;
>  static struct kvm_cpu *kvm_cpus[KVM_NR_CPUS];
> @@ -51,7 +52,7 @@ static u64 ram_size = MIN_RAM_SIZE_MB;
>  static const char *kernel_cmdline;
>  static const char *kernel_filename;
>  static const char *initrd_filename;
> -static const char *image_filename;
> +static const char *image_filename[MAX_DISK_IMAGES];
>  static const char *console;
>  static const char *kvm_dev;
>  static const char *network;
> @@ -59,7 +60,7 @@ static const char *host_ip_addr;
>  static const char *guest_mac;
>  static const char *script;
>  static bool single_step;
> -static bool readonly_image;
> +static bool readonly_image[MAX_DISK_IMAGES];
>  static bool virtio_rng;
>  extern bool ioport_debug;
>  extern int  active_console;
> @@ -75,8 +76,17 @@ static const struct option options[] = {
>  	OPT_GROUP("Basic options:"),
>  	OPT_INTEGER('\0', "cpus", &nrcpus, "Number of CPUs"),
>  	OPT_U64('m', "mem", &ram_size, "Virtual machine memory size in MiB."),
> -	OPT_STRING('i', "image", &image_filename, "image", "Disk image"),
> -	OPT_BOOLEAN('\0', "readonly", &readonly_image,
> +	OPT_STRING('i', "image", &image_filename[0], "image", "Disk image"),
> +	OPT_BOOLEAN('\0', "readonly", &readonly_image[0],
> +			"Don't write changes back to disk image"),
> +	OPT_STRING('\0', "image2", &image_filename[1], "image", "Disk image"),
> +	OPT_BOOLEAN('\0', "readonly2", &readonly_image[1],
> +			"Don't write changes back to disk image"),
> +	OPT_STRING('\0', "image3", &image_filename[2], "image", "Disk image"),
> +	OPT_BOOLEAN('\0', "readonly3", &readonly_image[2],
> +			"Don't write changes back to disk image"),
> +	OPT_STRING('\0', "image4", &image_filename[3], "image", "Disk image"),
> +	OPT_BOOLEAN('\0', "readonly4", &readonly_image[3],
>  			"Don't write changes back to disk image"),
>  	OPT_STRING('c', "console", &console, "serial or virtio",
>  			"Console to use"),
> @@ -397,23 +407,25 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>  		strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
>  
>  	hi = NULL;
> -	if (!image_filename) {
> +	if (!image_filename[0]) {
>  		hi = host_image(real_cmdline, sizeof(real_cmdline));
>  		if (hi) {
> -			image_filename = hi;
> -			readonly_image = true;
> +			image_filename[0] = hi;
> +			readonly_image[0] = true;
>  		}
>  	}
>  
>  	if (!strstr(real_cmdline, "root="))
>  		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
>  
> -	if (image_filename) {
> -		struct disk_image *disk = disk_image__open(image_filename, readonly_image);
> -		if (!disk)
> -			die("unable to load disk image %s", image_filename);
> +	for (i = 0; i < MAX_DISK_IMAGES; i++) {
> +		if (image_filename[i]) {
> +			struct disk_image *disk = disk_image__open(image_filename[i], readonly_image[i]);
> +			if (!disk)
> +				die("unable to load disk image %s", image_filename[i]);
>  
> -		virtio_blk__init(kvm, disk);
> +			virtio_blk__init(kvm, disk);
> +		}
>  	}
>  	free(hi);
>  

Re: [PATCH 3/3 V2] kvm tools: Add cmdline options for loading multiple images

[Sasha Levin]

On Wed, 2011-05-04 at 08:51 -0600, David Ahern wrote:
> 
> On 05/04/11 07:45, Sasha Levin wrote:
> > This is a simple cmdline addition to allow loading multiple images.
> > perf's cmdline parser doesn't support having multiple args
> > with the same name (i.e. --image <img1> --image <img2>), so
> > we have to choose either to extend the parser, or find a diiferent
> > way to assign multiple images.
> > 
> > Sample cmdline for loading 2 images:
> > ./kvm run --image=image1.raw --readonly --image2=image2.raw --readonly2
> 
> syntax is getting a bit unwieldy. Why not use a scheme similar to qemu
> and concatenate related arguments into one and handle multiple usages?
> e.g., --image=image1.raw,ro --image=image2.raw,ro

That'll probably how it'll end up being. Currently the cmdline parser
doesn't support it and I didn't want to mix parser changes with
virtio-blk patch.

This patch should just allow testing of the multiple virtio-blk feature.

-- 

Sasha.

Re: [PATCH 3/3 V2] kvm tools: Add cmdline options for loading multiple images

[David Ahern]

On 05/04/11 09:03, Sasha Levin wrote:
> On Wed, 2011-05-04 at 08:51 -0600, David Ahern wrote:
>>
>> On 05/04/11 07:45, Sasha Levin wrote:
>>> This is a simple cmdline addition to allow loading multiple images.
>>> perf's cmdline parser doesn't support having multiple args
>>> with the same name (i.e. --image <img1> --image <img2>), so
>>> we have to choose either to extend the parser, or find a diiferent
>>> way to assign multiple images.
>>>
>>> Sample cmdline for loading 2 images:
>>> ./kvm run --image=image1.raw --readonly --image2=image2.raw --readonly2
>>
>> syntax is getting a bit unwieldy. Why not use a scheme similar to qemu
>> and concatenate related arguments into one and handle multiple usages?
>> e.g., --image=image1.raw,ro --image=image2.raw,ro
> 
> That'll probably how it'll end up being. Currently the cmdline parser
> doesn't support it and I didn't want to mix parser changes with
> virtio-blk patch.

It's using the option parser from perf, so you need to change:

OPT_STRING('i', "image", &image_filename, "image", "Disk image"),

to OPT_CALLBACK and create a parser function specific to this input
argument. For example, checkout tools/perf/builtin-script.c,
parse_output_fields.

The parser is specific to the image argument and hence should be a part
of this change set.

David


> 
> This patch should just allow testing of the multiple virtio-blk feature.
> 

Re: [PATCH 3/3 V2] kvm tools: Add cmdline options for loading multiple images

[Sasha Levin]

On Wed, 2011-05-04 at 09:33 -0600, David Ahern wrote:
> On 05/04/11 09:03, Sasha Levin wrote:
> > On Wed, 2011-05-04 at 08:51 -0600, David Ahern wrote:
> >>
> >> On 05/04/11 07:45, Sasha Levin wrote:
> >>> This is a simple cmdline addition to allow loading multiple images.
> >>> perf's cmdline parser doesn't support having multiple args
> >>> with the same name (i.e. --image <img1> --image <img2>), so
> >>> we have to choose either to extend the parser, or find a diiferent
> >>> way to assign multiple images.
> >>>
> >>> Sample cmdline for loading 2 images:
> >>> ./kvm run --image=image1.raw --readonly --image2=image2.raw --readonly2
> >>
> >> syntax is getting a bit unwieldy. Why not use a scheme similar to qemu
> >> and concatenate related arguments into one and handle multiple usages?
> >> e.g., --image=image1.raw,ro --image=image2.raw,ro
> > 
> > That'll probably how it'll end up being. Currently the cmdline parser
> > doesn't support it and I didn't want to mix parser changes with
> > virtio-blk patch.
> 
> It's using the option parser from perf, so you need to change:
> 
> OPT_STRING('i', "image", &image_filename, "image", "Disk image"),
> 
> to OPT_CALLBACK and create a parser function specific to this input
> argument. For example, checkout tools/perf/builtin-script.c,
> parse_output_fields.
> 
> The parser is specific to the image argument and hence should be a part
> of this change set.
> 
> David

Thanks!

-- 

Sasha.