diff mbox

[2/2] kvm tools: Add scatter-gather support for disk images

Message ID 1302954344-11700-2-git-send-email-levinsasha928@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sasha Levin April 16, 2011, 11:45 a.m. UTC 
Add optional support for scatter-gather to disk_image.
Formats that can't take advantage of scatter-gather fallback to simple IO.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/disk-image.c             |   22 ++++++++++++++++++++
 tools/kvm/include/kvm/disk-image.h |   31 ++++++++++++++++++++++++++++
 tools/kvm/virtio-blk.c             |   39 +++++------------------------------
 3 files changed, 59 insertions(+), 33 deletions(-)

Comments

Pekka Enberg April 16, 2011, 1:55 p.m. UTC | #1 
On Sat, 16 Apr 2011, Sasha Levin wrote:
> Add optional support for scatter-gather to disk_image.
> Formats that can't take advantage of scatter-gather fallback to simple IO.

Cool!

> +static size_t raw_image__read_sector_sg(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount)
> +{
> +	uint64_t offset = sector << SECTOR_SHIFT;
> +	size_t ret = -1;
> +
> +	ret = preadv(self->fd, iov, iovcount, offset);
> +
> +	return ret;

Please just do

   return preadv(...);

Also, I guess you need preadv_in_full() similar to read_in_full() that 
handles EINTR and EAGAIN.

> +}
> +
> +static size_t raw_image__write_sector_sg(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount)
> +{
> +	uint64_t offset = sector << SECTOR_SHIFT;
> +	size_t ret = -1;
> +
> +	ret = pwritev(self->fd, iov, iovcount, offset);
> +
> +	return ret;

Same comments apply here as well.

> @@ -115,50 +115,23 @@ static bool virtio_blk_do_io_request(struct kvm *self, struct virt_queue *queue)
> {
> 	struct iovec iov[VIRTIO_BLK_QUEUE_SIZE];
> 	struct virtio_blk_outhdr *req;
> -	uint32_t block_len, block_cnt;
> +	size_t block_cnt = -1;
> 	uint16_t out, in, head;
> 	uint8_t *status;
> -	bool io_error;
> -	void *block;
> -	int err, i;
> -
> -	io_error	= false;
>
> 	head		= virt_queue__get_iov(queue, iov, &out, &in, self);
>
> 	/* head */
> 	req		= iov[0].iov_base;
>
> -	/* block */
> -	block_cnt	= 0;
> -
> -	for (i = 1; i < out + in - 1; i++) {
> -		block		= iov[i].iov_base;
> -		block_len	= iov[i].iov_len;
> -
> -		switch (req->type) {
> -		case VIRTIO_BLK_T_IN:
> -			err	= disk_image__read_sector(self->disk_image, req->sector, block, block_len);
> -			if (err)
> -				io_error = true;
> -			break;
> -		case VIRTIO_BLK_T_OUT:
> -			err	= disk_image__write_sector(self->disk_image, req->sector, block, block_len);
> -			if (err)
> -				io_error = true;
> -			break;
> -		default:
> -			warning("request type %d", req->type);
> -			io_error	= true;
> -		}
> -
> -		req->sector	+= block_len >> SECTOR_SHIFT;
> -		block_cnt	+= block_len;
> -	}
> +	if (req->type == VIRTIO_BLK_T_IN)
> +		block_cnt = disk_image__read_sector_sg(self->disk_image, req->sector, iov + 1, in + out - 2);
> +	else if (req->type == VIRTIO_BLK_T_OUT)
> +		block_cnt = disk_image__write_sector_sg(self->disk_image, req->sector, iov + 1, in + out - 2);

Please us switch statement like in the above loop instead of if-else and 
retain the warning.

>
> 	/* status */
> 	status			= iov[out + in - 1].iov_base;
> -	*status			= io_error ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
> +	*status			= (block_cnt == (size_t)-1) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
>
> 	virt_queue__set_used_elem(queue, head, block_cnt);
>
> -- 
> 1.7.5.rc1
>
>
--
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
diff mbox

Patch

diff --git a/tools/kvm/disk-image.c b/tools/kvm/disk-image.c
index 4198ebb..dd54b99 100644
--- a/tools/kvm/disk-image.c
+++ b/tools/kvm/disk-image.c
@@ -72,6 +72,26 @@  static int raw_image__write_sector(struct disk_image *self, uint64_t sector, voi
 	return 0;
 }
 
+static size_t raw_image__read_sector_sg(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount)
+{
+	uint64_t offset = sector << SECTOR_SHIFT;
+	size_t ret = -1;
+
+	ret = preadv(self->fd, iov, iovcount, offset);
+
+	return ret;
+}
+
+static size_t raw_image__write_sector_sg(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount)
+{
+	uint64_t offset = sector << SECTOR_SHIFT;
+	size_t ret = -1;
+
+	ret = pwritev(self->fd, iov, iovcount, offset);
+
+	return ret;
+}
+
 static int raw_image__read_sector_ro_mmap(struct disk_image *self, uint64_t sector, void *dst, uint32_t dst_len)
 {
 	uint64_t offset = sector << SECTOR_SHIFT;
@@ -105,6 +125,8 @@  static void raw_image__close_ro_mmap(struct disk_image *self)
 static struct disk_image_operations raw_image_ops = {
 	.read_sector		= raw_image__read_sector,
 	.write_sector		= raw_image__write_sector,
+	.read_sector_sg		= raw_image__read_sector_sg,
+	.write_sector_sg	= raw_image__write_sector_sg
 };
 
 static struct disk_image_operations raw_image_ro_mmap_ops = {
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index 33962c6..a4a96a2 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -3,6 +3,7 @@ 
 
 #include <stdint.h>
 #include <stdbool.h>
+#include <sys/uio.h>
 
 #define SECTOR_SHIFT		9
 #define SECTOR_SIZE		(1UL << SECTOR_SHIFT)
@@ -12,6 +13,8 @@  struct disk_image;
 struct disk_image_operations {
 	int (*read_sector)(struct disk_image *self, uint64_t sector, void *dst, uint32_t dst_len);
 	int (*write_sector)(struct disk_image *self, uint64_t sector, void *src, uint32_t src_len);
+	size_t (*read_sector_sg)(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount);
+	size_t (*write_sector_sg)(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount);
 	void (*close)(struct disk_image *self);
 };
 
@@ -37,4 +40,32 @@  static inline int disk_image__write_sector(struct disk_image *self, uint64_t sec
 	return self->ops->write_sector(self, sector, src, src_len);
 }
 
+static inline size_t disk_image__read_sector_sg(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount)
+{
+	if (self->ops->read_sector_sg)
+		return self->ops->read_sector_sg(self, sector, iov, iovcount);
+
+	while (iovcount--) {
+		self->ops->read_sector(self, sector, iov->iov_base, iov->iov_len);
+		sector += iov->iov_len >> SECTOR_SHIFT;
+		iov++;
+	}
+
+	return sector << SECTOR_SHIFT;
+}
+
+static inline size_t disk_image__write_sector_sg(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount)
+{
+	if (self->ops->write_sector_sg)
+		return self->ops->write_sector_sg(self, sector, iov, iovcount);
+
+	while (iovcount--) {
+		self->ops->write_sector(self, sector, iov->iov_base, iov->iov_len);
+		sector += iov->iov_len >> SECTOR_SHIFT;
+		iov++;
+	}
+
+	return sector << SECTOR_SHIFT;
+}
+
 #endif /* KVM__DISK_IMAGE_H */
diff --git a/tools/kvm/virtio-blk.c b/tools/kvm/virtio-blk.c
index cb344d08..aee3b73 100644
--- a/tools/kvm/virtio-blk.c
+++ b/tools/kvm/virtio-blk.c
@@ -115,50 +115,23 @@  static bool virtio_blk_do_io_request(struct kvm *self, struct virt_queue *queue)
 {
 	struct iovec iov[VIRTIO_BLK_QUEUE_SIZE];
 	struct virtio_blk_outhdr *req;
-	uint32_t block_len, block_cnt;
+	size_t block_cnt = -1;
 	uint16_t out, in, head;
 	uint8_t *status;
-	bool io_error;
-	void *block;
-	int err, i;
-
-	io_error	= false;
 
 	head		= virt_queue__get_iov(queue, iov, &out, &in, self);
 
 	/* head */
 	req		= iov[0].iov_base;
 
-	/* block */
-	block_cnt	= 0;
-
-	for (i = 1; i < out + in - 1; i++) {
-		block		= iov[i].iov_base;
-		block_len	= iov[i].iov_len;
-
-		switch (req->type) {
-		case VIRTIO_BLK_T_IN:
-			err	= disk_image__read_sector(self->disk_image, req->sector, block, block_len);
-			if (err)
-				io_error = true;
-			break;
-		case VIRTIO_BLK_T_OUT:
-			err	= disk_image__write_sector(self->disk_image, req->sector, block, block_len);
-			if (err)
-				io_error = true;
-			break;
-		default:
-			warning("request type %d", req->type);
-			io_error	= true;
-		}
-
-		req->sector	+= block_len >> SECTOR_SHIFT;
-		block_cnt	+= block_len;
-	}
+	if (req->type == VIRTIO_BLK_T_IN)
+		block_cnt = disk_image__read_sector_sg(self->disk_image, req->sector, iov + 1, in + out - 2);
+	else if (req->type == VIRTIO_BLK_T_OUT)
+		block_cnt = disk_image__write_sector_sg(self->disk_image, req->sector, iov + 1, in + out - 2);
 
 	/* status */
 	status			= iov[out + in - 1].iov_base;
-	*status			= io_error ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
+	*status			= (block_cnt == (size_t)-1) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
 
 	virt_queue__set_used_elem(queue, head, block_cnt);