Re: [Qemu-devel] [PATCH v1 21/22] migration: remove support for non-iovec based write handlers

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> All the remaining QEMUFile implementations provide an iovec
> based write handler, so the put_buffer callback can be removed
> to simplify the code.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  include/migration/qemu-file.h |  9 ---------
>  migration/qemu-file.c         | 36 ++++++++----------------------------
>  migration/savevm.c            |  8 --------
>  3 files changed, 8 insertions(+), 45 deletions(-)
> 
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index 6a66735..4ee40e4 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -29,14 +29,6 @@
>  
>  #include <stdint.h>
>  
> -/* This function writes a chunk of data to a file at the given position.
> - * The pos argument can be ignored if the file is only being used for
> - * streaming.  The handler must write all of the data or return a negative
> - * errno value.
> - */
> -typedef ssize_t (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
> -                                        int64_t pos, size_t size);
> -
>  /* 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.
> @@ -110,7 +102,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>  typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
>  
>  typedef struct QEMUFileOps {
> -    QEMUFilePutBufferFunc *put_buffer;
>      QEMUFileGetBufferFunc *get_buffer;
>      QEMUFileCloseFunc *close;
>      QEMUFileGetFD *get_fd;
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 333f418..372a6b9 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -128,7 +128,7 @@ void qemu_file_set_error(QEMUFile *f, int ret)
>  
>  bool qemu_file_is_writable(QEMUFile *f)
>  {
> -    return f->ops->writev_buffer || f->ops->put_buffer;
> +    return f->ops->writev_buffer;
>  }
>  
>  /**
> @@ -147,16 +147,9 @@ void qemu_fflush(QEMUFile *f)
>          return;
>      }
>  
> -    if (f->ops->writev_buffer) {
> -        if (f->iovcnt > 0) {
> -            expect = iov_size(f->iov, f->iovcnt);
> -            ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
> -        }
> -    } else {
> -        if (f->buf_index > 0) {
> -            expect = f->buf_index;
> -            ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
> -        }
> +    if (f->iovcnt > 0) {
> +        expect = iov_size(f->iov, f->iovcnt);
> +        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
>      }
>  
>      if (ret >= 0) {
> @@ -336,11 +329,6 @@ static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size)
>  
>  void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size)
>  {
> -    if (!f->ops->writev_buffer) {
> -        qemu_put_buffer(f, buf, size);
> -        return;
> -    }
> -
>      if (f->last_error) {
>          return;
>      }
> @@ -364,9 +352,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
>          }
>          memcpy(f->buf + f->buf_index, buf, l);
>          f->bytes_xfer += l;
> -        if (f->ops->writev_buffer) {
> -            add_to_iovec(f, f->buf + f->buf_index, l);
> -        }
> +        add_to_iovec(f, f->buf + f->buf_index, l);
>          f->buf_index += l;
>          if (f->buf_index == IO_BUF_SIZE) {
>              qemu_fflush(f);
> @@ -387,9 +373,7 @@ void qemu_put_byte(QEMUFile *f, int v)
>  
>      f->buf[f->buf_index] = v;
>      f->bytes_xfer++;
> -    if (f->ops->writev_buffer) {
> -        add_to_iovec(f, f->buf + f->buf_index, 1);
> -    }
> +    add_to_iovec(f, f->buf + f->buf_index, 1);
>      f->buf_index++;
>      if (f->buf_index == IO_BUF_SIZE) {
>          qemu_fflush(f);
> @@ -553,12 +537,8 @@ int64_t qemu_ftell_fast(QEMUFile *f)
>      int64_t ret = f->pos;
>      int i;
>  
> -    if (f->ops->writev_buffer) {
> -        for (i = 0; i < f->iovcnt; i++) {
> -            ret += f->iov[i].iov_len;
> -        }
> -    } else {
> -        ret += f->buf_index;
> +    for (i = 0; i < f->iovcnt; i++) {
> +        ret += f->iov[i].iov_len;
>      }
>  
>      return ret;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index e57d7ce..a991c0c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -160,13 +160,6 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>      return qiov.size;
>  }
>  
> -static ssize_t block_put_buffer(void *opaque, const uint8_t *buf,
> -                                int64_t pos, size_t size)
> -{
> -    bdrv_save_vmstate(opaque, buf, pos, size);
> -    return size;
> -}
> -
>  static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
>                                  size_t size)
>  {
> @@ -184,7 +177,6 @@ static const QEMUFileOps bdrv_read_ops = {
>  };
>  
>  static const QEMUFileOps bdrv_write_ops = {
> -    .put_buffer     = block_put_buffer,
>      .writev_buffer  = block_writev_buffer,
>      .close          = bdrv_fclose
>  };
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK