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

[Paolo Bonzini]

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.

>> - del_vm_change_state_handler

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

Paolo

>> - 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.
>