From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yoshiaki Tamura Subject: Re: [RFC PATCH 05/20] Introduce put_vector() and get_vector to QEMUFile and qemu_fopen_ops(). Date: Fri, 23 Apr 2010 12:37:40 +0900 Message-ID: <4BD11604.3060309@lab.ntt.co.jp> References: <1271829445-5328-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1271829445-5328-6-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4BD0A35E.8000205@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, avi@redhat.com, Anthony Liguori , mtosatti@redhat.com, ohmura.kei@lab.ntt.co.jp, yoshikawa.takuya@oss.ntt.co.jp To: Anthony Liguori Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:58067 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753519Ab0DWDiB (ORCPT ); Thu, 22 Apr 2010 23:38:01 -0400 In-Reply-To: <4BD0A35E.8000205@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 > > 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; > > > >