[PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Sasha Levin]

Added scatter-gather variants of [x,xp][read,write]() and [p]read_in_full().

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/read-write.h |   13 +++
 tools/kvm/read-write.c             |  172 ++++++++++++++++++++++++++++++++++++
 2 files changed, 185 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/include/kvm/read-write.h b/tools/kvm/include/kvm/read-write.h
index 069326b..3351103 100644
--- a/tools/kvm/include/kvm/read-write.h
+++ b/tools/kvm/include/kvm/read-write.h
@@ -2,6 +2,7 @@
 #define KVM_READ_WRITE_H
 
 #include <sys/types.h>
+#include <sys/uio.h>
 #include <unistd.h>
 
 ssize_t xread(int fd, void *buf, size_t count);
@@ -16,4 +17,16 @@ ssize_t xpwrite(int fd, const void *buf, size_t count, off_t offset);
 ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
 ssize_t pwrite_in_full(int fd, const void *buf, size_t count, off_t offset);
 
+ssize_t xreadv(int fd, const struct iovec *iov, int iovcnt);
+ssize_t xwritev(int fd, const struct iovec *iov, int iovcnt);
+
+ssize_t readv_in_full(int fd, const struct iovec *iov, int iovcnt);
+ssize_t writev_in_full(int fd, const struct iovec *iov, int iovcnt);
+
+ssize_t xpreadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+ssize_t xpwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+
+ssize_t preadv_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+
 #endif /* KVM_READ_WRITE_H */
diff --git a/tools/kvm/read-write.c b/tools/kvm/read-write.c
index 398918b..449e8a2 100644
--- a/tools/kvm/read-write.c
+++ b/tools/kvm/read-write.c
@@ -152,3 +152,175 @@ ssize_t pwrite_in_full(int fd, const void *buf, size_t count, off_t offset)
 
 	return total;
 }
