diff mbox series

migration: Add error_desc for file channel errors

Message ID 20190422103420.15686-1-yury-kotov@yandex-team.ru (mailing list archive)
State New, archived
Headers show
Series migration: Add error_desc for file channel errors | expand

Commit Message

Yury Kotov April 22, 2019, 10:34 a.m. UTC
Currently, there is no information about error if outgoing migration was failed
because of file channel errors.
Example (QMP session):
-> { "execute": "migrate", "arguments": { "uri": "exec:head -c 1" }}
<- { "return": {} }
...
-> { "execute": "query-migrate" }
<- { "return": { "status": "failed" }} // There is not error's description

And even in the QEMU's output there is nothing.

This patch
1) Adds errp for the most of QEMUFileOps
2) Adds qemu_file_get_error_obj/qemu_file_set_error_obj
3) And finally using of qemu_file_get_error_obj in migration.c

And now, the status for the mentioned fail will be:
-> { "execute": "query-migrate" }
<- { "return": { "status": "failed",
                 "error-desc": "Unable to write to command: Broken pipe" }}

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 migration/migration.c         | 10 ++++--
 migration/qemu-file-channel.c | 30 +++++++++--------
 migration/qemu-file.c         | 63 ++++++++++++++++++++++++++++-------
 migration/qemu-file.h         | 15 ++++++---
 migration/savevm.c            |  6 ++--
 5 files changed, 88 insertions(+), 36 deletions(-)

Comments

Yury Kotov June 7, 2019, 9:02 a.m. UTC | #1
Ping

22.04.2019, 13:50, "Yury Kotov" <yury-kotov@yandex-team.ru>:
> Currently, there is no information about error if outgoing migration was failed
> because of file channel errors.
> Example (QMP session):
> -> { "execute": "migrate", "arguments": { "uri": "exec:head -c 1" }}
> <- { "return": {} }
> ...
> -> { "execute": "query-migrate" }
> <- { "return": { "status": "failed" }} // There is not error's description
>
> And even in the QEMU's output there is nothing.
>
> This patch
> 1) Adds errp for the most of QEMUFileOps
> 2) Adds qemu_file_get_error_obj/qemu_file_set_error_obj
> 3) And finally using of qemu_file_get_error_obj in migration.c
>
> And now, the status for the mentioned fail will be:
> -> { "execute": "query-migrate" }
> <- { "return": { "status": "failed",
>                  "error-desc": "Unable to write to command: Broken pipe" }}
>
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> ---
>  migration/migration.c | 10 ++++--
>  migration/qemu-file-channel.c | 30 +++++++++--------
>  migration/qemu-file.c | 63 ++++++++++++++++++++++++++++-------
>  migration/qemu-file.h | 15 ++++++---
>  migration/savevm.c | 6 ++--
>  5 files changed, 88 insertions(+), 36 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..7bcdc4613b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2949,6 +2949,7 @@ static MigThrError migration_detect_error(MigrationState *s)
>  {
>      int ret;
>      int state = s->state;
> + Error *local_error = NULL;
>
>      if (state == MIGRATION_STATUS_CANCELLING ||
>          state == MIGRATION_STATUS_CANCELLED) {
> @@ -2957,13 +2958,18 @@ static MigThrError migration_detect_error(MigrationState *s)
>      }
>
>      /* Try to detect any file errors */
> - ret = qemu_file_get_error(s->to_dst_file);
> -
> + ret = qemu_file_get_error_obj(s->to_dst_file, &local_error);
>      if (!ret) {
>          /* Everything is fine */
> + assert(!local_error);
>          return MIG_THR_ERR_NONE;
>      }
>
> + if (local_error) {
> + migrate_set_error(s, local_error);
> + error_free(local_error);
> + }
> +
>      if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret == -EIO) {
>          /*
>           * For postcopy, we allow the network to be down for a
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 8e639eb496..c382ea2d78 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -33,7 +33,8 @@
>  static ssize_t channel_writev_buffer(void *opaque,
>                                       struct iovec *iov,
>                                       int iovcnt,
> - int64_t pos)
> + int64_t pos,
> + Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>      ssize_t done = 0;
> @@ -47,7 +48,7 @@ static ssize_t channel_writev_buffer(void *opaque,
>
>      while (nlocal_iov > 0) {
>          ssize_t len;
> - len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
> + 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);
> @@ -57,7 +58,6 @@ static ssize_t channel_writev_buffer(void *opaque,
>              continue;
>          }
>          if (len < 0) {
> - /* XXX handle Error objects */
>              done = -EIO;
>              goto cleanup;
>          }
> @@ -75,13 +75,14 @@ static ssize_t channel_writev_buffer(void *opaque,
>  static ssize_t channel_get_buffer(void *opaque,
>                                    uint8_t *buf,
>                                    int64_t pos,
> - size_t size)
> + size_t size,
> + Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>      ssize_t ret;
>
>      do {
> - ret = qio_channel_read(ioc, (char *)buf, size, NULL);
> + ret = qio_channel_read(ioc, (char *)buf, size, errp);
>          if (ret < 0) {
>              if (ret == QIO_CHANNEL_ERR_BLOCK) {
>                  if (qemu_in_coroutine()) {
> @@ -90,7 +91,6 @@ static ssize_t channel_get_buffer(void *opaque,
>                      qio_channel_wait(ioc, G_IO_IN);
>                  }
>              } else {
> - /* XXX handle Error * object */
>                  return -EIO;
>              }
>          }
> @@ -100,18 +100,20 @@ static ssize_t channel_get_buffer(void *opaque,
>  }
>
> -static int channel_close(void *opaque)
> +static int channel_close(void *opaque, Error **errp)
>  {
> + int ret;
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
> - qio_channel_close(ioc, NULL);
> + ret = qio_channel_close(ioc, errp);
>      object_unref(OBJECT(ioc));
> - return 0;
> + return ret;
>  }
>
>  static int channel_shutdown(void *opaque,
>                              bool rd,
> - bool wr)
> + bool wr,
> + Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>
> @@ -125,8 +127,7 @@ static int channel_shutdown(void *opaque,
>          } else {
>              mode = QIO_CHANNEL_SHUTDOWN_WRITE;
>          }
> - if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
> - /* XXX handler Error * object */
> + if (qio_channel_shutdown(ioc, mode, errp) < 0) {
>              return -EIO;
>          }
>      }
> @@ -135,11 +136,12 @@ static int channel_shutdown(void *opaque,
>
>  static int channel_set_blocking(void *opaque,
> - bool enabled)
> + bool enabled,
> + Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>
> - if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
> + if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
>          return -1;
>      }
>      return 0;
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 977b9ae07c..c52160e08b 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -29,6 +29,7 @@
>  #include "migration.h"
>  #include "qemu-file.h"
>  #include "trace.h"
> +#include "qapi/error.h"
>
>  #define IO_BUF_SIZE 32768
>  #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
> @@ -52,6 +53,7 @@ struct QEMUFile {
>      unsigned int iovcnt;
>
>      int last_error;
> + Error *last_error_obj;
>  };
>
>  /*
> @@ -63,7 +65,7 @@ int qemu_file_shutdown(QEMUFile *f)
>      if (!f->ops->shut_down) {
>          return -ENOSYS;
>      }
> - return f->ops->shut_down(f->opaque, true, true);
> + return f->ops->shut_down(f->opaque, true, true, NULL);
>  }
>
>  /*
> @@ -108,24 +110,55 @@ void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
>  }
>
>  /*
> - * Get last error for stream f
> + * Get last error for stream f with optional Error*
>   *
>   * Return negative error value if there has been an error on previous
>   * operations, return 0 if no error happened.
> + * Optional, it returns Error* in errp, but it may be NULL even if return value
> + * is not 0.
>   *
>   */
> -int qemu_file_get_error(QEMUFile *f)
> +int qemu_file_get_error_obj(QEMUFile *f, Error **errp)
>  {
> + if (errp) {
> + *errp = f->last_error_obj ? error_copy(f->last_error_obj) : NULL;
> + }
>      return f->last_error;
>  }
>
> -void qemu_file_set_error(QEMUFile *f, int ret)
> +/*
> + * Set the last error for stream f with optional Error*
> + */
> +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
>  {
> - if (f->last_error == 0) {
> + if (f->last_error == 0 && ret) {
>          f->last_error = ret;
> + error_propagate(&f->last_error_obj, err);
> + } else if (err) {
> + error_report_err(err);
>      }
>  }
>
> +/*
> + * Get last error for stream f
> + *
> + * Return negative error value if there has been an error on previous
> + * operations, return 0 if no error happened.
> + *
> + */
> +int qemu_file_get_error(QEMUFile *f)
> +{
> + return qemu_file_get_error_obj(f, NULL);
> +}
> +
> +/*
> + * Set the last error for stream f
> + */
> +void qemu_file_set_error(QEMUFile *f, int ret)
> +{
> + qemu_file_set_error_obj(f, ret, NULL);
> +}
> +
>  bool qemu_file_is_writable(QEMUFile *f)
>  {
>      return f->ops->writev_buffer;
> @@ -177,6 +210,7 @@ void qemu_fflush(QEMUFile *f)
>  {
>      ssize_t ret = 0;
>      ssize_t expect = 0;
> + Error *local_error = NULL;
>
>      if (!qemu_file_is_writable(f)) {
>          return;
> @@ -184,7 +218,8 @@ void qemu_fflush(QEMUFile *f)
>
>      if (f->iovcnt > 0) {
>          expect = iov_size(f->iov, f->iovcnt);
> - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
> + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> + &local_error);
>
>          qemu_iovec_release_ram(f);
>      }
> @@ -196,7 +231,7 @@ void qemu_fflush(QEMUFile *f)
>       * data set we requested, so sanity check that.
>       */
>      if (ret != expect) {
> - qemu_file_set_error(f, ret < 0 ? ret : -EIO);
> + qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
>      }
>      f->buf_index = 0;
>      f->iovcnt = 0;
> @@ -284,6 +319,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>  {
>      int len;
>      int pending;
> + Error *local_error = NULL;
>
>      assert(!qemu_file_is_writable(f));
>
> @@ -295,14 +331,16 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>      f->buf_size = pending;
>
>      len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> - IO_BUF_SIZE - pending);
> + IO_BUF_SIZE - pending, &local_error);
>      if (len > 0) {
>          f->buf_size += len;
>          f->pos += len;
>      } else if (len == 0) {
> - qemu_file_set_error(f, -EIO);
> + qemu_file_set_error_obj(f, -EIO, local_error);
>      } else if (len != -EAGAIN) {
> - qemu_file_set_error(f, len);
> + qemu_file_set_error_obj(f, len, local_error);
> + } else {
> + error_free(local_error);
>      }
>
>      return len;
> @@ -328,7 +366,7 @@ int qemu_fclose(QEMUFile *f)
>      ret = qemu_file_get_error(f);
>
>      if (f->ops->close) {
> - int ret2 = f->ops->close(f->opaque);
> + int ret2 = f->ops->close(f->opaque, NULL);
>          if (ret >= 0) {
>              ret = ret2;
>          }
> @@ -339,6 +377,7 @@ int qemu_fclose(QEMUFile *f)
>      if (f->last_error) {
>          ret = f->last_error;
>      }
> + error_free(f->last_error_obj);
>      g_free(f);
>      trace_qemu_file_fclose();
>      return ret;
> @@ -784,6 +823,6 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
>  void qemu_file_set_blocking(QEMUFile *f, bool block)
>  {
>      if (f->ops->set_blocking) {
> - f->ops->set_blocking(f->opaque, block);
> + f->ops->set_blocking(f->opaque, block, NULL);
>      }
>  }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 13baf896bd..eb886db65f 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -32,7 +32,8 @@
>   * bytes actually read should be returned.
>   */
>  typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> - int64_t pos, size_t size);
> + int64_t pos, size_t size,
> + Error **errp);
>
>  /* Close a file
>   *
> @@ -41,7 +42,7 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
>   * The meaning of return value on success depends on the specific back-end being
>   * used.
>   */
> -typedef int (QEMUFileCloseFunc)(void *opaque);
> +typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
>
>  /* Called to return the OS file descriptor associated to the QEMUFile.
>   */
> @@ -49,14 +50,15 @@ typedef int (QEMUFileGetFD)(void *opaque);
>
>  /* Called to change the blocking mode of the file
>   */
> -typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
> +typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
>
>  /*
>   * 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);
> + int iovcnt, int64_t pos,
> + Error **errp);
>
>  /*
>   * This function provides hooks around different
> @@ -97,7 +99,8 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>   * Existing blocking reads/writes must be woken
>   * Returns 0 on success, -err on error
>   */
> -typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
> +typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
> + Error **errp);
>
>  typedef struct QEMUFileOps {
>      QEMUFileGetBufferFunc *get_buffer;
> @@ -149,6 +152,8 @@ void qemu_update_position(QEMUFile *f, size_t size);
>  void qemu_file_reset_rate_limit(QEMUFile *f);
>  void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>  int64_t qemu_file_get_rate_limit(QEMUFile *f);
> +int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
> +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
>  void qemu_file_set_error(QEMUFile *f, int ret);
>  int qemu_file_shutdown(QEMUFile *f);
>  QEMUFile *qemu_file_get_return_path(QEMUFile *f);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 34bcad3807..a619af744d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -124,7 +124,7 @@ static struct mig_cmd_args {
>  /* savevm/loadvm support */
>
>  static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> - int64_t pos)
> + int64_t pos, Error **errp)
>  {
>      int ret;
>      QEMUIOVector qiov;
> @@ -139,12 +139,12 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>  }
>
>  static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> - size_t size)
> + size_t size, Error **errp)
>  {
>      return bdrv_load_vmstate(opaque, buf, pos, size);
>  }
>
> -static int bdrv_fclose(void *opaque)
> +static int bdrv_fclose(void *opaque, Error **errp)
>  {
>      return bdrv_flush(opaque);
>  }
> --
> 2.21.0
Dr. David Alan Gilbert June 14, 2019, 4:39 p.m. UTC | #2
* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Currently, there is no information about error if outgoing migration was failed
> because of file channel errors.
> Example (QMP session):
> -> { "execute": "migrate", "arguments": { "uri": "exec:head -c 1" }}
> <- { "return": {} }
> ...
> -> { "execute": "query-migrate" }
> <- { "return": { "status": "failed" }} // There is not error's description
> 
> And even in the QEMU's output there is nothing.
> 
> This patch
> 1) Adds errp for the most of QEMUFileOps
> 2) Adds qemu_file_get_error_obj/qemu_file_set_error_obj
> 3) And finally using of qemu_file_get_error_obj in migration.c
> 
> And now, the status for the mentioned fail will be:
> -> { "execute": "query-migrate" }
> <- { "return": { "status": "failed",
>                  "error-desc": "Unable to write to command: Broken pipe" }}
> 
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>

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

