Re: [Qemu-devel] [PATCH v2] migration/xbzrle: update cache and current_data in one place

[Dr. David Alan Gilbert]

* Wei Yang (address@hidden) wrote:
> When we are not in the last_stage, we need to update the cache if page
> is not the same.
> 
> Currently this procedure is scattered in two places and mixed with
> encoding status check.
> 
> This patch extract this general step out to make the code a little bit
> easy to read.
> 
> Signed-off-by: Wei Yang <address@hidden>
> 

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

> ---
> v2: give more comment on the behavior
> ---
>  migration/ram.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index e9b40d636d..17cc9b2b44 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1594,25 +1594,30 @@ static int save_xbzrle_page(RAMState *rs, uint8_t 
> **current_data,
>      encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf,
>                                         TARGET_PAGE_SIZE, XBZRLE.encoded_buf,
>                                         TARGET_PAGE_SIZE);
> +
> +    /*
> +     * Update the cache contents, so that it corresponds to the data
> +     * sent, in all cases except where we skip the page.
> +     */
> +    if (!last_stage && encoded_len != 0) {
> +        memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
> +        /*
> +         * In the case where we couldn't compress, ensure that the caller
> +         * sends the data from the cache, since the guest might have
> +         * changed the RAM since we copied it.
> +         */
> +        *current_data = prev_cached_page;
> +    }
> +
>      if (encoded_len == 0) {
>          trace_save_xbzrle_page_skipping();
>          return 0;
>      } else if (encoded_len == -1) {
>          trace_save_xbzrle_page_overflow();
>          xbzrle_counters.overflow++;
> -        /* update data in the cache */
> -        if (!last_stage) {
> -            memcpy(prev_cached_page, *current_data, TARGET_PAGE_SIZE);
> -            *current_data = prev_cached_page;
> -        }
>          return -1;
>      }
>  
> -    /* we need to update the data in the cache, in order to get the same 
> data */
> -    if (!last_stage) {
> -        memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
> -    }
> -
>      /* Send XBZRLE based compressed page */
>      bytes_xbzrle = save_page_header(rs, rs->f, block,
>                                      offset | RAM_SAVE_FLAG_XBZRLE);
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK