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

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

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@lab.ntt.co.jp>
>    

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.

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;
>