On Wed, 21 Aug 2024 at 20:33, Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Wed, 21 Aug 2024 at 19:56, Arman Nabiev <nabiev.arma...@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.mayd...@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-sta...@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2522 Signed-off-by: [your name here] <nabiev.arma...@gmail.com> ===endit=== thanks -- PMM
