qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Thomas Huth
Subject: Re: [PATCH 07/13] RFC migration: icp/server is a mess
Date: Fri, 20 Oct 2023 10:12:52 +0200
User-agent: Mozilla Thunderbird

On 20/10/2023 10.06, Greg Kurz wrote:
On Fri, 20 Oct 2023 09:30:44 +0200
Juan Quintela <quintela@redhat.com> wrote:

Greg Kurz <groug@kaod.org> wrote:
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.

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 <clg@kaod.org>
CC: Daniel Henrique Barboza <danielhb413@gmail.com>
CC: David Gibson <david@gibson.dropbear.id.au>
CC: Greg Kurz <groug@kaod.org>

Signed-off-by: Juan Quintela <quintela@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 <groug@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 ?

I think you could treat that as two work items. First the deprecation and removal of old machine types. Second the (optional) cleanups. If we don't immediately manage to find and remove each and every possible spot that could be cleaned up after the removal of the machine types, so be it. But better at least *start* to remove the old cruft, beginning with the machine type, than dragging this stuff along forever.

 Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]