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


Reply via email to