> ---
>  migration/migration.c         | 10 ++++--
>  migration/qemu-file-channel.c | 30 +++++++++--------
>  migration/qemu-file.c         | 63 ++++++++++++++++++++++++++++-------
>  migration/qemu-file.h         | 15 ++++++---
>  migration/savevm.c            |  6 ++--
>  5 files changed, 88 insertions(+), 36 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..7bcdc4613b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2949,6 +2949,7 @@ static MigThrError migration_detect_error(MigrationState *s)
>  {
>      int ret;
>      int state = s->state;
> +    Error *local_error = NULL;
>  
>      if (state == MIGRATION_STATUS_CANCELLING ||
>          state == MIGRATION_STATUS_CANCELLED) {
> @@ -2957,13 +2958,18 @@ static MigThrError migration_detect_error(MigrationState *s)
>      }
>  
>      /* Try to detect any file errors */
> -    ret = qemu_file_get_error(s->to_dst_file);
> -
> +    ret = qemu_file_get_error_obj(s->to_dst_file, &local_error);
>      if (!ret) {
>          /* Everything is fine */
> +        assert(!local_error);
>          return MIG_THR_ERR_NONE;
>      }
>  
> +    if (local_error) {
> +        migrate_set_error(s, local_error);
> +        error_free(local_error);
> +    }
> +
>      if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret == -EIO) {
>          /*
>           * For postcopy, we allow the network to be down for a
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 8e639eb496..c382ea2d78 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -33,7 +33,8 @@
>  static ssize_t channel_writev_buffer(void *opaque,
>                                       struct iovec *iov,
>                                       int iovcnt,
> -                                     int64_t pos)
> +                                     int64_t pos,
> +                                     Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>      ssize_t done = 0;
> @@ -47,7 +48,7 @@ static ssize_t channel_writev_buffer(void *opaque,
>  
>      while (nlocal_iov > 0) {
>          ssize_t len;
> -        len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
> +        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);
> @@ -57,7 +58,6 @@ static ssize_t channel_writev_buffer(void *opaque,
>              continue;
>          }
>          if (len < 0) {
> -            /* XXX handle Error objects */
>              done = -EIO;
>              goto cleanup;
>          }
> @@ -75,13 +75,14 @@ static ssize_t channel_writev_buffer(void *opaque,
>  static ssize_t channel_get_buffer(void *opaque,
>                                    uint8_t *buf,
>                                    int64_t pos,
> -                                  size_t size)
> +                                  size_t size,
> +                                  Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>      ssize_t ret;
>  
>      do {
> -        ret = qio_channel_read(ioc, (char *)buf, size, NULL);
> +        ret = qio_channel_read(ioc, (char *)buf, size, errp);
>          if (ret < 0) {
>              if (ret == QIO_CHANNEL_ERR_BLOCK) {
>                  if (qemu_in_coroutine()) {
> @@ -90,7 +91,6 @@ static ssize_t channel_get_buffer(void *opaque,
>                      qio_channel_wait(ioc, G_IO_IN);
>                  }
>              } else {
> -                /* XXX handle Error * object */
>                  return -EIO;
>              }
>          }
> @@ -100,18 +100,20 @@ static ssize_t channel_get_buffer(void *opaque,
>  }
>  
>  
> -static int channel_close(void *opaque)
> +static int channel_close(void *opaque, Error **errp)
>  {
> +    int ret;
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
> -    qio_channel_close(ioc, NULL);
> +    ret = qio_channel_close(ioc, errp);
>      object_unref(OBJECT(ioc));
> -    return 0;
> +    return ret;
>  }
>  
>  
>  static int channel_shutdown(void *opaque,
>                              bool rd,
> -                            bool wr)
> +                            bool wr,
> +                            Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>  
> @@ -125,8 +127,7 @@ static int channel_shutdown(void *opaque,
>          } else {
>              mode = QIO_CHANNEL_SHUTDOWN_WRITE;
>          }
> -        if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
> -            /* XXX handler Error * object */
> +        if (qio_channel_shutdown(ioc, mode, errp) < 0) {
>              return -EIO;
>          }
>      }
> @@ -135,11 +136,12 @@ static int channel_shutdown(void *opaque,
>  
>  
>  static int channel_set_blocking(void *opaque,
> -                                bool enabled)
> +                                bool enabled,
> +                                Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>  
> -    if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
> +    if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
>          return -1;
>      }
>      return 0;
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 977b9ae07c..c52160e08b 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -29,6 +29,7 @@
>  #include "migration.h"
>  #include "qemu-file.h"
>  #include "trace.h"
> +#include "qapi/error.h"
>  
>  #define IO_BUF_SIZE 32768
>  #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
> @@ -52,6 +53,7 @@ struct QEMUFile {
>      unsigned int iovcnt;
>  
>      int last_error;
> +    Error *last_error_obj;
>  };
>  
>  /*
> @@ -63,7 +65,7 @@ int qemu_file_shutdown(QEMUFile *f)
>      if (!f->ops->shut_down) {
>          return -ENOSYS;
>      }
> -    return f->ops->shut_down(f->opaque, true, true);
> +    return f->ops->shut_down(f->opaque, true, true, NULL);
>  }
>  
>  /*
> @@ -108,24 +110,55 @@ void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
>  }
>  
>  /*
> - * Get last error for stream f
> + * Get last error for stream f with optional Error*
>   *
>   * Return negative error value if there has been an error on previous
>   * operations, return 0 if no error happened.
> + * Optional, it returns Error* in errp, but it may be NULL even if return value
> + * is not 0.
>   *
>   */
> -int qemu_file_get_error(QEMUFile *f)
> +int qemu_file_get_error_obj(QEMUFile *f, Error **errp)
>  {
> +    if (errp) {
> +        *errp = f->last_error_obj ? error_copy(f->last_error_obj) : NULL;
> +    }
>      return f->last_error;
>  }
>  
> -void qemu_file_set_error(QEMUFile *f, int ret)
> +/*
> + * Set the last error for stream f with optional Error*
> + */
> +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
>  {
> -    if (f->last_error == 0) {
> +    if (f->last_error == 0 && ret) {
>          f->last_error = ret;
> +        error_propagate(&f->last_error_obj, err);
> +    } else if (err) {
> +        error_report_err(err);
>      }
>  }
>  
> +/*
> + * Get last error for stream f
> + *
> + * Return negative error value if there has been an error on previous
> + * operations, return 0 if no error happened.
> + *
> + */
> +int qemu_file_get_error(QEMUFile *f)
> +{
> +    return qemu_file_get_error_obj(f, NULL);
> +}
> +
> +/*
> + * Set the last error for stream f
> + */
> +void qemu_file_set_error(QEMUFile *f, int ret)
> +{
> +    qemu_file_set_error_obj(f, ret, NULL);
> +}
> +
>  bool qemu_file_is_writable(QEMUFile *f)
>  {
>      return f->ops->writev_buffer;
> @@ -177,6 +210,7 @@ void qemu_fflush(QEMUFile *f)
>  {
>      ssize_t ret = 0;
>      ssize_t expect = 0;
> +    Error *local_error = NULL;
>  
>      if (!qemu_file_is_writable(f)) {
>          return;
> @@ -184,7 +218,8 @@ void qemu_fflush(QEMUFile *f)
>  
>      if (f->iovcnt > 0) {
>          expect = iov_size(f->iov, f->iovcnt);
> -        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
> +        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> +                                    &local_error);
>  
>          qemu_iovec_release_ram(f);
>      }
> @@ -196,7 +231,7 @@ void qemu_fflush(QEMUFile *f)
>       * data set we requested, so sanity check that.
>       */
>      if (ret != expect) {
> -        qemu_file_set_error(f, ret < 0 ? ret : -EIO);
> +        qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
>      }
>      f->buf_index = 0;
>      f->iovcnt = 0;
> @@ -284,6 +319,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>  {
>      int len;
>      int pending;
> +    Error *local_error = NULL;
>  
>      assert(!qemu_file_is_writable(f));
>  
> @@ -295,14 +331,16 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>      f->buf_size = pending;
>  
>      len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> -                        IO_BUF_SIZE - pending);
> +                             IO_BUF_SIZE - pending, &local_error);
>      if (len > 0) {
>          f->buf_size += len;
>          f->pos += len;
>      } else if (len == 0) {
> -        qemu_file_set_error(f, -EIO);
> +        qemu_file_set_error_obj(f, -EIO, local_error);
>      } else if (len != -EAGAIN) {
> -        qemu_file_set_error(f, len);
> +        qemu_file_set_error_obj(f, len, local_error);
> +    } else {
> +        error_free(local_error);
>      }
>  
>      return len;
> @@ -328,7 +366,7 @@ int qemu_fclose(QEMUFile *f)
>      ret = qemu_file_get_error(f);
>  
>      if (f->ops->close) {
> -        int ret2 = f->ops->close(f->opaque);
> +        int ret2 = f->ops->close(f->opaque, NULL);
>          if (ret >= 0) {
>              ret = ret2;
>          }
> @@ -339,6 +377,7 @@ int qemu_fclose(QEMUFile *f)
>      if (f->last_error) {
>          ret = f->last_error;
>      }
> +    error_free(f->last_error_obj);
>      g_free(f);
>      trace_qemu_file_fclose();
>      return ret;
> @@ -784,6 +823,6 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
>  void qemu_file_set_blocking(QEMUFile *f, bool block)
>  {
>      if (f->ops->set_blocking) {
> -        f->ops->set_blocking(f->opaque, block);
> +        f->ops->set_blocking(f->opaque, block, NULL);
>      }
>  }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 13baf896bd..eb886db65f 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -32,7 +32,8 @@
>   * bytes actually read should be returned.
>   */
>  typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> -                                        int64_t pos, size_t size);
> +                                        int64_t pos, size_t size,
> +                                        Error **errp);
>  
>  /* Close a file
>   *
> @@ -41,7 +42,7 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
>   * The meaning of return value on success depends on the specific back-end being
>   * used.
>   */
> -typedef int (QEMUFileCloseFunc)(void *opaque);
> +typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
>  
>  /* Called to return the OS file descriptor associated to the QEMUFile.
>   */
> @@ -49,14 +50,15 @@ typedef int (QEMUFileGetFD)(void *opaque);
>  
>  /* Called to change the blocking mode of the file
>   */
> -typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
> +typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
>  
>  /*
>   * 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);
> +                                           int iovcnt, int64_t pos,
> +                                           Error **errp);
>  
>  /*
>   * This function provides hooks around different
> @@ -97,7 +99,8 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>   * Existing blocking reads/writes must be woken
>   * Returns 0 on success, -err on error
>   */
> -typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
> +typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
> +                                   Error **errp);
>  
>  typedef struct QEMUFileOps {
>      QEMUFileGetBufferFunc *get_buffer;
> @@ -149,6 +152,8 @@ void qemu_update_position(QEMUFile *f, size_t size);
>  void qemu_file_reset_rate_limit(QEMUFile *f);
>  void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>  int64_t qemu_file_get_rate_limit(QEMUFile *f);
> +int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
> +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
>  void qemu_file_set_error(QEMUFile *f, int ret);
>  int qemu_file_shutdown(QEMUFile *f);
>  QEMUFile *qemu_file_get_return_path(QEMUFile *f);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 34bcad3807..a619af744d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -124,7 +124,7 @@ static struct mig_cmd_args {
>  /* savevm/loadvm support */
>  
>  static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> -                                   int64_t pos)
> +                                   int64_t pos, Error **errp)
>  {
>      int ret;
>      QEMUIOVector qiov;
> @@ -139,12 +139,12 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>  }
>  
>  static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> -                                size_t size)
> +                                size_t size, Error **errp)
>  {
>      return bdrv_load_vmstate(opaque, buf, pos, size);
>  }
>  
> -static int bdrv_fclose(void *opaque)
> +static int bdrv_fclose(void *opaque, Error **errp)
>  {
>      return bdrv_flush(opaque);
>  }
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Yury Kotov July 17, 2019, 10:19 a.m. UTC | #3
Hi, I'm a bit worried that this patch might have been forgotten.
Is it queued? Thanks!

14.06.2019, 19:56, "Dr. David Alan Gilbert" <dgilbert@redhat.com>:
> * Yury Kotov (yury-kotov@yandex-team.ru) wrote:
>>  Currently, there is no information about error if outgoing migration was failed
>>  because of file channel errors.
>>  Example (QMP session):
>>  -> { "execute": "migrate", "arguments": { "uri": "exec:head -c 1" }}
>>  <- { "return": {} }
>>  ...
>>  -> { "execute": "query-migrate" }
>>  <- { "return": { "status": "failed" }} // There is not error's description
>>
>>  And even in the QEMU's output there is nothing.
>>
>>  This patch
>>  1) Adds errp for the most of QEMUFileOps
>>  2) Adds qemu_file_get_error_obj/qemu_file_set_error_obj
>>  3) And finally using of qemu_file_get_error_obj in migration.c
>>
>>  And now, the status for the mentioned fail will be:
>>  -> { "execute": "query-migrate" }
>>  <- { "return": { "status": "failed",
>>                   "error-desc": "Unable to write to command: Broken pipe" }}
>>
>>  Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>>  ---
>>   migration/migration.c | 10 ++++--
>>   migration/qemu-file-channel.c | 30 +++++++++--------
>>   migration/qemu-file.c | 63 ++++++++++++++++++++++++++++-------
>>   migration/qemu-file.h | 15 ++++++---
>>   migration/savevm.c | 6 ++--
>>   5 files changed, 88 insertions(+), 36 deletions(-)
>>
>>  diff --git a/migration/migration.c b/migration/migration.c
>>  index 609e0df5d0..7bcdc4613b 100644
>>  --- a/migration/migration.c
>>  +++ b/migration/migration.c
>>  @@ -2949,6 +2949,7 @@ static MigThrError migration_detect_error(MigrationState *s)
>>   {
>>       int ret;
>>       int state = s->state;
>>  + Error *local_error = NULL;
>>
>>       if (state == MIGRATION_STATUS_CANCELLING ||
>>           state == MIGRATION_STATUS_CANCELLED) {
>>  @@ -2957,13 +2958,18 @@ static MigThrError migration_detect_error(MigrationState *s)
>>       }
>>
>>       /* Try to detect any file errors */
>>  - ret = qemu_file_get_error(s->to_dst_file);
>>  -
>>  + ret = qemu_file_get_error_obj(s->to_dst_file, &local_error);
>>       if (!ret) {
>>           /* Everything is fine */
>>  + assert(!local_error);
>>           return MIG_THR_ERR_NONE;
>>       }
>>
>>  + if (local_error) {
>>  + migrate_set_error(s, local_error);
>>  + error_free(local_error);
>>  + }
>>  +
>>       if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret == -EIO) {
>>           /*
>>            * For postcopy, we allow the network to be down for a
>>  diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
>>  index 8e639eb496..c382ea2d78 100644
>>  --- a/migration/qemu-file-channel.c
>>  +++ b/migration/qemu-file-channel.c
>>  @@ -33,7 +33,8 @@
>>   static ssize_t channel_writev_buffer(void *opaque,
>>                                        struct iovec *iov,
>>                                        int iovcnt,
>>  - int64_t pos)
>>  + int64_t pos,
>>  + Error **errp)
>>   {
>>       QIOChannel *ioc = QIO_CHANNEL(opaque);
>>       ssize_t done = 0;
>>  @@ -47,7 +48,7 @@ static ssize_t channel_writev_buffer(void *opaque,
>>
>>       while (nlocal_iov > 0) {
>>           ssize_t len;
>>  - len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
>>  + 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);
>>  @@ -57,7 +58,6 @@ static ssize_t channel_writev_buffer(void *opaque,
>>               continue;
>>           }
>>           if (len < 0) {
>>  - /* XXX handle Error objects */
>>               done = -EIO;
>>               goto cleanup;
>>           }
>>  @@ -75,13 +75,14 @@ static ssize_t channel_writev_buffer(void *opaque,
>>   static ssize_t channel_get_buffer(void *opaque,
>>                                     uint8_t *buf,
>>                                     int64_t pos,
>>  - size_t size)
>>  + size_t size,
>>  + Error **errp)
>>   {
>>       QIOChannel *ioc = QIO_CHANNEL(opaque);
>>       ssize_t ret;
>>
>>       do {
>>  - ret = qio_channel_read(ioc, (char *)buf, size, NULL);
>>  + ret = qio_channel_read(ioc, (char *)buf, size, errp);
>>           if (ret < 0) {
>>               if (ret == QIO_CHANNEL_ERR_BLOCK) {
>>                   if (qemu_in_coroutine()) {
>>  @@ -90,7 +91,6 @@ static ssize_t channel_get_buffer(void *opaque,
>>                       qio_channel_wait(ioc, G_IO_IN);
>>                   }
>>               } else {
>>  - /* XXX handle Error * object */
>>                   return -EIO;
>>               }
>>           }
>>  @@ -100,18 +100,20 @@ static ssize_t channel_get_buffer(void *opaque,
>>   }
>>
>>  -static int channel_close(void *opaque)
>>  +static int channel_close(void *opaque, Error **errp)
>>   {
>>  + int ret;
>>       QIOChannel *ioc = QIO_CHANNEL(opaque);
>>  - qio_channel_close(ioc, NULL);
>>  + ret = qio_channel_close(ioc, errp);
>>       object_unref(OBJECT(ioc));
>>  - return 0;
>>  + return ret;
>>   }
>>
>>   static int channel_shutdown(void *opaque,
>>                               bool rd,
>>  - bool wr)
>>  + bool wr,
>>  + Error **errp)
>>   {
>>       QIOChannel *ioc = QIO_CHANNEL(opaque);
>>
>>  @@ -125,8 +127,7 @@ static int channel_shutdown(void *opaque,
>>           } else {
>>               mode = QIO_CHANNEL_SHUTDOWN_WRITE;
>>           }
>>  - if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
>>  - /* XXX handler Error * object */
>>  + if (qio_channel_shutdown(ioc, mode, errp) < 0) {
>>               return -EIO;
>>           }
>>       }
>>  @@ -135,11 +136,12 @@ static int channel_shutdown(void *opaque,
>>
>>   static int channel_set_blocking(void *opaque,
>>  - bool enabled)
>>  + bool enabled,
>>  + Error **errp)
>>   {
>>       QIOChannel *ioc = QIO_CHANNEL(opaque);
>>
>>  - if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
>>  + if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
>>           return -1;
>>       }
>>       return 0;
>>  diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>>  index 977b9ae07c..c52160e08b 100644
>>  --- a/migration/qemu-file.c
>>  +++ b/migration/qemu-file.c
>>  @@ -29,6 +29,7 @@
>>   #include "migration.h"
>>   #include "qemu-file.h"
>>   #include "trace.h"
>>  +#include "qapi/error.h"
>>
>>   #define IO_BUF_SIZE 32768
>>   #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
>>  @@ -52,6 +53,7 @@ struct QEMUFile {
>>       unsigned int iovcnt;
>>
>>       int last_error;
>>  + Error *last_error_obj;
>>   };
>>
>>   /*
>>  @@ -63,7 +65,7 @@ int qemu_file_shutdown(QEMUFile *f)
>>       if (!f->ops->shut_down) {
>>           return -ENOSYS;
>>       }
>>  - return f->ops->shut_down(f->opaque, true, true);
>>  + return f->ops->shut_down(f->opaque, true, true, NULL);
>>   }
>>
>>   /*
>>  @@ -108,24 +110,55 @@ void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
>>   }
>>
>>   /*
>>  - * Get last error for stream f
>>  + * Get last error for stream f with optional Error*
>>    *
>>    * Return negative error value if there has been an error on previous
>>    * operations, return 0 if no error happened.
>>  + * Optional, it returns Error* in errp, but it may be NULL even if return value
>>  + * is not 0.
>>    *
>>    */
>>  -int qemu_file_get_error(QEMUFile *f)
>>  +int qemu_file_get_error_obj(QEMUFile *f, Error **errp)
>>   {
>>  + if (errp) {
>>  + *errp = f->last_error_obj ? error_copy(f->last_error_obj) : NULL;
>>  + }
>>       return f->last_error;
>>   }
>>
>>  -void qemu_file_set_error(QEMUFile *f, int ret)
>>  +/*
>>  + * Set the last error for stream f with optional Error*
>>  + */
>>  +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
>>   {
>>  - if (f->last_error == 0) {
>>  + if (f->last_error == 0 && ret) {
>>           f->last_error = ret;
>>  + error_propagate(&f->last_error_obj, err);
>>  + } else if (err) {
>>  + error_report_err(err);
>>       }
>>   }
>>
>>  +/*
>>  + * Get last error for stream f
>>  + *
>>  + * Return negative error value if there has been an error on previous
>>  + * operations, return 0 if no error happened.
>>  + *
>>  + */
>>  +int qemu_file_get_error(QEMUFile *f)
>>  +{
>>  + return qemu_file_get_error_obj(f, NULL);
>>  +}
>>  +
>>  +/*
>>  + * Set the last error for stream f
>>  + */
>>  +void qemu_file_set_error(QEMUFile *f, int ret)
>>  +{
>>  + qemu_file_set_error_obj(f, ret, NULL);
>>  +}
>>  +
>>   bool qemu_file_is_writable(QEMUFile *f)
>>   {
>>       return f->ops->writev_buffer;
>>  @@ -177,6 +210,7 @@ void qemu_fflush(QEMUFile *f)
>>   {
>>       ssize_t ret = 0;
>>       ssize_t expect = 0;
>>  + Error *local_error = NULL;
>>
>>       if (!qemu_file_is_writable(f)) {
>>           return;
>>  @@ -184,7 +218,8 @@ void qemu_fflush(QEMUFile *f)
>>
>>       if (f->iovcnt > 0) {
>>           expect = iov_size(f->iov, f->iovcnt);
>>  - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
>>  + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
>>  + &local_error);
>>
>>           qemu_iovec_release_ram(f);
>>       }
>>  @@ -196,7 +231,7 @@ void qemu_fflush(QEMUFile *f)
>>        * data set we requested, so sanity check that.
>>        */
>>       if (ret != expect) {
>>  - qemu_file_set_error(f, ret < 0 ? ret : -EIO);
>>  + qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
>>       }
>>       f->buf_index = 0;
>>       f->iovcnt = 0;
>>  @@ -284,6 +319,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>>   {
>>       int len;
>>       int pending;
>>  + Error *local_error = NULL;
>>
>>       assert(!qemu_file_is_writable(f));
>>
>>  @@ -295,14 +331,16 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>>       f->buf_size = pending;
>>
>>       len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
>>  - IO_BUF_SIZE - pending);
>>  + IO_BUF_SIZE - pending, &local_error);
>>       if (len > 0) {
>>           f->buf_size += len;
>>           f->pos += len;
>>       } else if (len == 0) {
>>  - qemu_file_set_error(f, -EIO);
>>  + qemu_file_set_error_obj(f, -EIO, local_error);
>>       } else if (len != -EAGAIN) {
>>  - qemu_file_set_error(f, len);
>>  + qemu_file_set_error_obj(f, len, local_error);
>>  + } else {
>>  + error_free(local_error);
>>       }
>>
>>       return len;
>>  @@ -328,7 +366,7 @@ int qemu_fclose(QEMUFile *f)
>>       ret = qemu_file_get_error(f);
>>
>>       if (f->ops->close) {
>>  - int ret2 = f->ops->close(f->opaque);
>>  + int ret2 = f->ops->close(f->opaque, NULL);
>>           if (ret >= 0) {
>>               ret = ret2;
>>           }
>>  @@ -339,6 +377,7 @@ int qemu_fclose(QEMUFile *f)
>>       if (f->last_error) {
>>           ret = f->last_error;
>>       }
>>  + error_free(f->last_error_obj);
>>       g_free(f);
>>       trace_qemu_file_fclose();
>>       return ret;
>>  @@ -784,6 +823,6 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
>>   void qemu_file_set_blocking(QEMUFile *f, bool block)
>>   {
>>       if (f->ops->set_blocking) {
>>  - f->ops->set_blocking(f->opaque, block);
>>  + f->ops->set_blocking(f->opaque, block, NULL);
>>       }
>>   }
>>  diff --git a/migration/qemu-file.h b/migration/qemu-file.h
>>  index 13baf896bd..eb886db65f 100644
>>  --- a/migration/qemu-file.h
>>  +++ b/migration/qemu-file.h
>>  @@ -32,7 +32,8 @@
>>    * bytes actually read should be returned.
>>    */
>>   typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
>>  - int64_t pos, size_t size);
>>  + int64_t pos, size_t size,
>>  + Error **errp);
>>
>>   /* Close a file
>>    *
>>  @@ -41,7 +42,7 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
>>    * The meaning of return value on success depends on the specific back-end being
>>    * used.
>>    */
>>  -typedef int (QEMUFileCloseFunc)(void *opaque);
>>  +typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
>>
>>   /* Called to return the OS file descriptor associated to the QEMUFile.
>>    */
>>  @@ -49,14 +50,15 @@ typedef int (QEMUFileGetFD)(void *opaque);
>>
>>   /* Called to change the blocking mode of the file
>>    */
>>  -typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
>>  +typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
>>
>>   /*
>>    * 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);
>>  + int iovcnt, int64_t pos,
>>  + Error **errp);
>>
>>   /*
>>    * This function provides hooks around different
>>  @@ -97,7 +99,8 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>>    * Existing blocking reads/writes must be woken
>>    * Returns 0 on success, -err on error
>>    */
>>  -typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
>>  +typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
>>  + Error **errp);
>>
>>   typedef struct QEMUFileOps {
>>       QEMUFileGetBufferFunc *get_buffer;
>>  @@ -149,6 +152,8 @@ void qemu_update_position(QEMUFile *f, size_t size);
>>   void qemu_file_reset_rate_limit(QEMUFile *f);
>>   void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>>   int64_t qemu_file_get_rate_limit(QEMUFile *f);
>>  +int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
>>  +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
>>   void qemu_file_set_error(QEMUFile *f, int ret);
>>   int qemu_file_shutdown(QEMUFile *f);
>>   QEMUFile *qemu_file_get_return_path(QEMUFile *f);
>>  diff --git a/migration/savevm.c b/migration/savevm.c
>>  index 34bcad3807..a619af744d 100644
>>  --- a/migration/savevm.c
>>  +++ b/migration/savevm.c
>>  @@ -124,7 +124,7 @@ static struct mig_cmd_args {
>>   /* savevm/loadvm support */
>>
>>   static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>>  - int64_t pos)
>>  + int64_t pos, Error **errp)
>>   {
>>       int ret;
>>       QEMUIOVector qiov;
>>  @@ -139,12 +139,12 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>>   }
>>
>>   static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
>>  - size_t size)
>>  + size_t size, Error **errp)
>>   {
>>       return bdrv_load_vmstate(opaque, buf, pos, size);
>>   }
>>
>>  -static int bdrv_fclose(void *opaque)
>>  +static int bdrv_fclose(void *opaque, Error **errp)
>>   {
>>       return bdrv_flush(opaque);
>>   }
>>  --
>>  2.21.0
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Regards,
Yury
Dr. David Alan Gilbert Aug. 6, 2019, 3:51 p.m. UTC | #4
* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Hi, I'm a bit worried that this patch might have been forgotten.
> Is it queued? Thanks!