+
+/* Same as readv(2) except that this function never returns EAGAIN or EINTR. */
+ssize_t xreadv(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t nr;
+
+restart:
+	nr = readv(fd, iov, iovcnt);
+	if ((nr < 0) && ((errno == EAGAIN) || (errno == EINTR)))
+		goto restart;
+
+	return nr;
+}
+
+/* Same as writev(2) except that this function never returns EAGAIN or EINTR. */
+ssize_t xwritev(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t nr;
+
+restart:
+	nr = write(fd, iov, iovcnt);
+	if ((nr < 0) && ((errno == EAGAIN) || (errno == EINTR)))
+		goto restart;
+
+	return nr;
+}
+
+static inline ssize_t get_iov_size(const struct iovec *iov, int iovcnt)
+{
+	size_t size = 0;
+	while (iovcnt--)
+		size += (iov++)->iov_len;
+
+	return size;
+}
+
+ssize_t readv_in_full(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t total = 0;
+	ssize_t count = get_iov_size(iov, iovcnt);
+
+	while (count > 0) {
+		ssize_t nr;
+
+		nr = xreadv(fd, iov, iovcnt);
+		if (nr <= 0) {
+			if (total > 0)
+				return total;
+
+			return -1;
+		}
+
+		while ((size_t)nr >= iov->iov_len) {
+			nr -= iov->iov_len;
+			total += iov->iov_len;
+			count -= iov->iov_len;
+			iovcnt--;
+			iov++;
+		}
+	}
+
+	return total;
+}
+
+ssize_t writev_in_full(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t total = 0;
+	size_t count = get_iov_size(iov, iovcnt);
+
+	while (count > 0) {
+		ssize_t nr;
+
+		nr = xwritev(fd, iov, iovcnt);
+		if (nr < 0)
+			return -1;
+		if (nr == 0) {
+			errno = ENOSPC;
+			return -1;
+		}
+
+		while ((size_t)nr >= iov->iov_len) {
+			nr -= iov->iov_len;
+			total += iov->iov_len;
+			count -= iov->iov_len;
+			iovcnt--;
+			iov++;
+		}
+	}
+
+	return total;
+}
+
+/* Same as preadv(2) except that this function never returns EAGAIN or EINTR. */
+ssize_t xpreadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+{
+	ssize_t nr;
+
+restart:
+	nr = preadv(fd, iov, iovcnt, offset);
+	if ((nr < 0) && ((errno == EAGAIN) || (errno == EINTR)))
+		goto restart;
+
+	return nr;
+}
+
+/* Same as pwritev(2) except that this function never returns EAGAIN or EINTR. */
+ssize_t xpwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+{
+	ssize_t nr;
+
+restart:
+	nr = pwritev(fd, iov, iovcnt, offset);
+	if ((nr < 0) && ((errno == EAGAIN) || (errno == EINTR)))
+		goto restart;
+
+	return nr;
+}
+
+ssize_t preadv_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+{
+	ssize_t total = 0;
+	size_t count = get_iov_size(iov, iovcnt);
+
+	while (count > 0) {
+		ssize_t nr;
+
+		nr = xpreadv(fd, iov, iovcnt, offset);
+		if (nr <= 0) {
+			if (total > 0)
+				return total;
+
+			return -1;
+		}
+
+		while ((size_t)nr >= iov->iov_len) {
+			nr -= iov->iov_len;
+			total += iov->iov_len;
+			count -= iov->iov_len;
+			iovcnt--;
+			iov++;
+		}
+	}
+
+	return total;
+}
+
+ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+{
+	ssize_t total = 0;
+	size_t count = get_iov_size(iov, iovcnt);
+
+	while (count > 0) {
+		ssize_t nr;
+
+		nr = xpwritev(fd, iov, iovcnt, offset);
+		if (nr < 0)
+			return -1;
+		if (nr == 0) {
+			errno = ENOSPC;
+			return -1;
+		}
+		while ((size_t)nr >= iov->iov_len) {
+			nr -= iov->iov_len;
+			total += iov->iov_len;
+			count -= iov->iov_len;
+			iovcnt--;
+			iov++;
+		}
+	}
+
+	return total;
+}
-- 
1.7.5.rc1

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

[Sasha Levin]

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             |   16 ++++++++++++
 tools/kvm/include/kvm/disk-image.h |   31 +++++++++++++++++++++++
 tools/kvm/virtio-blk.c             |   47 +++++++++++------------------------
 3 files changed, 62 insertions(+), 32 deletions(-)

diff --git a/tools/kvm/disk-image.c b/tools/kvm/disk-image.c
index 4198ebb..3a93645 100644
--- a/tools/kvm/disk-image.c
+++ b/tools/kvm/disk-image.c
@@ -72,6 +72,20 @@ static int raw_image__write_sector(struct disk_image *self, uint64_t sector, voi
 	return 0;
 }
 
+static ssize_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;
+
+	return preadv_in_full(self->fd, iov, iovcount, offset);
+}
+
+static ssize_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;
+
+	return pwritev_in_full(self->fd, iov, iovcount, offset);
+}
+
 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 +119,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..a3ba360 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);
+	ssize_t (*read_sector_sg)(struct disk_image *self, uint64_t sector, const struct iovec *iov, int iovcount);
+	ssize_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 ssize_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 ssize_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..58c4be9 100644
--- a/tools/kvm/virtio-blk.c
+++ b/tools/kvm/virtio-blk.c
@@ -115,50 +115,33 @@ 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;
+	ssize_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;
+	switch (req->type) {
+	case VIRTIO_BLK_T_IN:
+		block_cnt = disk_image__read_sector_sg(self->disk_image, req->sector, iov + 1, in + out - 2);
+
+		break;
+	case VIRTIO_BLK_T_OUT:
+		block_cnt = disk_image__write_sector_sg(self->disk_image, req->sector, iov + 1, in + out - 2);
+
+		break;
+
+	default:
+		warning("request type %d", req->type);
+		block_cnt = -1;
 	}
 
 	/* status */
 	status			= iov[out + in - 1].iov_base;
-	*status			= io_error ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
+	*status			= (block_cnt < 0) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
 
 	virt_queue__set_used_elem(queue, head, block_cnt);
 
-- 
1.7.5.rc1

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Konstantin Khlebnikov]

one note below

Sasha Levin wrote:
> Added scatter-gather variants of [x,xp][read,write]() and [p]read_in_full().
>
> Signed-off-by: Sasha Levin<levinsasha928@gmail.com>
> ---
>   tools/kvm/include/kvm/read-write.h |   13 +++
>   tools/kvm/read-write.c             |  172 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 185 insertions(+), 0 deletions(-)
>
> diff --git a/tools/kvm/include/kvm/read-write.h b/tools/kvm/include/kvm/read-write.h
> index 069326b..3351103 100644
> --- a/tools/kvm/include/kvm/read-write.h
> +++ b/tools/kvm/include/kvm/read-write.h
> @@ -2,6 +2,7 @@
>   #define KVM_READ_WRITE_H
>
>   #include<sys/types.h>
> +#include<sys/uio.h>
>   #include<unistd.h>
>
>   ssize_t xread(int fd, void *buf, size_t count);
> @@ -16,4 +17,16 @@ ssize_t xpwrite(int fd, const void *buf, size_t count, off_t offset);
>   ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
>   ssize_t pwrite_in_full(int fd, const void *buf, size_t count, off_t offset);
>
> +ssize_t xreadv(int fd, const struct iovec *iov, int iovcnt);
> +ssize_t xwritev(int fd, const struct iovec *iov, int iovcnt);
> +
> +ssize_t readv_in_full(int fd, const struct iovec *iov, int iovcnt);
> +ssize_t writev_in_full(int fd, const struct iovec *iov, int iovcnt);
> +
> +ssize_t xpreadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
> +ssize_t xpwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
> +
> +ssize_t preadv_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset);
> +ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset);
> +
>   #endif /* KVM_READ_WRITE_H */
> diff --git a/tools/kvm/read-write.c b/tools/kvm/read-write.c
> index 398918b..449e8a2 100644
> --- a/tools/kvm/read-write.c
> +++ b/tools/kvm/read-write.c
> @@ -152,3 +152,175 @@ ssize_t pwrite_in_full(int fd, const void *buf, size_t count, off_t offset)
>
>   	return total;
>   }
> +
> +/* Same as readv(2) except that this function never returns EAGAIN or EINTR. */
> +ssize_t xreadv(int fd, const struct iovec *iov, int iovcnt)
> +{
> +	ssize_t nr;
> +
> +restart:
> +	nr = readv(fd, iov, iovcnt);
> +	if ((nr<  0)&&  ((errno == EAGAIN) || (errno == EINTR)))
> +		goto restart;
> +
> +	return nr;
> +}
> +
> +/* Same as writev(2) except that this function never returns EAGAIN or EINTR. */
> +ssize_t xwritev(int fd, const struct iovec *iov, int iovcnt)
> +{
> +	ssize_t nr;
> +
> +restart:
> +	nr = write(fd, iov, iovcnt);
> +	if ((nr<  0)&&  ((errno == EAGAIN) || (errno == EINTR)))
> +		goto restart;
> +
> +	return nr;
> +}
> +
> +static inline ssize_t get_iov_size(const struct iovec *iov, int iovcnt)
> +{
> +	size_t size = 0;
> +	while (iovcnt--)
> +		size += (iov++)->iov_len;
> +
> +	return size;
> +}
> +
> +ssize_t readv_in_full(int fd, const struct iovec *iov, int iovcnt)
> +{
> +	ssize_t total = 0;
> +	ssize_t count = get_iov_size(iov, iovcnt);
> +
> +	while (count>  0) {
> +		ssize_t nr;
> +
> +		nr = xreadv(fd, iov, iovcnt);
> +		if (nr<= 0) {
> +			if (total>  0)
> +				return total;
> +
> +			return -1;
> +		}
> +
> +		while ((size_t)nr>= iov->iov_len) {
> +			nr -= iov->iov_len;
> +			total += iov->iov_len;
> +			count -= iov->iov_len;
> +			iovcnt--;
> +			iov++;
> +		}
> +	}
> +
> +	return total;
> +}
> +
> +ssize_t writev_in_full(int fd, const struct iovec *iov, int iovcnt)
> +{
> +	ssize_t total = 0;
> +	size_t count = get_iov_size(iov, iovcnt);
> +
> +	while (count>  0) {
> +		ssize_t nr;
> +
> +		nr = xwritev(fd, iov, iovcnt);
> +		if (nr<  0)
> +			return -1;
> +		if (nr == 0) {
> +			errno = ENOSPC;
> +			return -1;
> +		}
> +
> +		while ((size_t)nr>= iov->iov_len) {
> +			nr -= iov->iov_len;
> +			total += iov->iov_len;
> +			count -= iov->iov_len;
> +			iovcnt--;
> +			iov++;
> +		}
> +	}
> +
> +	return total;
> +}
> +
> +/* Same as preadv(2) except that this function never returns EAGAIN or EINTR. */
> +ssize_t xpreadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
> +{
> +	ssize_t nr;
> +
> +restart:
> +	nr = preadv(fd, iov, iovcnt, offset);
> +	if ((nr<  0)&&  ((errno == EAGAIN) || (errno == EINTR)))
> +		goto restart;
> +
> +	return nr;
> +}
> +
> +/* Same as pwritev(2) except that this function never returns EAGAIN or EINTR. */
> +ssize_t xpwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
> +{
> +	ssize_t nr;
> +
> +restart:
> +	nr = pwritev(fd, iov, iovcnt, offset);
> +	if ((nr<  0)&&  ((errno == EAGAIN) || (errno == EINTR)))
> +		goto restart;
> +
> +	return nr;
> +}
> +
> +ssize_t preadv_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset)
> +{
> +	ssize_t total = 0;
> +	size_t count = get_iov_size(iov, iovcnt);
> +
> +	while (count>  0) {
> +		ssize_t nr;
> +
> +		nr = xpreadv(fd, iov, iovcnt, offset);
> +		if (nr<= 0) {
> +			if (total>  0)
> +				return total;
> +
> +			return -1;
> +		}
> +
> +		while ((size_t)nr>= iov->iov_len) {
> +			nr -= iov->iov_len;
> +			total += iov->iov_len;
> +			count -= iov->iov_len;
> +			iovcnt--;
> +			iov++;
> +		}
> +	}
> +
> +	return total;
> +}
> +
> +ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset)
> +{
> +	ssize_t total = 0;
> +	size_t count = get_iov_size(iov, iovcnt);
> +
> +	while (count>  0) {
> +		ssize_t nr;
> +
> +		nr = xpwritev(fd, iov, iovcnt, offset);
> +		if (nr<  0)
> +			return -1;
> +		if (nr == 0) {
> +			errno = ENOSPC;
> +			return -1;
> +		}
> +		while ((size_t)nr>= iov->iov_len) {
> +			nr -= iov->iov_len;
> +			total += iov->iov_len;
> +			count -= iov->iov_len;
> +			iovcnt--;
> +			iov++;

You forget here to change offset.

> +		}
> +	}
> +
> +	return total;
> +}


