On Fri, 20 Oct 2023 09:30:44 +0200 Juan Quintela <quint...@redhat.com> wrote:
> Greg Kurz <gr...@kaod.org> wrote: > > 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. > >> > >> This is wrong at many levels: > >> - we shouldn't have two VMSTATEDescriptions with 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. > >> > >> Volunteers? > >> > >> CC: Cedric Le Goater <c...@kaod.org> > >> CC: Daniel Henrique Barboza <danielhb...@gmail.com> > >> CC: David Gibson <da...@gibson.dropbear.id.au> > >> CC: Greg Kurz <gr...@kaod.org> > >> > >> Signed-off-by: Juan Quintela <quint...@redhat.com> > >> --- > >> hw/ppc/spapr.c | 7 ++++++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >> index cb840676d3..8531d13492 100644 > >> --- a/hw/ppc/spapr.c > >> +++ b/hw/ppc/spapr.c > >> @@ -143,7 +143,12 @@ static bool pre_2_10_vmstate_dummy_icp_needed(void > >> *opaque) > >> } > >> > >> static const VMStateDescription pre_2_10_vmstate_dummy_icp = { > >> - .name = "icp/server", > >> + /* > >> + * Hack ahead. We can't have two devices with the same name and > >> + * instance id. So I rename this to pass make check. > >> + * Real help from people who knows the hardware is needed. > >> + */ > >> + .name = "pre-2.10-icp/server", > >> .version_id = 1, > >> .minimum_version_id = 1, > >> .needed = pre_2_10_vmstate_dummy_icp_needed, > > > > I guess this fix is acceptable as well and a lot simpler than > > reverting the hack actually. Outcome is the same : drop > > compat with pseries-2.9 and older. > > > > Reviewed-by: Greg Kurz <gr...@kaod.org> > > I fully agree with you here. > The other options given on this thread is deprecate that machines, but I > would like to have this series sooner than 2 releases. Yeah and, especially, the deprecation of all these machine types is itself a massive chunk of work as it will call to identify and remove other related workarounds as well. Given that pretty much everyone working in PPC/PAPR moved away, can the community handle such a big change ? > And what ppc is > doing here is (and has always been) a hack and an abuse about how > vmstate registrations is supposed to work. > Sorry again... We should have involved migration experts at the time. :-) > Thanks, Juan. > Cheers, -- Greg