On 10/17/2026 1:42 AM, [email protected] wrote: >> Now that all targets implement this interface, we can replace the >> cpu_get_dump_info() call with the new interface. >> >> We always check if there is at least one cpu available. For instance, >> machine null has no cpu present. Heterogeneous emulation is >> already handled by the check above which disable dump. >> >> Reviewed-by: Anton Johansson <[email protected]> >> Signed-off-by: Pierrick Bouvier <[email protected]> >> Message-ID: <[email protected]> >> >> diff --git a/dump/dump.c b/dump/dump.c >> index 0ecff682dd4e..e0a7dfd58785 100644 >> --- a/dump/dump.c >> +++ b/dump/dump.c >> @@ -32,6 +32,7 @@ >> #include "migration/blocker.h" >> #include "migration/misc.h" >> #include "hw/core/cpu.h" >> +#include "hw/core/sysemu-cpu-ops.h" >> #include "win_dump.h" >> #include "qemu/range.h" >> >> @@ -1793,7 +1794,6 @@ static void dump_init(DumpState *s, int fd, bool >> has_format, >> VMCoreInfoState *vmci = vmcoreinfo_find(); >> CPUState *cpu; >> int nr_cpus; >> - int ret; >> >> if (target_heterogeneous()) { >> error_setg(errp, >> @@ -1858,12 +1858,16 @@ static void dump_init(DumpState *s, int fd, bool >> has_format, >> goto cleanup; >> } >> >> - /* get dump info: endian, class and architecture. >> - * If the target architecture is not supported, cpu_get_dump_info() will >> - * return -1. >> + /* >> + * get dump info: endian, class and architecture. >> + * If the cpu does not support dumping this information, get_dump_info >> + * returns -1. >> */ >> - ret = cpu_get_dump_info(&s->dump_info, &s->guest_phys_blocks); >> - if (ret < 0) { >> + const SysemuCPUOps *sysemu_ops = >> + first_cpu ? CPU_GET_CLASS(first_cpu)->sysemu_ops : NULL; > > Per style, avoid mixed declarations, please declare it at the top of > dump_init(). > >> + if (!sysemu_ops || !sysemu_ops->get_dump_info || >> + sysemu_ops->get_dump_info(first_cpu, &s->dump_info, >> + &s->guest_phys_blocks) < 0) { >> error_setg(errp, >> "dumping guest memory is not supported on this target"); >> goto cleanup; > > Something like this would be clearer: > > if (!first_cpu) { > error_setg(errp, "dump: no CPU to dump"); > goto cleanup; > } > sysemu_ops = first_cpu->cc->sysemu_ops; > if (!sysemu_ops->get_dump_info || > sysemu_ops->get_dump_info(first_cpu, &s->dump_info, > &s->guest_phys_blocks) < 0) { > > Another option is a cpu_get_dump_info(CPUState *, ...) wrapper in > hw/core/cpu-system.c, next to cpu_write_elf64_note() and friends. That would > match how the other dump-related sysemu ops are called and keep dump.c from > dereferencing SysemuCPUOps directly. >
Works for me, and it follows other ones indeed. I'll do the change for next version. Thanks, Pierrick
