Re: [Qemu-devel] [PATCH 8/8] migration: remove ram_save_compressed_page()

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

* guangrong.xiao@gmail.com (guangrong.xiao@gmail.com) wrote:
> From: Xiao Guangrong <xiaoguangrong@tencent.com>
> 
> Now, we can reuse the path in ram_save_page() to post the page out
> as normal, then the only thing remained in ram_save_compressed_page()
> is compression that we can move it out to the caller
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@tencent.com>

Thanks, that does simplify stuff a lot in the end!

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

> ---
>  migration/ram.c | 45 ++++++++-------------------------------------
>  1 file changed, 8 insertions(+), 37 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index f778627992..8f4f8aca86 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1162,41 +1162,6 @@ static int compress_page_with_multi_thread(RAMState *rs, RAMBlock *block,
>      return pages;
>  }
>  
> -/**
> - * ram_save_compressed_page: compress the given page and send it to the stream
> - *
> - * Returns the number of pages written.
> - *
> - * @rs: current RAM state
> - * @block: block that contains the page we want to send
> - * @offset: offset inside the block for the page
> - * @last_stage: if we are at the completion stage
> - */
> -static int ram_save_compressed_page(RAMState *rs, PageSearchStatus *pss,
> -                                    bool last_stage)
> -{
> -    int pages = -1;
> -    uint8_t *p;
> -    RAMBlock *block = pss->block;
> -    ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
> -
> -    p = block->host + offset;
> -
> -    if (block != rs->last_sent_block) {
> -        /*
> -         * Make sure the first page is sent out before other pages.
> -         *
> -         * we post it as normal page as compression will take much
> -         * CPU resource.
> -         */
> -        pages = save_normal_page(rs, block, offset, p, true);
> -    } else {
> -        pages = compress_page_with_multi_thread(rs, block, offset);
> -    }
> -
> -    return pages;
> -}
> -
>  /**
>   * find_dirty_block: find the next dirty page and update any state
>   * associated with the search process.
> @@ -1501,8 +1466,14 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,
>              goto page_saved;
>          }
>  
> -        if (save_page_use_compression(rs)) {
> -            res = ram_save_compressed_page(rs, pss, last_stage);
> +        /*
> +         * Make sure the first page is sent out before other pages.
> +         *
> +         * we post it as normal page as compression will take much
> +         * CPU resource.
> +         */
> +        if (block == rs->last_sent_block && save_page_use_compression(rs)) {
> +            res = compress_page_with_multi_thread(rs, block, offset);
>          } else {
>              res = ram_save_page(rs, pss, last_stage);
>          }
> -- 
> 2.14.3
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK