Re: [PATCH 16/20] migration: remove the QEMUFileOps 'close' callback

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the close logic using QIOChannel APIs.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

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

> ---
>  migration/qemu-file-channel.c | 12 ------------
>  migration/qemu-file.c         | 12 ++++++------
>  migration/qemu-file.h         | 10 ----------
>  3 files changed, 6 insertions(+), 28 deletions(-)
> 
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 0350d367ec..8ff58e81f9 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -102,16 +102,6 @@ static ssize_t channel_get_buffer(void *opaque,
>  }
>  
>  
> -static int channel_close(void *opaque, Error **errp)
> -{
> -    int ret;
> -    QIOChannel *ioc = QIO_CHANNEL(opaque);
> -    ret = qio_channel_close(ioc, errp);
> -    object_unref(OBJECT(ioc));
> -    return ret;
> -}
> -
> -
>  static QEMUFile *channel_get_input_return_path(void *opaque)
>  {
>      QIOChannel *ioc = QIO_CHANNEL(opaque);
> @@ -128,14 +118,12 @@ static QEMUFile *channel_get_output_return_path(void 
> *opaque)
>  
>  static const QEMUFileOps channel_input_ops = {
>      .get_buffer = channel_get_buffer,
> -    .close = channel_close,
>      .get_return_path = channel_get_input_return_path,
>  };
>  
>  
>  static const QEMUFileOps channel_output_ops = {
>      .writev_buffer = channel_writev_buffer,
> -    .close = channel_close,
>      .get_return_path = channel_get_output_return_path,
>  };
>  
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 171b9f85bf..a855ce33dc 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -405,16 +405,16 @@ void qemu_file_credit_transfer(QEMUFile *f, size_t size)
>   */
>  int qemu_fclose(QEMUFile *f)
>  {
> -    int ret;
> +    int ret, ret2;
>      qemu_fflush(f);
>      ret = qemu_file_get_error(f);
>  
> -    if (f->ops->close) {
> -        int ret2 = f->ops->close(f->ioc, NULL);
> -        if (ret >= 0) {
> -            ret = ret2;
> -        }
> +    ret2 = qio_channel_close(f->ioc, NULL);
> +    if (ret >= 0) {
> +        ret = ret2;
>      }
> +    g_clear_pointer(&f->ioc, object_unref);
> +
>      /* If any error was spotted before closing, we should report it
>       * instead of the close() return value.
>       */
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 6db4bb9fdb..7ec105bf96 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -37,15 +37,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, 
> uint8_t *buf,
>                                          int64_t pos, size_t size,
>                                          Error **errp);
>  
> -/* Close a file
> - *
> - * Return negative error number on error, 0 or positive value on success.
> - *
> - * The meaning of return value on success depends on the specific back-end 
> being
> - * used.
> - */
> -typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
> -
>  /*
>   * This function writes an iovec to file. The handler must write all
>   * of the data or return a negative errno value.
> @@ -87,7 +78,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>  
>  typedef struct QEMUFileOps {
>      QEMUFileGetBufferFunc *get_buffer;
> -    QEMUFileCloseFunc *close;
>      QEMUFileWritevBufferFunc *writev_buffer;
>      QEMURetPathFunc *get_return_path;
>  } QEMUFileOps;
> -- 
> 2.36.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK