Re: [PATCH 10/20] migration: stop passing 'opaque' parameter to QEMUFile hooks

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The only user of the hooks is RDMA which provides a QIOChannel backed
> impl of QEMUFile. It can thus use the qemu_file_get_ioc() method.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

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

> ---
>  migration/qemu-file.c |  8 ++++----
>  migration/qemu-file.h | 14 ++++++--------
>  migration/rdma.c      | 19 ++++++++++---------
>  3 files changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 9a7f715e17..6badc8b0ec 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -266,7 +266,7 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t 
> flags)
>      int ret = 0;
>  
>      if (f->hooks && f->hooks->before_ram_iterate) {
> -        ret = f->hooks->before_ram_iterate(f, f->opaque, flags, NULL);
> +        ret = f->hooks->before_ram_iterate(f, flags, NULL);
>          if (ret < 0) {
>              qemu_file_set_error(f, ret);
>          }
> @@ -278,7 +278,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t 
> flags)
>      int ret = 0;
>  
>      if (f->hooks && f->hooks->after_ram_iterate) {
> -        ret = f->hooks->after_ram_iterate(f, f->opaque, flags, NULL);
> +        ret = f->hooks->after_ram_iterate(f, flags, NULL);
>          if (ret < 0) {
>              qemu_file_set_error(f, ret);
>          }
> @@ -290,7 +290,7 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, 
> void *data)
>      int ret = -EINVAL;
>  
>      if (f->hooks && f->hooks->hook_ram_load) {
> -        ret = f->hooks->hook_ram_load(f, f->opaque, flags, data);
> +        ret = f->hooks->hook_ram_load(f, flags, data);
>          if (ret < 0) {
>              qemu_file_set_error(f, ret);
>          }
> @@ -310,7 +310,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t 
> block_offset,
>                               uint64_t *bytes_sent)
>  {
>      if (f->hooks && f->hooks->save_page) {
> -        int ret = f->hooks->save_page(f, f->opaque, block_offset,
> +        int ret = f->hooks->save_page(f, block_offset,
>                                        offset, size, bytes_sent);
>          if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
>              f->rate_limit_used += size;
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 81f6fd7db8..6310071f90 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -65,11 +65,9 @@ typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, 
> struct iovec *iov,
>  /*
>   * This function provides hooks around different
>   * stages of RAM migration.
> - * 'opaque' is the backend specific data in QEMUFile
>   * 'data' is call specific data associated with the 'flags' value
>   */
> -typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags,
> -                              void *data);
> +typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
>  
>  /*
>   * Constants used by ram_control_* hooks
> @@ -84,11 +82,11 @@ typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, 
> uint64_t flags,
>   * This function allows override of where the RAM page
>   * is saved (such as RDMA, for example.)
>   */
> -typedef size_t (QEMURamSaveFunc)(QEMUFile *f, void *opaque,
> -                               ram_addr_t block_offset,
> -                               ram_addr_t offset,
> -                               size_t size,
> -                               uint64_t *bytes_sent);
> +typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> +                                 ram_addr_t block_offset,
> +                                 ram_addr_t offset,
> +                                 size_t size,
> +                                 uint64_t *bytes_sent);
>  
>  /*
>   * Return a QEMUFile for comms in the opposite direction
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6e7756bee7..83265513d9 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3256,11 +3256,11 @@ qio_channel_rdma_shutdown(QIOChannel *ioc,
>   *                  sent. Usually, this will not be more than a few bytes of
>   *                  the protocol because most transfers are sent 
> asynchronously.
>   */
> -static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
> +static size_t qemu_rdma_save_page(QEMUFile *f,
>                                    ram_addr_t block_offset, ram_addr_t offset,
>                                    size_t size, uint64_t *bytes_sent)
>  {
> -    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
>      RDMAContext *rdma;
>      int ret;
>  
> @@ -3872,14 +3872,15 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, 
> const char *name)
>      return 0;
>  }
>  
> -static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void 
> *data)
> +static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
>  {
> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
>      switch (flags) {
>      case RAM_CONTROL_BLOCK_REG:
> -        return rdma_block_notification_handle(opaque, data);
> +        return rdma_block_notification_handle(rioc, data);
>  
>      case RAM_CONTROL_HOOK:
> -        return qemu_rdma_registration_handle(f, opaque);
> +        return qemu_rdma_registration_handle(f, rioc);
>  
>      default:
>          /* Shouldn't be called with any other values */
> @@ -3887,10 +3888,10 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, 
> uint64_t flags, void *data)
>      }
>  }
>  
> -static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
> +static int qemu_rdma_registration_start(QEMUFile *f,
>                                          uint64_t flags, void *data)
>  {
> -    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
>      RDMAContext *rdma;
>  
>      RCU_READ_LOCK_GUARD();
> @@ -3916,10 +3917,10 @@ static int qemu_rdma_registration_start(QEMUFile *f, 
> void *opaque,
>   * Inform dest that dynamic registrations are done for now.
>   * First, flush writes, if any.
>   */
> -static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
> +static int qemu_rdma_registration_stop(QEMUFile *f,
>                                         uint64_t flags, void *data)
>  {
> -    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
> +    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
>      RDMAContext *rdma;
>      RDMAControlHeader head = { .len = 0, .repeat = 1 };
>      int ret = 0;
> -- 
> 2.36.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK