Re: [Qemu-devel] [RFC 12/13] migration/ram: Fix some helper functions' parameter to use PageSearchStatus

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

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> Some helper functions use parameters 'RAMBlock *block' and 'ram_addr_t *offset',
> We can use 'PageSearchStatus *pss' directly instead, with this change, we
> can reduce the number of parameters for these helper function, also
> it is easily to add new parameters for these helper functions.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

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

You should post this as a separate patch, it's independent of
the rest of the series and makes sense to go in anyway.

Dave

> ---
>  migration/ram.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index fc4c788..8656719 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -726,7 +726,7 @@ static int save_zero_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>   * @last_stage: if we are at the completion stage
>   * @bytes_transferred: increase it with the number of transferred bytes
>   */
> -static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
> +static int ram_save_page(QEMUFile *f, PageSearchStatus *pss,
>                           bool last_stage, uint64_t *bytes_transferred)
>  {
>      int pages = -1;
> @@ -739,6 +739,8 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
>      * changed before it is really been saved.
>      */
>      bool send_async = !migration_in_snapshot(migrate_get_current());
> +    RAMBlock *block = pss->block;
> +    ram_addr_t offset = pss->offset;
>  
>      p = block->host + offset;
>  
> @@ -913,14 +915,16 @@ static int compress_page_with_multi_thread(QEMUFile *f, RAMBlock *block,
>   * @last_stage: if we are at the completion stage
>   * @bytes_transferred: increase it with the number of transferred bytes
>   */
> -static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
> -                                    ram_addr_t offset, bool last_stage,
> +static int ram_save_compressed_page(QEMUFile *f, PageSearchStatus *pss,
> +                                    bool last_stage,
>                                      uint64_t *bytes_transferred)
>  {
>      int pages = -1;
>      uint64_t bytes_xmit;
>      uint8_t *p;
>      int ret;
> +    RAMBlock *block = pss->block;
> +    ram_addr_t offset = pss->offset;
>  
>      p = block->host + offset;
>  
> @@ -1230,7 +1234,7 @@ err:
>   * Returns: Number of pages written.
>   */
>  static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
> -                                RAMBlock *block, ram_addr_t offset,
> +                                PageSearchStatus *pss,
>                                  bool last_stage,
>                                  uint64_t *bytes_transferred,
>                                  ram_addr_t dirty_ram_abs)
> @@ -1241,11 +1245,11 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
>      if (migration_bitmap_clear_dirty(dirty_ram_abs)) {
>          unsigned long *unsentmap;
>          if (compression_switch && migrate_use_compression()) {
> -            res = ram_save_compressed_page(f, block, offset,
> +            res = ram_save_compressed_page(f, pss,
>                                             last_stage,
>                                             bytes_transferred);
>          } else {
> -            res = ram_save_page(f, block, offset, last_stage,
> +            res = ram_save_page(f, pss, last_stage,
>                                  bytes_transferred);
>          }
>  
> @@ -1261,7 +1265,7 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
>           * to the stream.
>           */
>          if (res > 0) {
> -            last_sent_block = block;
> +            last_sent_block = pss->block;
>          }
>      }
>  
> @@ -1285,26 +1289,27 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
>   * @bytes_transferred: increase it with the number of transferred bytes
>   * @dirty_ram_abs: Address of the start of the dirty page in ram_addr_t space
>   */
> -static int ram_save_host_page(MigrationState *ms, QEMUFile *f, RAMBlock *block,
> -                              ram_addr_t *offset, bool last_stage,
> +static int ram_save_host_page(MigrationState *ms, QEMUFile *f,
> +                              PageSearchStatus *pss,
> +                              bool last_stage,
>                                uint64_t *bytes_transferred,
>                                ram_addr_t dirty_ram_abs)
>  {
>      int tmppages, pages = 0;
>      do {
> -        tmppages = ram_save_target_page(ms, f, block, *offset, last_stage,
> +        tmppages = ram_save_target_page(ms, f, pss, last_stage,
>                                          bytes_transferred, dirty_ram_abs);
>          if (tmppages < 0) {
>              return tmppages;
>          }
>  
>          pages += tmppages;
> -        *offset += TARGET_PAGE_SIZE;
> +        pss->offset += TARGET_PAGE_SIZE;
>          dirty_ram_abs += TARGET_PAGE_SIZE;
> -    } while (*offset & (qemu_host_page_size - 1));
> +    } while (pss->offset & (qemu_host_page_size - 1));
>  
>      /* The offset we leave with is the last one we looked at */
> -    *offset -= TARGET_PAGE_SIZE;
> +    pss->offset -= TARGET_PAGE_SIZE;
>      return pages;
>  }
>  
> @@ -1352,7 +1357,7 @@ static int ram_find_and_save_block(QEMUFile *f, bool last_stage,
>          }
>  
>          if (found) {
> -            pages = ram_save_host_page(ms, f, pss.block, &pss.offset,
> +            pages = ram_save_host_page(ms, f, &pss,
>                                         last_stage, bytes_transferred,
>                                         dirty_ram_abs);
>              /* For snapshot, we will remove the page write-protect here */
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK