Re: [PATCH] ppc: fixed incorrect name filed in vmstate_tlbemb_entry

[Arman Nabiev]

Thank you.
Do I need to resend the patch with the specified corrections, or will you do it and all I need is the name in signed-off? In the second case, you need to specify Arman Nabiev. Just in case I'll attach the fixed patch here.

On Thu, 22 Aug 2024 at 16:12, Peter Maydell <peter.maydell@linaro.org> wrote:

On Wed, 21 Aug 2024 at 20:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 21 Aug 2024 at 19:56, Arman Nabiev <nabiev.arman13@gmail.com> wrote:
> >
> > In my example in https://gitlab.com/qemu-project/qemu/-/issues/2522 the .needed function returns true for vmstate_tlbemb, but not for vmstate_tlb6xx. I tried to do some tests without fixing the typo. When I changed the .fields in the two structures to the same value so that the size of the data they stored matched, everything worked. I also changed the order of vmstate_tlb6xx and vmstate_tlbemb in the subsections field of vmstate_ppc_cpu, everything worked as well.
> > According to https://www.qemu.org/docs/master/devel/migration/main.html#:~:text=On%20the%20receiving%20side%2C%20if,that%20didn%E2%80%99t%20send%20the%20subsection and on my own tests I think the problem is that when reading saved data, qemu uses the device name to determine an object that extracts a certain size of data. Since the names are the same for vmstate_tlb6xx and vmstate_tlbemb, it uses the functions for the first one due to a certain order, which leads to an error, since the data from the second one was saved.
>
> Aha, yes, that would explain it -- the PPC CPU has both
> subsections in its subsection list, but they have the
> same name, so we pick the wrong one when we see the
> name in the incoming data.
>
> In that case we can take this fix without worrying
> about a migration compat break, because clearly
> migration has never worked for this CPU type...

I did a quick test and indeed migration doesn't work, not
just record-and-replay:

$ ./build/ppc/qemu-system-ppc -drive
if=none,format=qcow2,file=/home/petmay01/test-images/virt/dummy.qcow2
-monitor stdio -M bamboo
QEMU 9.0.92 monitor - type 'help' for more information
(qemu) savevm foo
(qemu) loadvm foo
Missing section footer for cpu
Error: Error -22 while loading VM state

So I'm happy that this patch is the right fix, and
it can have my

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

provided that we fix a couple of things in the commit message:

(1) For QEMU, Signed-off-by lines should generally be your
full name, not a pseudonym (as I assume "armanincredible" is).

(2) We should give in the commit message details of
what has been fixed and also a Resolves: line for the
gitlab issue, something like:

===begin==
target/ppc: Fix migration of CPUs with TLB_EMB TLB type

In vmstate_tlbemb a cut-and-paste error meant we gave
this vmstate subsection the same "cpu/tlb6xx" name as
the vmstate_tlb6xx subsection. This breaks migration load
for any CPU using the TLB_EMB CPU type, because when we
see the "tlb6xx" name in the incoming data we try to
interpret it as a vmstate_tlb6xx subsection, which it
isn't the right format for:

 $ qemu-system-ppc -drive
if=none,format=qcow2,file=/home/petmay01/test-images/virt/dummy.qcow2
-monitor stdio -M bamboo
 QEMU 9.0.92 monitor - type 'help' for more information
 (qemu) savevm foo
 (qemu) loadvm foo
 Missing section footer for cpu
 Error: Error -22 while loading VM state

Correct the incorrect vmstate section name. Since migration
for these CPU types was completely broken before, we don't
need to care that this is a migration compatibility break.

This affects the PPC 405, 440, 460 and e200 CPU families.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2522
Signed-off-by: [your name here] <nabiev.arman13@gmail.com>
===endit===

thanks
-- PMM

0001-target-ppc-Fix-migration-of-CPUs-with-TLB_EMB-TLB-ty.patch
Description: Text Data