It might be better to add an helper function, something like this:

static inline void shift_iovec(const struct iovec **iov, int *iovcnt,
			       size_t nr, ssize_t *total, off_t *offset)
{
	while (nr >= (*iov)->iov_len) {
		nr -= (*iov)->iov_len;
		*total += (*iov)->iov_len;
		*offset += (*iov)->iov_len;
		*iovcnt--;
		*iov++;
	}
}

ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset)
{
	ssize_t nr, total = 0;

	while (iovcnt) {
		nr = xpwritev(fd, iov, iovcnt, offset);
		if (nr < 0)
			return -1;
		if (nr == 0) {
			errno = ENOSPC;
			return -1;
		}
		shift_iovec(&iov, &iovcnt, nr, &total, &offset);
	}

	return total;
}

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Sasha Levin]

On Sat, 2011-04-16 at 22:40 +0400, Konstantin Khlebnikov wrote:
> It might be better to add an helper function, something like this:
> 
> static inline void shift_iovec(const struct iovec **iov, int *iovcnt,
> 			       size_t nr, ssize_t *total, off_t *offset)
> {
> 	while (nr >= (*iov)->iov_len) {
> 		nr -= (*iov)->iov_len;
> 		*total += (*iov)->iov_len;
> 		*offset += (*iov)->iov_len;
> 		*iovcnt--;
> 		*iov++;
> 	}
> }
> 
> ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, off_t offset)
> {
> 	ssize_t nr, total = 0;
> 
> 	while (iovcnt) {
> 		nr = xpwritev(fd, iov, iovcnt, offset);
> 		if (nr < 0)
> 			return -1;
> 		if (nr == 0) {
> 			errno = ENOSPC;
> 			return -1;
> 		}
> 		shift_iovec(&iov, &iovcnt, nr, &total, &offset);
> 	}
> 
> 	return total;
> }

Yup, Good idea - Let's do that.

Thanks!

-- 

Sasha.

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Avi Kivity]

On 04/16/2011 09:40 PM, Konstantin Khlebnikov wrote:
>
> It might be better to add an helper function, something like this:
>
> static inline void shift_iovec(const struct iovec **iov, int *iovcnt,
>                    size_t nr, ssize_t *total, off_t *offset)
> {
>     while (nr >= (*iov)->iov_len) {
>         nr -= (*iov)->iov_len;
>         *total += (*iov)->iov_len;
>         *offset += (*iov)->iov_len;
>         *iovcnt--;
>         *iov++;
>     }
> }
>

What if nr is nonzero when the loop terminates?  You need to update the 
first iovec entry so you don't redo that segment.

> ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt, 
> off_t offset)
> {
>     ssize_t nr, total = 0;
>
>     while (iovcnt) {
>         nr = xpwritev(fd, iov, iovcnt, offset);
>         if (nr < 0)
>             return -1;
>         if (nr == 0) {
>             errno = ENOSPC;
>             return -1;
>         }
>         shift_iovec(&iov, &iovcnt, nr, &total, &offset);
>     }
>
>     return total;
> }
> -- 
> 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


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

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Konstantin Khlebnikov]

Avi Kivity wrote:
> On 04/16/2011 09:40 PM, Konstantin Khlebnikov wrote:
>>
>> It might be better to add an helper function, something like this:
>>
>> static inline void shift_iovec(const struct iovec **iov, int *iovcnt,
>>                     size_t nr, ssize_t *total, off_t *offset)
>> {
>>      while (nr>= (*iov)->iov_len) {
>>          nr -= (*iov)->iov_len;
>>          *total += (*iov)->iov_len;
>>          *offset += (*iov)->iov_len;
>>          *iovcnt--;
>>          *iov++;
>>      }
	if (nr) {
		(*iov)->iov_base += nr;
	  	(*iov)->iov_len -= nr;
	}
>> }
>>
>
> What if nr is nonzero when the loop terminates?  You need to update the
> first iovec entry so you don't redo that segment.

Oh yes, it is simple (hunk above), so "const" from prototype must be removed,
while original syscalls declare iov argument as const.

