Re: [RFC PATCH 05/20] Introduce put_vector() and get_vector to QEMUFile and qemu_fopen_ops().

Yoshiaki Tamura <tamura.yoshiaki@xxxxxxxxxxxxx> · Fri, 23 Apr 2010 12:37:40 +0900

Anthony Liguori wrote:
On 04/21/2010 12:57 AM, Yoshiaki Tamura wrote:
QEMUFile currently doesn't support writev(). For sending multiple
data, such as pages, using writev() should be more efficient.

Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@xxxxxxxxxxxxx>

Is there performance data that backs this up? Since QEMUFile uses a
linear buffer for most operations that's limited to 16k, I suspect you
wouldn't be able to observe a difference in practice.

I currently don't have data, but I'll prepare it.
There were two things I wanted to avoid.

1. Pages to be copied to QEMUFile buf through qemu_put_buffer.
2. Calling write() everytime even when we want to send multiple pages at once.

I think 2 may be neglectable.
But 1 seems to be problematic if we want make to the latency as small as 
possible, no?


Regards,

Anthony Liguori

---
buffered_file.c | 2 +-
hw/hw.h | 16 ++++++++++++++++
savevm.c | 43 +++++++++++++++++++++++++------------------
3 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/buffered_file.c b/buffered_file.c
index 54dc6c2..187d1d4 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -256,7 +256,7 @@ QEMUFile *qemu_fopen_ops_buffered(void *opaque,
s->wait_for_unfreeze = wait_for_unfreeze;
s->close = close;

- s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
+ s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL, NULL, NULL,
buffered_close, buffered_rate_limit,
buffered_set_rate_limit,
buffered_get_rate_limit);
diff --git a/hw/hw.h b/hw/hw.h
index fc9ed29..921cf90 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -23,6 +23,13 @@
typedef int (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
int64_t pos, int size);

+/* This function writes a chunk of vector to a file at the given
position.
+ * The pos argument can be ignored if the file is only being used for
+ * streaming.
+ */
+typedef int (QEMUFilePutVectorFunc)(void *opaque, struct iovec *iov,
+ int64_t pos, int iovcnt);
+
/* Read a chunk of data from a file at the given position. The pos
argument
* can be ignored if the file is only be used for streaming. The number of
* bytes actually read should be returned.
@@ -30,6 +37,13 @@ typedef int (QEMUFilePutBufferFunc)(void *opaque,
const uint8_t *buf,
typedef int (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
int64_t pos, int size);

+/* Read a chunk of vector from a file at the given position. The pos
argument
+ * can be ignored if the file is only be used for streaming. The
number of
+ * bytes actually read should be returned.
+ */
+typedef int (QEMUFileGetVectorFunc)(void *opaque, struct iovec *iov,
+ int64_t pos, int iovcnt);
+
/* Close a file and return an error code */
typedef int (QEMUFileCloseFunc)(void *opaque);

@@ -46,7 +60,9 @@ typedef size_t (QEMUFileSetRateLimit)(void *opaque,
size_t new_rate);
typedef size_t (QEMUFileGetRateLimit)(void *opaque);

QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
+ QEMUFilePutVectorFunc *put_vector,
QEMUFileGetBufferFunc *get_buffer,
+ QEMUFileGetVectorFunc *get_vector,
QEMUFileCloseFunc *close,
QEMUFileRateLimit *rate_limit,
QEMUFileSetRateLimit *set_rate_limit,
diff --git a/savevm.c b/savevm.c
index 490ab70..944e788 100644
--- a/savevm.c
+++ b/savevm.c
@@ -162,7 +162,9 @@ void qemu_announce_self(void)

struct QEMUFile {
QEMUFilePutBufferFunc *put_buffer;
+ QEMUFilePutVectorFunc *put_vector;
QEMUFileGetBufferFunc *get_buffer;
+ QEMUFileGetVectorFunc *get_vector;
QEMUFileCloseFunc *close;
QEMUFileRateLimit *rate_limit;
QEMUFileSetRateLimit *set_rate_limit;
@@ -263,11 +265,11 @@ QEMUFile *qemu_popen(FILE *stdio_file, const
char *mode)
s->stdio_file = stdio_file;

if(mode[0] == 'r') {
- s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_pclose,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, NULL, NULL, stdio_get_buffer,
+ NULL, stdio_pclose, NULL, NULL, NULL);
} else {
- s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_pclose,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, NULL, NULL,
+ stdio_pclose, NULL, NULL, NULL);
}
return s->file;
}
@@ -312,11 +314,11 @@ QEMUFile *qemu_fdopen(int fd, const char *mode)
goto fail;

if(mode[0] == 'r') {
- s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, NULL, NULL, stdio_get_buffer, NULL,
+ stdio_fclose, NULL, NULL, NULL);
} else {
- s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, NULL, NULL,
+ stdio_fclose, NULL, NULL, NULL);
}
return s->file;

@@ -330,8 +332,8 @@ QEMUFile *qemu_fopen_socket(int fd)
QEMUFileSocket *s = qemu_mallocz(sizeof(QEMUFileSocket));

s->fd = fd;
- s->file = qemu_fopen_ops(s, NULL, socket_get_buffer, socket_close,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, NULL, NULL, socket_get_buffer, NULL,
+ socket_close, NULL, NULL, NULL);
return s->file;
}

@@ -368,11 +370,11 @@ QEMUFile *qemu_fopen(const char *filename, const
char *mode)
goto fail;

if(mode[0] == 'w') {
- s->file = qemu_fopen_ops(s, file_put_buffer, NULL, stdio_fclose,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, file_put_buffer, NULL, NULL, NULL,
+ stdio_fclose, NULL, NULL, NULL);
} else {
- s->file = qemu_fopen_ops(s, NULL, file_get_buffer, stdio_fclose,
- NULL, NULL, NULL);
+ s->file = qemu_fopen_ops(s, NULL, NULL, file_get_buffer, NULL,
+ stdio_fclose, NULL, NULL, NULL);
}
return s->file;
fail:
@@ -400,13 +402,16 @@ static int bdrv_fclose(void *opaque)
static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
{
if (is_writable)
- return qemu_fopen_ops(bs, block_put_buffer, NULL, bdrv_fclose,
- NULL, NULL, NULL);
- return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL,
NULL, NULL);
+ return qemu_fopen_ops(bs, block_put_buffer, NULL, NULL, NULL,
+ bdrv_fclose, NULL, NULL, NULL);
+ return qemu_fopen_ops(bs, NULL, NULL, block_get_buffer, NULL,
bdrv_fclose, NULL, NULL, NULL);
}

-QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc
*put_buffer,
+QEMUFile *qemu_fopen_ops(void *opaque,
+ QEMUFilePutBufferFunc *put_buffer,
+ QEMUFilePutVectorFunc *put_vector,
QEMUFileGetBufferFunc *get_buffer,
+ QEMUFileGetVectorFunc *get_vector,
QEMUFileCloseFunc *close,
QEMUFileRateLimit *rate_limit,
QEMUFileSetRateLimit *set_rate_limit,
@@ -418,7 +423,9 @@ QEMUFile *qemu_fopen_ops(void *opaque,
QEMUFilePutBufferFunc *put_buffer,

f->opaque = opaque;
f->put_buffer = put_buffer;
+ f->put_vector = put_vector;
f->get_buffer = get_buffer;
+ f->get_vector = get_vector;
f->close = close;
f->rate_limit = rate_limit;
f->set_rate_limit = set_rate_limit;





--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




