Re: [Qemu-block] [PATCH v3 07/33] automatically add vmstate for reset support in devices

[Damien Hedde]

On 8/9/19 12:07 PM, Peter Maydell wrote:
> On Thu, 8 Aug 2019 at 16:42, Dr. David Alan Gilbert <address@hidden> wrote:
>>
>> * Peter Maydell (address@hidden) wrote:
>>> On Mon, 29 Jul 2019 at 15:59, Damien Hedde <address@hidden> wrote:
>>>>
>>>> This add the reset related sections for every QOM
>>>> device.
>>>
>>> A bit more detail in the commit message would help, I think --
>>> this is adding extra machinery which has to copy and modify
>>> the VMStateDescription passed in by the device in order to
>>> add the subsection that handles reset.
>>>
>>> I've added Dave Gilbert to the already long cc list since this
>>> is migration related.
>>
>> I don't like dynamically modifying all the vmsds.
> 
> Yeah, I'm not a huge fan of it either, but it does mean that
> the state gets migrated and we don't have a pile of really
> easy to miss bugs where we forgot to say "this device needs to
> migrate the reset state" and it means we don't have every
> device we ever write in the future needing to say "this device
> needs to migrate reset state"...
> 
>> Aren't you going to have to understand each devices reset behaviour
>> and make sure it does something sane? e.g. it might have a postload
>> that registers a timer or something that you wouldn't want to do if it's
>> in reset.
>>
>> The easiest way is to write a macro that you can easily add to devices
>> you've checked subsection list (like the way we have a
>> VMSTATE_USB_DEVICE).
> 
> One problem is that as soon as somebody writes a USB controller
> or PCI controller model that can be held in reset, every
> device that could sat behind it automatically now could find
> that it's migrated while it's in reset.
> 

One way to keep the feature without copy-pasting vmsd would be to add
a new vmstate_register with an additional argument to pass the base
class vmsd section and handle the whole thing there.

--
Damien