>
>> ssize_t pwritev_in_full(int fd, const struct iovec *iov, int iovcnt,
>> off_t offset)
>> {
>>      ssize_t nr, total = 0;
>>
>>      while (iovcnt) {
>>          nr = xpwritev(fd, iov, iovcnt, offset);
>>          if (nr<  0)
>>              return -1;
>>          if (nr == 0) {
>>              errno = ENOSPC;
>>              return -1;
>>          }
>>          shift_iovec(&iov,&iovcnt, nr,&total,&offset);
>>      }
>>
>>      return total;
>> }
>> --
>> 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 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Sasha Levin]

On Sun, 2011-04-17 at 14:03 +0400, Konstantin Khlebnikov wrote:
> Avi Kivity wrote:
> > What if nr is nonzero when the loop terminates?  You need to update the
> > first iovec entry so you don't redo that segment.
> 
> Oh yes, it is simple (hunk above), so "const" from prototype must be removed,
> while original syscalls declare iov argument as const.

The problem here is that if we start changing the internals of the iovec
struct it forces us to copy it before calling the I/O functions since if
we change it within the functions it's unusable to the caller anymore.

Currently it's passed as a const because we only move pointers outside
the struct.

Basically the trade-off here is copying iovec before every read/write vs
having to sometimes read data again (< 1 block), so I've went with the
latter.

Do you think it's better to go with the first alternative?


-- 

Sasha.

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Konstantin Khlebnikov]

Sasha Levin wrote:
> On Sun, 2011-04-17 at 14:03 +0400, Konstantin Khlebnikov wrote:
>> Avi Kivity wrote:
>>> What if nr is nonzero when the loop terminates?  You need to update the
>>> first iovec entry so you don't redo that segment.
>>
>> Oh yes, it is simple (hunk above), so "const" from prototype must be removed,
>> while original syscalls declare iov argument as const.
>
> The problem here is that if we start changing the internals of the iovec
> struct it forces us to copy it before calling the I/O functions since if
> we change it within the functions it's unusable to the caller anymore.
>
> Currently it's passed as a const because we only move pointers outside
> the struct.
>
> Basically the trade-off here is copying iovec before every read/write vs
> having to sometimes read data again (<  1 block), so I've went with the
> latter.
>
> Do you think it's better to go with the first alternative?
>
>

Or we can call one non-vectored operation for the rest of last io-verctor.

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Sasha Levin]

On Sun, 2011-04-17 at 14:34 +0400, Konstantin Khlebnikov wrote:
> Sasha Levin wrote:
> > On Sun, 2011-04-17 at 14:03 +0400, Konstantin Khlebnikov wrote:
> >> Avi Kivity wrote:
> >>> What if nr is nonzero when the loop terminates?  You need to update the
> >>> first iovec entry so you don't redo that segment.
> >>
> >> Oh yes, it is simple (hunk above), so "const" from prototype must be removed,
> >> while original syscalls declare iov argument as const.
> >
> > The problem here is that if we start changing the internals of the iovec
> > struct it forces us to copy it before calling the I/O functions since if
> > we change it within the functions it's unusable to the caller anymore.
> >
> > Currently it's passed as a const because we only move pointers outside
> > the struct.
> >
> > Basically the trade-off here is copying iovec before every read/write vs
> > having to sometimes read data again (<  1 block), so I've went with the
> > latter.
> >
> > Do you think it's better to go with the first alternative?
> >
> >
> 
> Or we can call one non-vectored operation for the rest of last io-verctor.

I don't think it's worth delaying the next readv() to run through
read_in_full, and whats more, the part we're going to read again is
probably sitting in the file cache anyway.

-- 

Sasha.

Re: [PATCH 1/2 V2] kvm tools: Add scatter-gather variants of IO functions

[Avi Kivity]

On 04/17/2011 02:11 PM, Sasha Levin wrote:
> I don't think it's worth delaying the next readv() to run through
> read_in_full, and whats more, the part we're going to read again is
> probably sitting in the file cache anyway.

If you have a file cache.  High performance often means bypassing the 
cache, and in those cases submitting guest requests exactly as the guest 
passed them is important.

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