I've added it to my list for the pull I'll do as soon as 4.2 opens.

Dave

> 14.06.2019, 19:56, "Dr. David Alan Gilbert" <dgilbert@redhat.com>:
> > * Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> >>  Currently, there is no information about error if outgoing migration was failed
> >>  because of file channel errors.
> >>  Example (QMP session):
> >>  -> { "execute": "migrate", "arguments": { "uri": "exec:head -c 1" }}
> >>  <- { "return": {} }
> >>  ...
> >>  -> { "execute": "query-migrate" }
> >>  <- { "return": { "status": "failed" }} // There is not error's description
> >>
> >>  And even in the QEMU's output there is nothing.
> >>
> >>  This patch
> >>  1) Adds errp for the most of QEMUFileOps
> >>  2) Adds qemu_file_get_error_obj/qemu_file_set_error_obj
> >>  3) And finally using of qemu_file_get_error_obj in migration.c
> >>
> >>  And now, the status for the mentioned fail will be:
> >>  -> { "execute": "query-migrate" }
> >>  <- { "return": { "status": "failed",
> >>                   "error-desc": "Unable to write to command: Broken pipe" }}
> >>
> >>  Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> >>  ---
> >>   migration/migration.c | 10 ++++--
> >>   migration/qemu-file-channel.c | 30 +++++++++--------
> >>   migration/qemu-file.c | 63 ++++++++++++++++++++++++++++-------
> >>   migration/qemu-file.h | 15 ++++++---
> >>   migration/savevm.c | 6 ++--
> >>   5 files changed, 88 insertions(+), 36 deletions(-)
> >>
> >>  diff --git a/migration/migration.c b/migration/migration.c
> >>  index 609e0df5d0..7bcdc4613b 100644
> >>  --- a/migration/migration.c
> >>  +++ b/migration/migration.c
> >>  @@ -2949,6 +2949,7 @@ static MigThrError migration_detect_error(MigrationState *s)
> >>   {
> >>       int ret;
> >>       int state = s->state;
> >>  + Error *local_error = NULL;
> >>
> >>       if (state == MIGRATION_STATUS_CANCELLING ||
> >>           state == MIGRATION_STATUS_CANCELLED) {
> >>  @@ -2957,13 +2958,18 @@ static MigThrError migration_detect_error(MigrationState *s)
> >>       }
> >>
> >>       /* Try to detect any file errors */
> >>  - ret = qemu_file_get_error(s->to_dst_file);
> >>  -
> >>  + ret = qemu_file_get_error_obj(s->to_dst_file, &local_error);
> >>       if (!ret) {
> >>           /* Everything is fine */
> >>  + assert(!local_error);
> >>           return MIG_THR_ERR_NONE;
> >>       }
> >>
> >>  + if (local_error) {
> >>  + migrate_set_error(s, local_error);
> >>  + error_free(local_error);
> >>  + }
> >>  +
> >>       if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret == -EIO) {
> >>           /*
> >>            * For postcopy, we allow the network to be down for a
> >>  diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> >>  index 8e639eb496..c382ea2d78 100644
> >>  --- a/migration/qemu-file-channel.c
> >>  +++ b/migration/qemu-file-channel.c
> >>  @@ -33,7 +33,8 @@
> >>   static ssize_t channel_writev_buffer(void *opaque,
> >>                                        struct iovec *iov,
> >>                                        int iovcnt,
> >>  - int64_t pos)
> >>  + int64_t pos,
> >>  + Error **errp)
> >>   {
> >>       QIOChannel *ioc = QIO_CHANNEL(opaque);
> >>       ssize_t done = 0;
> >>  @@ -47,7 +48,7 @@ static ssize_t channel_writev_buffer(void *opaque,
> >>
> >>       while (nlocal_iov > 0) {
> >>           ssize_t len;
> >>  - len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
> >>  + 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);
> >>  @@ -57,7 +58,6 @@ static ssize_t channel_writev_buffer(void *opaque,
> >>               continue;
> >>           }
> >>           if (len < 0) {
> >>  - /* XXX handle Error objects */
> >>               done = -EIO;
> >>               goto cleanup;
> >>           }
> >>  @@ -75,13 +75,14 @@ static ssize_t channel_writev_buffer(void *opaque,
> >>   static ssize_t channel_get_buffer(void *opaque,
> >>                                     uint8_t *buf,
> >>                                     int64_t pos,
> >>  - size_t size)
> >>  + size_t size,
> >>  + Error **errp)
> >>   {
> >>       QIOChannel *ioc = QIO_CHANNEL(opaque);
> >>       ssize_t ret;
> >>
> >>       do {
> >>  - ret = qio_channel_read(ioc, (char *)buf, size, NULL);
> >>  + ret = qio_channel_read(ioc, (char *)buf, size, errp);
> >>           if (ret < 0) {
> >>               if (ret == QIO_CHANNEL_ERR_BLOCK) {
> >>                   if (qemu_in_coroutine()) {
> >>  @@ -90,7 +91,6 @@ static ssize_t channel_get_buffer(void *opaque,
> >>                       qio_channel_wait(ioc, G_IO_IN);
> >>                   }
> >>               } else {
> >>  - /* XXX handle Error * object */
> >>                   return -EIO;
> >>               }
> >>           }
> >>  @@ -100,18 +100,20 @@ static ssize_t channel_get_buffer(void *opaque,
> >>   }
> >>
> >>  -static int channel_close(void *opaque)
> >>  +static int channel_close(void *opaque, Error **errp)
> >>   {
> >>  + int ret;
> >>       QIOChannel *ioc = QIO_CHANNEL(opaque);
> >>  - qio_channel_close(ioc, NULL);
> >>  + ret = qio_channel_close(ioc, errp);
> >>       object_unref(OBJECT(ioc));
> >>  - return 0;
> >>  + return ret;
> >>   }
> >>
> >>   static int channel_shutdown(void *opaque,
> >>                               bool rd,
> >>  - bool wr)
> >>  + bool wr,
> >>  + Error **errp)
> >>   {
> >>       QIOChannel *ioc = QIO_CHANNEL(opaque);
> >>
> >>  @@ -125,8 +127,7 @@ static int channel_shutdown(void *opaque,
> >>           } else {
> >>               mode = QIO_CHANNEL_SHUTDOWN_WRITE;
> >>           }
> >>  - if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
> >>  - /* XXX handler Error * object */
> >>  + if (qio_channel_shutdown(ioc, mode, errp) < 0) {
> >>               return -EIO;
> >>           }
> >>       }
> >>  @@ -135,11 +136,12 @@ static int channel_shutdown(void *opaque,
> >>
> >>   static int channel_set_blocking(void *opaque,
> >>  - bool enabled)
> >>  + bool enabled,
> >>  + Error **errp)
> >>   {
> >>       QIOChannel *ioc = QIO_CHANNEL(opaque);
> >>
> >>  - if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
> >>  + if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
> >>           return -1;
> >>       }
> >>       return 0;
> >>  diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> >>  index 977b9ae07c..c52160e08b 100644
> >>  --- a/migration/qemu-file.c
> >>  +++ b/migration/qemu-file.c
> >>  @@ -29,6 +29,7 @@
> >>   #include "migration.h"
> >>   #include "qemu-file.h"
> >>   #include "trace.h"
> >>  +#include "qapi/error.h"
> >>
> >>   #define IO_BUF_SIZE 32768
> >>   #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
> >>  @@ -52,6 +53,7 @@ struct QEMUFile {
> >>       unsigned int iovcnt;
> >>
> >>       int last_error;
> >>  + Error *last_error_obj;
> >>   };
> >>
> >>   /*
> >>  @@ -63,7 +65,7 @@ int qemu_file_shutdown(QEMUFile *f)
> >>       if (!f->ops->shut_down) {
> >>           return -ENOSYS;
> >>       }
> >>  - return f->ops->shut_down(f->opaque, true, true);
> >>  + return f->ops->shut_down(f->opaque, true, true, NULL);
> >>   }
> >>
> >>   /*
> >>  @@ -108,24 +110,55 @@ void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
> >>   }
> >>
> >>   /*
> >>  - * Get last error for stream f
> >>  + * Get last error for stream f with optional Error*
> >>    *
> >>    * Return negative error value if there has been an error on previous
> >>    * operations, return 0 if no error happened.
> >>  + * Optional, it returns Error* in errp, but it may be NULL even if return value
> >>  + * is not 0.
> >>    *
> >>    */
> >>  -int qemu_file_get_error(QEMUFile *f)
> >>  +int qemu_file_get_error_obj(QEMUFile *f, Error **errp)
> >>   {
> >>  + if (errp) {
> >>  + *errp = f->last_error_obj ? error_copy(f->last_error_obj) : NULL;
> >>  + }
> >>       return f->last_error;
> >>   }
> >>
> >>  -void qemu_file_set_error(QEMUFile *f, int ret)
> >>  +/*
> >>  + * Set the last error for stream f with optional Error*
> >>  + */
> >>  +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
> >>   {
> >>  - if (f->last_error == 0) {
> >>  + if (f->last_error == 0 && ret) {
> >>           f->last_error = ret;
> >>  + error_propagate(&f->last_error_obj, err);
> >>  + } else if (err) {
> >>  + error_report_err(err);
> >>       }
> >>   }
> >>
> >>  +/*
> >>  + * Get last error for stream f
> >>  + *
> >>  + * Return negative error value if there has been an error on previous
> >>  + * operations, return 0 if no error happened.
> >>  + *
> >>  + */
> >>  +int qemu_file_get_error(QEMUFile *f)
> >>  +{
> >>  + return qemu_file_get_error_obj(f, NULL);
> >>  +}
> >>  +
> >>  +/*
> >>  + * Set the last error for stream f
> >>  + */
> >>  +void qemu_file_set_error(QEMUFile *f, int ret)
> >>  +{
> >>  + qemu_file_set_error_obj(f, ret, NULL);
> >>  +}
> >>  +
> >>   bool qemu_file_is_writable(QEMUFile *f)
> >>   {
> >>       return f->ops->writev_buffer;
> >>  @@ -177,6 +210,7 @@ void qemu_fflush(QEMUFile *f)
> >>   {
> >>       ssize_t ret = 0;
> >>       ssize_t expect = 0;
> >>  + Error *local_error = NULL;
> >>
> >>       if (!qemu_file_is_writable(f)) {
> >>           return;
> >>  @@ -184,7 +218,8 @@ void qemu_fflush(QEMUFile *f)
> >>
> >>       if (f->iovcnt > 0) {
> >>           expect = iov_size(f->iov, f->iovcnt);
> >>  - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
> >>  + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> >>  + &local_error);
> >>
> >>           qemu_iovec_release_ram(f);
> >>       }
> >>  @@ -196,7 +231,7 @@ void qemu_fflush(QEMUFile *f)
> >>        * data set we requested, so sanity check that.
> >>        */
> >>       if (ret != expect) {
> >>  - qemu_file_set_error(f, ret < 0 ? ret : -EIO);
> >>  + qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
> >>       }
> >>       f->buf_index = 0;
> >>       f->iovcnt = 0;
> >>  @@ -284,6 +319,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> >>   {
> >>       int len;
> >>       int pending;
> >>  + Error *local_error = NULL;
> >>
> >>       assert(!qemu_file_is_writable(f));
> >>
> >>  @@ -295,14 +331,16 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> >>       f->buf_size = pending;
> >>
> >>       len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> >>  - IO_BUF_SIZE - pending);
> >>  + IO_BUF_SIZE - pending, &local_error);
> >>       if (len > 0) {
> >>           f->buf_size += len;
> >>           f->pos += len;
> >>       } else if (len == 0) {
> >>  - qemu_file_set_error(f, -EIO);
> >>  + qemu_file_set_error_obj(f, -EIO, local_error);
> >>       } else if (len != -EAGAIN) {
> >>  - qemu_file_set_error(f, len);
> >>  + qemu_file_set_error_obj(f, len, local_error);
> >>  + } else {
> >>  + error_free(local_error);
> >>       }
> >>
> >>       return len;
> >>  @@ -328,7 +366,7 @@ int qemu_fclose(QEMUFile *f)
> >>       ret = qemu_file_get_error(f);
> >>
> >>       if (f->ops->close) {
> >>  - int ret2 = f->ops->close(f->opaque);
> >>  + int ret2 = f->ops->close(f->opaque, NULL);
> >>           if (ret >= 0) {
> >>               ret = ret2;
> >>           }
> >>  @@ -339,6 +377,7 @@ int qemu_fclose(QEMUFile *f)
> >>       if (f->last_error) {
> >>           ret = f->last_error;
> >>       }
> >>  + error_free(f->last_error_obj);
> >>       g_free(f);
> >>       trace_qemu_file_fclose();
> >>       return ret;
> >>  @@ -784,6 +823,6 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
> >>   void qemu_file_set_blocking(QEMUFile *f, bool block)
> >>   {
> >>       if (f->ops->set_blocking) {
> >>  - f->ops->set_blocking(f->opaque, block);
> >>  + f->ops->set_blocking(f->opaque, block, NULL);
> >>       }
> >>   }
> >>  diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> >>  index 13baf896bd..eb886db65f 100644
> >>  --- a/migration/qemu-file.h
> >>  +++ b/migration/qemu-file.h
> >>  @@ -32,7 +32,8 @@
> >>    * bytes actually read should be returned.
> >>    */
> >>   typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> >>  - int64_t pos, size_t size);
> >>  + int64_t pos, size_t size,
> >>  + Error **errp);
> >>
> >>   /* Close a file
> >>    *
> >>  @@ -41,7 +42,7 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> >>    * The meaning of return value on success depends on the specific back-end being
> >>    * used.
> >>    */
> >>  -typedef int (QEMUFileCloseFunc)(void *opaque);
> >>  +typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
> >>
> >>   /* Called to return the OS file descriptor associated to the QEMUFile.
> >>    */
> >>  @@ -49,14 +50,15 @@ typedef int (QEMUFileGetFD)(void *opaque);
> >>
> >>   /* Called to change the blocking mode of the file
> >>    */
> >>  -typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
> >>  +typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
> >>
> >>   /*
> >>    * 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);
> >>  + int iovcnt, int64_t pos,
> >>  + Error **errp);
> >>
> >>   /*
> >>    * This function provides hooks around different
> >>  @@ -97,7 +99,8 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
> >>    * Existing blocking reads/writes must be woken
> >>    * Returns 0 on success, -err on error
> >>    */
> >>  -typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
> >>  +typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
> >>  + Error **errp);
> >>
> >>   typedef struct QEMUFileOps {
> >>       QEMUFileGetBufferFunc *get_buffer;
> >>  @@ -149,6 +152,8 @@ void qemu_update_position(QEMUFile *f, size_t size);
> >>   void qemu_file_reset_rate_limit(QEMUFile *f);
> >>   void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
> >>   int64_t qemu_file_get_rate_limit(QEMUFile *f);
> >>  +int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
> >>  +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
> >>   void qemu_file_set_error(QEMUFile *f, int ret);
> >>   int qemu_file_shutdown(QEMUFile *f);
> >>   QEMUFile *qemu_file_get_return_path(QEMUFile *f);
> >>  diff --git a/migration/savevm.c b/migration/savevm.c
> >>  index 34bcad3807..a619af744d 100644
> >>  --- a/migration/savevm.c
> >>  +++ b/migration/savevm.c
> >>  @@ -124,7 +124,7 @@ static struct mig_cmd_args {
> >>   /* savevm/loadvm support */
> >>
> >>   static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> >>  - int64_t pos)
> >>  + int64_t pos, Error **errp)
> >>   {
> >>       int ret;
> >>       QEMUIOVector qiov;
> >>  @@ -139,12 +139,12 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> >>   }
> >>
> >>   static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> >>  - size_t size)
> >>  + size_t size, Error **errp)
> >>   {
> >>       return bdrv_load_vmstate(opaque, buf, pos, size);
> >>   }
> >>
> >>  -static int bdrv_fclose(void *opaque)
> >>  +static int bdrv_fclose(void *opaque, Error **errp)
> >>   {
> >>       return bdrv_flush(opaque);
> >>   }
> >>  --
> >>  2.21.0
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> Regards,
> Yury
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert Aug. 7, 2019, 5:08 p.m. UTC | #5
* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Currently, there is no information about error if outgoing migration was failed
> because of file channel errors.
> Example (QMP session):
> -> { "execute": "migrate", "arguments": { "uri": "exec:head -c 1" }}
> <- { "return": {} }
> ...
> -> { "execute": "query-migrate" }
> <- { "return": { "status": "failed" }} // There is not error's description
> 
> And even in the QEMU's output there is nothing.
> 
> This patch
> 1) Adds errp for the most of QEMUFileOps
> 2) Adds qemu_file_get_error_obj/qemu_file_set_error_obj
> 3) And finally using of qemu_file_get_error_obj in migration.c
> 
> And now, the status for the mentioned fail will be:
> -> { "execute": "query-migrate" }
> <- { "return": { "status": "failed",
>                  "error-desc": "Unable to write to command: Broken pipe" }}
> 
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>

Quued for 4.2

> ---
>  migration/migration.c         | 10 ++++--
>  migration/qemu-file-channel.c | 30 +++++++++--------
>  migration/qemu-file.c         | 63 ++++++++++++++++++++++++++++-------
>  migration/qemu-file.h         | 15 ++++++---
>  migration/savevm.c            |  6 ++--
>  5 files changed, 88 insertions(+), 36 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..7bcdc4613b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2949,6 +2949,7 @@ static MigThrError migration_detect_error(MigrationState *s)
>  {
>      int ret;
>      int state = s->state;
> +    Error *local_error = NULL;
>  
>      if (state == MIGRATION_STATUS_CANCELLING ||
>          state == MIGRATION_STATUS_CANCELLED) {
> @@ -2957,13 +2958,18 @@ static MigThrError migration_detect_error(MigrationState *s)
>      }
>  
>      /* Try to detect any file errors */
> -    ret = qemu_file_get_error(s->to_dst_file);
> -
> +    ret = qemu_file_get_error_obj(s->to_dst_file, &local_error);
>      if (!ret) {
>          /* Everything is fine */
> +        assert(!local_error);
>          return MIG_THR_ERR_NONE;
>      }
>  
> +    if (local_error) {
> +        migrate_set_error(s, local_error);
> +        error_free(local_error);
> +    }
> +
>      if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret == -EIO) {
>          /*
>           * For postcopy, we allow the network to be down for a
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 8e639eb496..c382ea2d78 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -33,7 +33,8 @@
>  static ssize_t channel_writev_buffer(void *opaque,
>                                       struct iovec *iov,
>                                       int iovcnt,
> -                                     int64_t pos)
> +                                     int64_t pos,
> +                                     Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>      ssize_t done = 0;
> @@ -47,7 +48,7 @@ static ssize_t channel_writev_buffer(void *opaque,
>  
>      while (nlocal_iov > 0) {
>          ssize_t len;
> -        len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
> +        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);
> @@ -57,7 +58,6 @@ static ssize_t channel_writev_buffer(void *opaque,
>              continue;
>          }
>          if (len < 0) {
> -            /* XXX handle Error objects */
>              done = -EIO;
>              goto cleanup;
>          }
> @@ -75,13 +75,14 @@ static ssize_t channel_writev_buffer(void *opaque,
>  static ssize_t channel_get_buffer(void *opaque,
>                                    uint8_t *buf,
>                                    int64_t pos,
> -                                  size_t size)
> +                                  size_t size,
> +                                  Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>      ssize_t ret;
>  
>      do {
> -        ret = qio_channel_read(ioc, (char *)buf, size, NULL);
> +        ret = qio_channel_read(ioc, (char *)buf, size, errp);
>          if (ret < 0) {
>              if (ret == QIO_CHANNEL_ERR_BLOCK) {
>                  if (qemu_in_coroutine()) {
> @@ -90,7 +91,6 @@ static ssize_t channel_get_buffer(void *opaque,
>                      qio_channel_wait(ioc, G_IO_IN);
>                  }
>              } else {
> -                /* XXX handle Error * object */
>                  return -EIO;
>              }
>          }
> @@ -100,18 +100,20 @@ static ssize_t channel_get_buffer(void *opaque,
>  }
>  
>  
> -static int channel_close(void *opaque)
> +static int channel_close(void *opaque, Error **errp)
>  {
> +    int ret;
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
> -    qio_channel_close(ioc, NULL);
> +    ret = qio_channel_close(ioc, errp);
>      object_unref(OBJECT(ioc));
> -    return 0;
> +    return ret;
>  }
>  
>  
>  static int channel_shutdown(void *opaque,
>                              bool rd,
> -                            bool wr)
> +                            bool wr,
> +                            Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>  
> @@ -125,8 +127,7 @@ static int channel_shutdown(void *opaque,
>          } else {
>              mode = QIO_CHANNEL_SHUTDOWN_WRITE;
>          }
> -        if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
> -            /* XXX handler Error * object */
> +        if (qio_channel_shutdown(ioc, mode, errp) < 0) {
>              return -EIO;
>          }
>      }
> @@ -135,11 +136,12 @@ static int channel_shutdown(void *opaque,
>  
>  
>  static int channel_set_blocking(void *opaque,
> -                                bool enabled)
> +                                bool enabled,
> +                                Error **errp)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
>  
> -    if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
> +    if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
>          return -1;
>      }
>      return 0;
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 977b9ae07c..c52160e08b 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -29,6 +29,7 @@
>  #include "migration.h"
>  #include "qemu-file.h"
>  #include "trace.h"
> +#include "qapi/error.h"
>  
>  #define IO_BUF_SIZE 32768
>  #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
> @@ -52,6 +53,7 @@ struct QEMUFile {
>      unsigned int iovcnt;
>  
>      int last_error;
> +    Error *last_error_obj;
>  };
>  
>  /*
> @@ -63,7 +65,7 @@ int qemu_file_shutdown(QEMUFile *f)
>      if (!f->ops->shut_down) {
>          return -ENOSYS;
>      }
> -    return f->ops->shut_down(f->opaque, true, true);
> +    return f->ops->shut_down(f->opaque, true, true, NULL);
>  }
>  
>  /*
> @@ -108,24 +110,55 @@ void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
>  }
>  
>  /*
> - * Get last error for stream f
> + * Get last error for stream f with optional Error*
>   *
>   * Return negative error value if there has been an error on previous
>   * operations, return 0 if no error happened.
> + * Optional, it returns Error* in errp, but it may be NULL even if return value
> + * is not 0.
>   *
>   */
> -int qemu_file_get_error(QEMUFile *f)
> +int qemu_file_get_error_obj(QEMUFile *f, Error **errp)
>  {
> +    if (errp) {
> +        *errp = f->last_error_obj ? error_copy(f->last_error_obj) : NULL;
> +    }
>      return f->last_error;
>  }
>  
> -void qemu_file_set_error(QEMUFile *f, int ret)
> +/*
> + * Set the last error for stream f with optional Error*
> + */
> +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
>  {
> -    if (f->last_error == 0) {
> +    if (f->last_error == 0 && ret) {
>          f->last_error = ret;
> +        error_propagate(&f->last_error_obj, err);
> +    } else if (err) {
> +        error_report_err(err);
>      }
>  }
>  
> +/*
> + * Get last error for stream f
> + *
> + * Return negative error value if there has been an error on previous
> + * operations, return 0 if no error happened.
> + *
> + */
> +int qemu_file_get_error(QEMUFile *f)
> +{
> +    return qemu_file_get_error_obj(f, NULL);
> +}
> +
> +/*
> + * Set the last error for stream f
> + */
> +void qemu_file_set_error(QEMUFile *f, int ret)
> +{
> +    qemu_file_set_error_obj(f, ret, NULL);
> +}
> +
>  bool qemu_file_is_writable(QEMUFile *f)
>  {
>      return f->ops->writev_buffer;
> @@ -177,6 +210,7 @@ void qemu_fflush(QEMUFile *f)
>  {
>      ssize_t ret = 0;
>      ssize_t expect = 0;
> +    Error *local_error = NULL;
>  
>      if (!qemu_file_is_writable(f)) {
>          return;
> @@ -184,7 +218,8 @@ void qemu_fflush(QEMUFile *f)
>  
>      if (f->iovcnt > 0) {
>          expect = iov_size(f->iov, f->iovcnt);
> -        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
> +        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> +                                    &local_error);
>  
>          qemu_iovec_release_ram(f);
>      }
> @@ -196,7 +231,7 @@ void qemu_fflush(QEMUFile *f)
>       * data set we requested, so sanity check that.
>       */
>      if (ret != expect) {
> -        qemu_file_set_error(f, ret < 0 ? ret : -EIO);
> +        qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
>      }
>      f->buf_index = 0;
>      f->iovcnt = 0;
> @@ -284,6 +319,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>  {
>      int len;
>      int pending;
> +    Error *local_error = NULL;
>  
>      assert(!qemu_file_is_writable(f));
>  
> @@ -295,14 +331,16 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>      f->buf_size = pending;
>  
>      len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> -                        IO_BUF_SIZE - pending);
> +                             IO_BUF_SIZE - pending, &local_error);
>      if (len > 0) {
>          f->buf_size += len;
>          f->pos += len;
>      } else if (len == 0) {
> -        qemu_file_set_error(f, -EIO);
> +        qemu_file_set_error_obj(f, -EIO, local_error);
>      } else if (len != -EAGAIN) {
> -        qemu_file_set_error(f, len);
> +        qemu_file_set_error_obj(f, len, local_error);
> +    } else {
> +        error_free(local_error);
>      }
>  
>      return len;
> @@ -328,7 +366,7 @@ int qemu_fclose(QEMUFile *f)
>      ret = qemu_file_get_error(f);
>  
>      if (f->ops->close) {
> -        int ret2 = f->ops->close(f->opaque);
> +        int ret2 = f->ops->close(f->opaque, NULL);
>          if (ret >= 0) {
>              ret = ret2;
>          }
> @@ -339,6 +377,7 @@ int qemu_fclose(QEMUFile *f)
>      if (f->last_error) {
>          ret = f->last_error;
>      }
> +    error_free(f->last_error_obj);
>      g_free(f);
>      trace_qemu_file_fclose();
>      return ret;
> @@ -784,6 +823,6 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
>  void qemu_file_set_blocking(QEMUFile *f, bool block)
>  {
>      if (f->ops->set_blocking) {
> -        f->ops->set_blocking(f->opaque, block);
> +        f->ops->set_blocking(f->opaque, block, NULL);
>      }
>  }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 13baf896bd..eb886db65f 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -32,7 +32,8 @@
>   * bytes actually read should be returned.
>   */
>  typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> -                                        int64_t pos, size_t size);
> +                                        int64_t pos, size_t size,
> +                                        Error **errp);
>  
>  /* Close a file
>   *
> @@ -41,7 +42,7 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
>   * The meaning of return value on success depends on the specific back-end being
>   * used.
>   */
> -typedef int (QEMUFileCloseFunc)(void *opaque);
> +typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
>  
>  /* Called to return the OS file descriptor associated to the QEMUFile.
>   */
> @@ -49,14 +50,15 @@ typedef int (QEMUFileGetFD)(void *opaque);
>  
>  /* Called to change the blocking mode of the file
>   */
> -typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
> +typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
>  
>  /*
>   * 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);
> +                                           int iovcnt, int64_t pos,
> +                                           Error **errp);
>  
>  /*
>   * This function provides hooks around different
> @@ -97,7 +99,8 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>   * Existing blocking reads/writes must be woken
>   * Returns 0 on success, -err on error
>   */
> -typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
> +typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
> +                                   Error **errp);
>  
>  typedef struct QEMUFileOps {
>      QEMUFileGetBufferFunc *get_buffer;
> @@ -149,6 +152,8 @@ void qemu_update_position(QEMUFile *f, size_t size);
>  void qemu_file_reset_rate_limit(QEMUFile *f);
>  void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>  int64_t qemu_file_get_rate_limit(QEMUFile *f);
> +int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
> +void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
>  void qemu_file_set_error(QEMUFile *f, int ret);
>  int qemu_file_shutdown(QEMUFile *f);
>  QEMUFile *qemu_file_get_return_path(QEMUFile *f);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 34bcad3807..a619af744d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -124,7 +124,7 @@ static struct mig_cmd_args {
>  /* savevm/loadvm support */
>  
>  static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> -                                   int64_t pos)
> +                                   int64_t pos, Error **errp)
>  {
>      int ret;
>      QEMUIOVector qiov;
> @@ -139,12 +139,12 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
>  }
>  
>  static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> -                                size_t size)
> +                                size_t size, Error **errp)
>  {
>      return bdrv_load_vmstate(opaque, buf, pos, size);
>  }
>  
> -static int bdrv_fclose(void *opaque)
> +static int bdrv_fclose(void *opaque, Error **errp)
>  {
>      return bdrv_flush(opaque);
>  }
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 609e0df5d0..7bcdc4613b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2949,6 +2949,7 @@  static MigThrError migration_detect_error(MigrationState *s)
 {
     int ret;
     int state = s->state;
+    Error *local_error = NULL;
 
     if (state == MIGRATION_STATUS_CANCELLING ||
         state == MIGRATION_STATUS_CANCELLED) {
@@ -2957,13 +2958,18 @@  static MigThrError migration_detect_error(MigrationState *s)
     }
 
     /* Try to detect any file errors */
-    ret = qemu_file_get_error(s->to_dst_file);
-
+    ret = qemu_file_get_error_obj(s->to_dst_file, &local_error);
     if (!ret) {
         /* Everything is fine */
+        assert(!local_error);
         return MIG_THR_ERR_NONE;
     }
 
+    if (local_error) {
+        migrate_set_error(s, local_error);
+        error_free(local_error);
+    }
+
     if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret == -EIO) {
         /*
          * For postcopy, we allow the network to be down for a
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 8e639eb496..c382ea2d78 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -33,7 +33,8 @@ 
 static ssize_t channel_writev_buffer(void *opaque,
                                      struct iovec *iov,
                                      int iovcnt,
-                                     int64_t pos)
+                                     int64_t pos,
+                                     Error **errp)
 {
     QIOChannel *ioc = QIO_CHANNEL(opaque);
     ssize_t done = 0;
@@ -47,7 +48,7 @@  static ssize_t channel_writev_buffer(void *opaque,
 
     while (nlocal_iov > 0) {
         ssize_t len;
-        len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
+        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);
@@ -57,7 +58,6 @@  static ssize_t channel_writev_buffer(void *opaque,
             continue;
         }
         if (len < 0) {
-            /* XXX handle Error objects */
             done = -EIO;
             goto cleanup;
         }
@@ -75,13 +75,14 @@  static ssize_t channel_writev_buffer(void *opaque,
 static ssize_t channel_get_buffer(void *opaque,
                                   uint8_t *buf,
                                   int64_t pos,
-                                  size_t size)
+                                  size_t size,
+                                  Error **errp)
 {
     QIOChannel *ioc = QIO_CHANNEL(opaque);
     ssize_t ret;
 
     do {
-        ret = qio_channel_read(ioc, (char *)buf, size, NULL);
+        ret = qio_channel_read(ioc, (char *)buf, size, errp);
         if (ret < 0) {
             if (ret == QIO_CHANNEL_ERR_BLOCK) {
                 if (qemu_in_coroutine()) {
@@ -90,7 +91,6 @@  static ssize_t channel_get_buffer(void *opaque,
                     qio_channel_wait(ioc, G_IO_IN);
                 }
             } else {
-                /* XXX handle Error * object */
                 return -EIO;
             }
         }
@@ -100,18 +100,20 @@  static ssize_t channel_get_buffer(void *opaque,
 }
 
 
-static int channel_close(void *opaque)
+static int channel_close(void *opaque, Error **errp)
 {
+    int ret;
     QIOChannel *ioc = QIO_CHANNEL(opaque);
-    qio_channel_close(ioc, NULL);
+    ret = qio_channel_close(ioc, errp);
     object_unref(OBJECT(ioc));
-    return 0;
+    return ret;
 }
 
 
 static int channel_shutdown(void *opaque,
                             bool rd,
-                            bool wr)
+                            bool wr,
+                            Error **errp)
 {
     QIOChannel *ioc = QIO_CHANNEL(opaque);
 
@@ -125,8 +127,7 @@  static int channel_shutdown(void *opaque,
         } else {
             mode = QIO_CHANNEL_SHUTDOWN_WRITE;
         }
-        if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
-            /* XXX handler Error * object */
+        if (qio_channel_shutdown(ioc, mode, errp) < 0) {
             return -EIO;
         }
     }
@@ -135,11 +136,12 @@  static int channel_shutdown(void *opaque,
 
 
 static int channel_set_blocking(void *opaque,
-                                bool enabled)
+                                bool enabled,
+                                Error **errp)
 {
     QIOChannel *ioc = QIO_CHANNEL(opaque);
 
-    if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
+    if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
         return -1;
     }
     return 0;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 977b9ae07c..c52160e08b 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -29,6 +29,7 @@ 
 #include "migration.h"
 #include "qemu-file.h"
 #include "trace.h"
+#include "qapi/error.h"
 
 #define IO_BUF_SIZE 32768
 #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
@@ -52,6 +53,7 @@  struct QEMUFile {
     unsigned int iovcnt;
 
     int last_error;
+    Error *last_error_obj;
 };
 
 /*
@@ -63,7 +65,7 @@  int qemu_file_shutdown(QEMUFile *f)
     if (!f->ops->shut_down) {
         return -ENOSYS;
     }
-    return f->ops->shut_down(f->opaque, true, true);
+    return f->ops->shut_down(f->opaque, true, true, NULL);
 }
 
 /*
@@ -108,24 +110,55 @@  void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
 }
 
 /*
- * Get last error for stream f
+ * Get last error for stream f with optional Error*
  *
  * Return negative error value if there has been an error on previous
  * operations, return 0 if no error happened.
+ * Optional, it returns Error* in errp, but it may be NULL even if return value
+ * is not 0.
  *
  */
-int qemu_file_get_error(QEMUFile *f)
+int qemu_file_get_error_obj(QEMUFile *f, Error **errp)
 {
+    if (errp) {
+        *errp = f->last_error_obj ? error_copy(f->last_error_obj) : NULL;
+    }
     return f->last_error;
 }
 
-void qemu_file_set_error(QEMUFile *f, int ret)
+/*
+ * Set the last error for stream f with optional Error*
+ */
+void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
 {
-    if (f->last_error == 0) {
+    if (f->last_error == 0 && ret) {
         f->last_error = ret;
+        error_propagate(&f->last_error_obj, err);
+    } else if (err) {
+        error_report_err(err);
     }
 }
 
+/*
+ * Get last error for stream f
+ *
+ * Return negative error value if there has been an error on previous
+ * operations, return 0 if no error happened.
+ *
+ */
+int qemu_file_get_error(QEMUFile *f)
+{
+    return qemu_file_get_error_obj(f, NULL);
+}
+
+/*
+ * Set the last error for stream f
+ */
+void qemu_file_set_error(QEMUFile *f, int ret)
+{
+    qemu_file_set_error_obj(f, ret, NULL);
+}
+
 bool qemu_file_is_writable(QEMUFile *f)
 {
     return f->ops->writev_buffer;
@@ -177,6 +210,7 @@  void qemu_fflush(QEMUFile *f)
 {
     ssize_t ret = 0;
     ssize_t expect = 0;
+    Error *local_error = NULL;
 
     if (!qemu_file_is_writable(f)) {
         return;
@@ -184,7 +218,8 @@  void qemu_fflush(QEMUFile *f)
 
     if (f->iovcnt > 0) {
         expect = iov_size(f->iov, f->iovcnt);
-        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
+        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
+                                    &local_error);
 
         qemu_iovec_release_ram(f);
     }
@@ -196,7 +231,7 @@  void qemu_fflush(QEMUFile *f)
      * data set we requested, so sanity check that.
      */
     if (ret != expect) {
-        qemu_file_set_error(f, ret < 0 ? ret : -EIO);
+        qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
     }
     f->buf_index = 0;
     f->iovcnt = 0;
@@ -284,6 +319,7 @@  static ssize_t qemu_fill_buffer(QEMUFile *f)
 {
     int len;
     int pending;
+    Error *local_error = NULL;
 
     assert(!qemu_file_is_writable(f));
 
@@ -295,14 +331,16 @@  static ssize_t qemu_fill_buffer(QEMUFile *f)
     f->buf_size = pending;
 
     len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
-                        IO_BUF_SIZE - pending);
+                             IO_BUF_SIZE - pending, &local_error);
     if (len > 0) {
         f->buf_size += len;
         f->pos += len;
     } else if (len == 0) {
-        qemu_file_set_error(f, -EIO);
+        qemu_file_set_error_obj(f, -EIO, local_error);
     } else if (len != -EAGAIN) {
-        qemu_file_set_error(f, len);
+        qemu_file_set_error_obj(f, len, local_error);
+    } else {
+        error_free(local_error);
     }
 
     return len;
@@ -328,7 +366,7 @@  int qemu_fclose(QEMUFile *f)
     ret = qemu_file_get_error(f);
 
     if (f->ops->close) {
-        int ret2 = f->ops->close(f->opaque);
+        int ret2 = f->ops->close(f->opaque, NULL);
         if (ret >= 0) {
             ret = ret2;
         }
@@ -339,6 +377,7 @@  int qemu_fclose(QEMUFile *f)
     if (f->last_error) {
         ret = f->last_error;
     }
+    error_free(f->last_error_obj);
     g_free(f);
     trace_qemu_file_fclose();
     return ret;
@@ -784,6 +823,6 @@  void qemu_put_counted_string(QEMUFile *f, const char *str)
 void qemu_file_set_blocking(QEMUFile *f, bool block)
 {
     if (f->ops->set_blocking) {
-        f->ops->set_blocking(f->opaque, block);
+        f->ops->set_blocking(f->opaque, block, NULL);
     }
 }
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 13baf896bd..eb886db65f 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -32,7 +32,8 @@ 
  * bytes actually read should be returned.
  */
 typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
-                                        int64_t pos, size_t size);
+                                        int64_t pos, size_t size,
+                                        Error **errp);
 
 /* Close a file
  *
@@ -41,7 +42,7 @@  typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
  * The meaning of return value on success depends on the specific back-end being
  * used.
  */
-typedef int (QEMUFileCloseFunc)(void *opaque);
+typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
 
 /* Called to return the OS file descriptor associated to the QEMUFile.
  */
@@ -49,14 +50,15 @@  typedef int (QEMUFileGetFD)(void *opaque);
 
 /* Called to change the blocking mode of the file
  */
-typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
+typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
 
 /*
  * 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);
+                                           int iovcnt, int64_t pos,
+                                           Error **errp);
 
 /*
  * This function provides hooks around different
@@ -97,7 +99,8 @@  typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
  * Existing blocking reads/writes must be woken
  * Returns 0 on success, -err on error
  */
-typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
+typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
+                                   Error **errp);
 
 typedef struct QEMUFileOps {
     QEMUFileGetBufferFunc *get_buffer;
@@ -149,6 +152,8 @@  void qemu_update_position(QEMUFile *f, size_t size);
 void qemu_file_reset_rate_limit(QEMUFile *f);
 void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
+int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
+void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
 void qemu_file_set_error(QEMUFile *f, int ret);
 int qemu_file_shutdown(QEMUFile *f);
 QEMUFile *qemu_file_get_return_path(QEMUFile *f);
diff --git a/migration/savevm.c b/migration/savevm.c
index 34bcad3807..a619af744d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -124,7 +124,7 @@  static struct mig_cmd_args {
 /* savevm/loadvm support */
 
 static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
-                                   int64_t pos)
+                                   int64_t pos, Error **errp)
 {
     int ret;
     QEMUIOVector qiov;
@@ -139,12 +139,12 @@  static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
 }
 
 static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
-                                size_t size)
+                                size_t size, Error **errp)
 {
     return bdrv_load_vmstate(opaque, buf, pos, size);
 }
 
-static int bdrv_fclose(void *opaque)
+static int bdrv_fclose(void *opaque, Error **errp)
 {
     return bdrv_flush(opaque);
 }