Re: [PATCH] Memory: Only call ramblock_ptr when needed in qemu_ram_writeback

[Beata Michalska <beata.michalska@linaro.org>]

Hi Anthony,

On Thu, 19 Dec 2019 at 15:43, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> It is possible that a ramblock doesn't have memory that QEMU can
> access, this is the case with the Xen hypervisor.
>
> In order to avoid to trigger an assert, only call ramblock_ptr() when
> needed in qemu_ram_writeback(). This should fix migration of Xen
> guests that was broken with bd108a44bc29 ("migration: ram: Switch to
> ram block writeback").
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  exec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index a34c34818404..b11010e0cb4c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2166,14 +2166,13 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
>   */
>  void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length)
>  {
> -    void *addr = ramblock_ptr(block, start);
> -
>      /* The requested range should fit in within the block range */
>      g_assert((start + length) <= block->used_length);
>
>  #ifdef CONFIG_LIBPMEM
>      /* The lack of support for pmem should not block the sync */
>      if (ramblock_is_pmem(block)) {
> +        void *addr = ramblock_ptr(block, start);
>          pmem_persist(addr, length);
>          return;
>      }
> @@ -2184,6 +2183,7 @@ void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length)
>           * specified as persistent (or is not one) - use the msync.
>           * Less optimal but still achieves the same goal
>           */
> +        void *addr = ramblock_ptr(block, start);
>          if (qemu_msync(addr, length, block->fd)) {
>              warn_report("%s: failed to sync memory range: start: "
>                      RAM_ADDR_FMT " length: " RAM_ADDR_FMT,

We could also do :
void *addr = block->host ? ramblock_ptr : NULL

Looks good to me thought.
Thanks for fixing.

BR

Beata
> --
> Anthony PERARD
>