Re: [PATCH 07/13] RFC migration: icp/server is a mess

[Thomas Huth <thuth@redhat.com>]

On 19/10/2023 23.15, Cédric Le Goater wrote:
> On 10/19/23 22:49, Greg Kurz wrote:
>> Hi Juan,
>>
>> On Thu, 19 Oct 2023 21:08:25 +0200
>> Juan Quintela <quintela@redhat.com> wrote:
>>
>>> Current code does:
>>> - register pre_2_10_vmstate_dummy_icp with "icp/server" and instance
>>>    dependinfg on cpu number
>>> - for newer machines, it register vmstate_icp with "icp/server" name
>>>    and instance 0
>>> - now it unregisters "icp/server" for the 1st instance.
>>>
>>
>> Heh I remember about this hack... it was caused by some rework in
>> the interrupt controller that broke migration.
>>
>>> This is wrong at many levels:
>>> - we shouldn't have two VMSTATEDescriptions with the same name
>>
>> I don't know how bad it is. The idea here is to send extra
>> state in the stream because older QEMU expect it (but won't use
>> it), so it made sense to keep the same name.
>>
>>> - In case this is the only solution that we can came with, it needs to
>>>    be:
>>>    * register pre_2_10_vmstate_dummy_icp
>>>    * unregister pre_2_10_vmstate_dummy_icp
>>>    * register real vmstate_icp
>>>
>>> As the initialization of this machine is already complex enough, I
>>> need help from PPC maintainers to fix this.
>>>
>>
>> What about dropping all this code, i.e. basically reverting 46f7afa37096 
>> ("spapr:
>> fix migration of ICPState objects from/to older QEMU") ?
> 
> I'd vote for removing the dummy ICP states for pre-2.10 pseries machines.
> Migration compatibility would be broken for these old versions but, with
> a clear error report, it should be more than enough. I doubt anyone will
> need such a feature now days.

In that case: Please also put the pseries-2.1 machine up to pseries-2.9 onto 
the deprecation list, so that they can finally get removed after two 
releases. It does not make sense to keep compat machines around if the 
compatibility is not available anymore.

  Thomas