Re: [Qemu-devel] [PATCH v2 00/38] Delay destruction of memory regions to instance_finalize

[Michael S. Tsirkin]

On Tue, Sep 17, 2013 at 05:41:52PM +0200, Paolo Bonzini wrote:
> Il 17/09/2013 16:45, Michael S. Tsirkin ha scritto:
> > On Tue, Sep 17, 2013 at 04:41:45PM +0200, Paolo Bonzini wrote:
> >> Il 17/09/2013 14:47, Michael S. Tsirkin ha scritto:
> >>> Ergo, to make sure a referenced MR does not get
> >>> destroyed, we must make sure only finalize
> >>> calls memory_region_destroy.
> >>>
> >>> So I think this patchset should do exactly that,
> >>> not try to move out more stuff to finalize.
> >>
> >> Yes, this is the part of the problem that applies to all devices.
> >>
> >> Some devices may have timers or other dynamically-allocated data that
> >> may be used between exit and finalize.  The same applies to capabilities
> >> such as MSI and MSI-X.  All this must also be moved to finalize, but
> >> this can be done in separate patches.
> >>
> >> I'll separate the series as follows:
> >>
> >> - memory_region_destroy
> >>
> >> - unregister_savevm
> > 
> > What's the problem here exactly?
> 
> I was thinking about AIO callbacks here, but we don't support device
> hot(un)plug during migration so I can remove this part.
> 
> >> - other dynamically allocated data
> > 
> > And here? I don't think we should move rundom stuff out to finalize
> > just because we can.
> 
> It's not random stuff, AIO callbacks can access dynamically allocated
> data between exit and finalize.
> Even if a particular device doesn't do
> it, there's no hope of having best practices from contributors if we do
> not do things consistently.

Yes but just not freeing it is unlikely to be enough.
We need to make sure data structures are consistent.
So this really needs to be done carefully, device by device.

> 
> >> - del_vm_change_state_handler
> 
> Same here: user can stop/cont between exit and finalize, for example
> because the I/O failed.
> 
> Paolo

Device that is not guest visible is very unlikely to
care about whether guest is running.


> >> - MSI-X
> >>
> >> - SHPC, AER and other PCI/bridge stuff
> >>
> >> Paolo
> > 
> > Let's document specific problems and fix them.
> > Randomly moving cleanup order has a high chance to
> > introduce bugs.
> >