On 03/09/2017 02:22 PM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > > Postcopy doesn't support migration of RAM shared with another process > yet (we've got a bunch of things to understand). > Check for the case and don't allow postcopy to be enabled. > > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > --- > migration/postcopy-ram.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index effbeb6..dc80dbb 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -95,6 +95,19 @@ static bool ufd_version_check(int ufd) > return true; > } > > +/* Callback from postcopy_ram_supported_by_host block iterator. > + */ > +static int test_range_shared(const char *block_name, void *host_addr, > + ram_addr_t offset, ram_addr_t length, void > *opaque) > +{ > + if (qemu_ram_is_shared(qemu_ram_block_by_name(block_name))) { > + error_report("Postcopy on shared RAM (%s) is not yet supported", > + block_name); > + return 1; > + } > + return 0; > +} > +
Hm, this stuff with the iterator seemed a bit strange (too complicated) first, but I'm not familiar with this code. I have no idea why is RAMBlockIterFunc typedef int (RAMBlockIterFunc)(const char *block_name, void *host_addr, ram_addr_t offset, ram_addr_t length, void *opaque) and not typedef int (RAMBlockIterFunc)(RAMBlock *block, void *opaque). The reason does not seem to be abstraction. > /* > * Note: This has the side effect of munlock'ing all of RAM, that's > * normally fine since if the postcopy succeeds it gets turned back on at the > @@ -127,6 +140,11 @@ bool postcopy_ram_supported_by_host(void) > goto out; > } > > + /* We don't support postcopy with shared RAM yet */ > + if (qemu_ram_foreach_block(test_range_shared, NULL)) { > + goto out; > + } > + But using ram_list directly does not seem to be a good alternative to me, and I do not see a third alternative. So besides some cosmetic stuff I have nothing to add. Cosmetic stuff is: * why range instead of block in test_range_shared * I think we could move this up so that we can return directly and do not acquire resources which need cleanup Regardless of the cosmetics: Reviewed-by: Halil Pasic <pa...@linux.vnet.ibm.com> > /* > * userfault and mlock don't go together; we'll put it back later if > * it was enabled. >