Re: [PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback

2022-06-09 Thread Dr. David Alan Gilbert
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> This directly implements the writev_buffer logic using QIOChannel APIs.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  migration/qemu-file-channel.c | 43 ---
>  migration/qemu-file.c | 24 +++
>  migration/qemu-file.h |  9 
>  3 files changed, 8 insertions(+), 68 deletions(-)
> 
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 7b32831752..2e139f7bcd 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -32,48 +32,6 @@
>  #include "yank_functions.h"
>  
>  
> -static ssize_t channel_writev_buffer(void *opaque,
> - struct iovec *iov,
> - int iovcnt,
> - int64_t pos,
> - Error **errp)
> -{
> -QIOChannel *ioc = QIO_CHANNEL(opaque);
> -ssize_t done = 0;
> -struct iovec *local_iov = g_new(struct iovec, iovcnt);
> -struct iovec *local_iov_head = local_iov;
> -unsigned int nlocal_iov = iovcnt;
> -
> -nlocal_iov = iov_copy(local_iov, nlocal_iov,
> -  iov, iovcnt,
> -  0, iov_size(iov, iovcnt));
> -
> -while (nlocal_iov > 0) {
> -ssize_t len;
> -len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
> -if (len == QIO_CHANNEL_ERR_BLOCK) {
> -if (qemu_in_coroutine()) {
> -qio_channel_yield(ioc, G_IO_OUT);
> -} else {
> -qio_channel_wait(ioc, G_IO_OUT);
> -}
> -continue;

I wondered where that code went, but it turns out it's already copied
into qio_channel_writev_full_all, so:


Reviewed-by: Dr. David Alan Gilbert 

> -}
> -if (len < 0) {
> -done = -EIO;
> -goto cleanup;
> -}
> -
> -iov_discard_front(_iov, _iov, len);
> -done += len;
> -}
> -
> - cleanup:
> -g_free(local_iov_head);
> -return done;
> -}
> -
> -
>  static QEMUFile *channel_get_input_return_path(void *opaque)
>  {
>  QIOChannel *ioc = QIO_CHANNEL(opaque);
> @@ -94,7 +52,6 @@ static const QEMUFileOps channel_input_ops = {
>  
>  
>  static const QEMUFileOps channel_output_ops = {
> -.writev_buffer = channel_writev_buffer,
>  .get_return_path = channel_get_output_return_path,
>  };
>  
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index e024b43851..72a6f58af5 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -245,10 +245,6 @@ static void qemu_iovec_release_ram(QEMUFile *f)
>   */
>  void qemu_fflush(QEMUFile *f)
>  {
> -ssize_t ret = 0;
> -ssize_t expect = 0;
> -Error *local_error = NULL;
> -
>  if (!qemu_file_is_writable(f)) {
>  return;
>  }
> @@ -257,22 +253,18 @@ void qemu_fflush(QEMUFile *f)
>  return;
>  }
>  if (f->iovcnt > 0) {
> -expect = iov_size(f->iov, f->iovcnt);
> -ret = f->ops->writev_buffer(f->ioc, f->iov, f->iovcnt, 
> f->total_transferred,
> -_error);
> +Error *local_error = NULL;
> +if (qio_channel_writev_all(f->ioc,
> +   f->iov, f->iovcnt,
> +   _error) < 0) {
> +qemu_file_set_error_obj(f, -EIO, local_error);
> +} else {
> +f->total_transferred += iov_size(f->iov, f->iovcnt);
> +}
>  
>  qemu_iovec_release_ram(f);
>  }
>  
> -if (ret >= 0) {
> -f->total_transferred += ret;
> -}
> -/* We expect the QEMUFile write impl to send the full
> - * data set we requested, so sanity check that.
> - */
> -if (ret != expect) {
> -qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
> -}
>  f->buf_index = 0;
>  f->iovcnt = 0;
>  }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index cd49184c00..542c637934 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -29,14 +29,6 @@
>  #include "exec/cpu-common.h"
>  #include "io/channel.h"
>  
> -/*
> - * This function writes an iovec to file. The handler must write all
> - * of the data or return a negative errno value.
> - */
> -typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
> -   int iovcnt, int64_t pos,
> -   Error **errp);
> -
>  /*
>   * This function provides hooks around different
>   * stages of RAM migration.
> @@ -69,7 +61,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
>  typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>  
>  typedef struct QEMUFileOps {
> -QEMUFileWritevBufferFunc *writev_buffer;
>  QEMURetPathFunc *get_return_path;
>  } QEMUFileOps;
>  
> -- 
> 2.36.1
> 
-- 
Dr. David Alan Gilbert / 

[PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback

2022-05-24 Thread Daniel P . Berrangé
This directly implements the writev_buffer logic using QIOChannel APIs.

Signed-off-by: Daniel P. Berrangé 
---
 migration/qemu-file-channel.c | 43 ---
 migration/qemu-file.c | 24 +++
 migration/qemu-file.h |  9 
 3 files changed, 8 insertions(+), 68 deletions(-)

diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 7b32831752..2e139f7bcd 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -32,48 +32,6 @@
 #include "yank_functions.h"
 
 
-static ssize_t channel_writev_buffer(void *opaque,
- struct iovec *iov,
- int iovcnt,
- int64_t pos,
- Error **errp)
-{
-QIOChannel *ioc = QIO_CHANNEL(opaque);
-ssize_t done = 0;
-struct iovec *local_iov = g_new(struct iovec, iovcnt);
-struct iovec *local_iov_head = local_iov;
-unsigned int nlocal_iov = iovcnt;
-
-nlocal_iov = iov_copy(local_iov, nlocal_iov,
-  iov, iovcnt,
-  0, iov_size(iov, iovcnt));
-
-while (nlocal_iov > 0) {
-ssize_t len;
-len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
-if (len == QIO_CHANNEL_ERR_BLOCK) {
-if (qemu_in_coroutine()) {
-qio_channel_yield(ioc, G_IO_OUT);
-} else {
-qio_channel_wait(ioc, G_IO_OUT);
-}
-continue;
-}
-if (len < 0) {
-done = -EIO;
-goto cleanup;
-}
-
-iov_discard_front(_iov, _iov, len);
-done += len;
-}
-
- cleanup:
-g_free(local_iov_head);
-return done;
-}
-
-
 static QEMUFile *channel_get_input_return_path(void *opaque)
 {
 QIOChannel *ioc = QIO_CHANNEL(opaque);
@@ -94,7 +52,6 @@ static const QEMUFileOps channel_input_ops = {
 
 
 static const QEMUFileOps channel_output_ops = {
-.writev_buffer = channel_writev_buffer,
 .get_return_path = channel_get_output_return_path,
 };
 
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index e024b43851..72a6f58af5 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -245,10 +245,6 @@ static void qemu_iovec_release_ram(QEMUFile *f)
  */
 void qemu_fflush(QEMUFile *f)
 {
-ssize_t ret = 0;
-ssize_t expect = 0;
-Error *local_error = NULL;
-
 if (!qemu_file_is_writable(f)) {
 return;
 }
@@ -257,22 +253,18 @@ void qemu_fflush(QEMUFile *f)
 return;
 }
 if (f->iovcnt > 0) {
-expect = iov_size(f->iov, f->iovcnt);
-ret = f->ops->writev_buffer(f->ioc, f->iov, f->iovcnt, 
f->total_transferred,
-_error);
+Error *local_error = NULL;
+if (qio_channel_writev_all(f->ioc,
+   f->iov, f->iovcnt,
+   _error) < 0) {
+qemu_file_set_error_obj(f, -EIO, local_error);
+} else {
+f->total_transferred += iov_size(f->iov, f->iovcnt);
+}
 
 qemu_iovec_release_ram(f);
 }
 
-if (ret >= 0) {
-f->total_transferred += ret;
-}
-/* We expect the QEMUFile write impl to send the full
- * data set we requested, so sanity check that.
- */
-if (ret != expect) {
-qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
-}
 f->buf_index = 0;
 f->iovcnt = 0;
 }
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index cd49184c00..542c637934 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -29,14 +29,6 @@
 #include "exec/cpu-common.h"
 #include "io/channel.h"
 
-/*
- * This function writes an iovec to file. The handler must write all
- * of the data or return a negative errno value.
- */
-typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
-   int iovcnt, int64_t pos,
-   Error **errp);
-
 /*
  * This function provides hooks around different
  * stages of RAM migration.
@@ -69,7 +61,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
 typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
 
 typedef struct QEMUFileOps {
-QEMUFileWritevBufferFunc *writev_buffer;
 QEMURetPathFunc *get_return_path;
 } QEMUFileOps;
 
-- 
2.36.1