Re: [Qemu-devel] [RFC PATCH] migration: discard RAMBlocks of type ram_device

[Dr. David Alan Gilbert]

* Cédric Le Goater (address@hidden) wrote:
> On 04/11/2018 09:21 PM, Dr. David Alan Gilbert wrote:
> > * Cédric Le Goater (address@hidden) wrote:
> >> Here is some context for this strange change request.
> >>
> >> On the POWER9 processor, the XIVE interrupt controller can control
> >> interrupt sources using MMIO to trigger events, to EOI or to turn off
> >> the sources. Priority management and interrupt acknowledgment is also
> >> controlled by MMIO in the presenter subengine.
> >>
> >> These MMIO regions are exposed to guests in QEMU with a set of 'ram
> >> device' memory mappings, similarly to VFIO, and the VMAs are populated
> >> dynamically with the appropriate pages using a fault handler.
> >>
> >> But, these regions are an issue for migration. We need to discard the
> >> associated RAMBlocks from the RAM state on the source VM and let the
> >> destination VM rebuild the memory mappings on the new host in the
> >> post_load() operation just before resuming the system.
> >>
> >> This is the goal of the following proposal. Does it make sense ? It
> >> seems to be working enough to migrate a running guest but there might
> >> be a better, more subtle, approach.
> > 
> > If this is always true of RAM devices (which I suspect it is).
> >
> > Interestingly, your patch comes less than 2 weeks after Lai Jiangshan's
> >  'add capability to bypass the shared memory'
> >    https://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg07511.html
> 
> I missed that.
> 
> > which is the only other case I think we've got of someone trying to
> > avoid transmitting a block.
> > 
> > We should try and merge the two sets to make them consistent; you've
> > covered some more cases (the other patch wasn't expected to work with
> > Postcopy anyway).
> > (At this rate then we can expect another 20 for the year....)
> > 
> > We should probably have:
> >    1) A     bool is_migratable_block(RAMBlock *)
> >    2) A RAMBLOCK_FOREACH_MIGRATABLE(block)  macro that is like
> > RAMBLOCK_FOREACH but does the call to is_migratable_block
> >
> > then the changes should be mostly pretty tidy.
> 
> yes, indeed, they do.
> 
> > A sanity check is probably needed on load as well, to give a neat
> > error if for some reason the source transmits pages to you.
> 
> OK. 
> 
> Would a check on the block migratability at the end of function 
> ram_block_from_stream() be enough ? This is called in ram_load()
> and ram_load_postcopy()

Yes I think that's fine.  Maybe also add one in ram_load() in
the     case RAM_SAVE_FLAG_MEM_SIZE:      which happens right at the
start of the migration stream.

> > One other thing I notice is your code changes ram_bytes_total(),
> > where as the other patch avoids it;  I think your code is actually
> > more correct.
> > 
> > Is there *any* case in existing QEMUs where we migrate ram devices
> > succesfully, if so we've got to make it backwards compatible; but I
> > think you're saying there isn't.
> 
> The only RAM devices I know of are the VFIOs.

Great, so we should be OK.

Dave

> Thanks,
> 
> C.
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK