| Message ID | 20191219154323.325255-1-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Memory: Only call ramblock_ptr when needed in qemu_ram_writeback | expand |
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 >
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> Reviewed-by: Juan Quintela <quintela@redhat.com> This is exec.c, nothing related to migration. Paolo, are you taking this one? It could even go through the trivial one. Thanks, Juan.
On Thu, Dec 19, 2019 at 07:10:24PM +0100, Juan Quintela wrote: > 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> > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > This is exec.c, nothing related to migration. > > Paolo, are you taking this one? > It could even go through the trivial one. Hi, I'm going to send a pull request for the xen queue with this patch. Unless that's an issue? Cheers,
On 25/02/20 17:02, Anthony PERARD wrote: > On Thu, Dec 19, 2019 at 07:10:24PM +0100, Juan Quintela wrote: >> 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> >> >> Reviewed-by: Juan Quintela <quintela@redhat.com> >> >> This is exec.c, nothing related to migration. >> >> Paolo, are you taking this one? >> It could even go through the trivial one. > > Hi, > > I'm going to send a pull request for the xen queue with this patch. > Unless that's an issue? No, absolutely. Acked-by: Paolo Bonzini <pbonzini@redhat.com> Paolo
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,
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(-)