[Qemu-devel] [PATCH qom-cpu-next] cpus: Replace open-coded CPU loop in qmp_memsave() with qemu_get_cpu()
No functional change, just less usages of first_cpu and next_cpu fields. Signed-off-by: Andreas Färber --- cpus.c | 11 +++ 1 Datei geändert, 3 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-) diff --git a/cpus.c b/cpus.c index 41779eb..845e915 100644 --- a/cpus.c +++ b/cpus.c @@ -1262,18 +1262,13 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename, cpu_index = 0; } -for (env = first_cpu; env; env = env->next_cpu) { -cpu = ENV_GET_CPU(env); -if (cpu_index == cpu->cpu_index) { -break; -} -} - -if (env == NULL) { +cpu = qemu_get_cpu(cpu_index); +if (cpu == NULL) { error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index", "a CPU number"); return; } +env = cpu->env_ptr; f = fopen(filename, "wb"); if (!f) { -- 1.7.10.4
Re: [Qemu-devel] [PATCH qom-cpu-next] cpus: Replace open-coded CPU loop in qmp_memsave() with qemu_get_cpu()
On Fri, 15 Feb 2013 15:47:41 +0100 Andreas Färber wrote: > No functional change, just less usages of first_cpu and next_cpu fields. > > Signed-off-by: Andreas Färber Reviewed-by: Luiz Capitulino One comment below. > --- > cpus.c | 11 +++ > 1 Datei geändert, 3 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-) > > diff --git a/cpus.c b/cpus.c > index 41779eb..845e915 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1262,18 +1262,13 @@ void qmp_memsave(int64_t addr, int64_t size, const > char *filename, > cpu_index = 0; > } > > -for (env = first_cpu; env; env = env->next_cpu) { > -cpu = ENV_GET_CPU(env); > -if (cpu_index == cpu->cpu_index) { > -break; > -} > -} > - > -if (env == NULL) { > +cpu = qemu_get_cpu(cpu_index); > +if (cpu == NULL) { > error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index", >"a CPU number"); > return; > } > +env = cpu->env_ptr; I wonder if the callees should be taking a CPUState object instead of CPUArchState. > > f = fopen(filename, "wb"); > if (!f) {
Re: [Qemu-devel] [PATCH qom-cpu-next] cpus: Replace open-coded CPU loop in qmp_memsave() with qemu_get_cpu()
Am 18.02.2013 13:57, schrieb Luiz Capitulino: > On Fri, 15 Feb 2013 15:47:41 +0100 > Andreas Färber wrote: > >> No functional change, just less usages of first_cpu and next_cpu fields. >> >> Signed-off-by: Andreas Färber > > Reviewed-by: Luiz Capitulino > > One comment below. > >> --- >> cpus.c | 11 +++ >> 1 Datei geändert, 3 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-) >> >> diff --git a/cpus.c b/cpus.c >> index 41779eb..845e915 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -1262,18 +1262,13 @@ void qmp_memsave(int64_t addr, int64_t size, const >> char *filename, >> cpu_index = 0; >> } >> >> -for (env = first_cpu; env; env = env->next_cpu) { >> -cpu = ENV_GET_CPU(env); >> -if (cpu_index == cpu->cpu_index) { >> -break; >> -} >> -} >> - >> -if (env == NULL) { >> +cpu = qemu_get_cpu(cpu_index); >> +if (cpu == NULL) { >> error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index", >>"a CPU number"); >> return; >> } >> +env = cpu->env_ptr; > > I wonder if the callees should be taking a CPUState object instead of > CPUArchState. In general yes. However this was a side-product of a bug investigation for ppc, re-reviewing my cpu_index field movement patch. The user here is cpu_memory_rw_debug(), which for softmmu calls cpu_get_phys_page_debug(), which in turn is a target-specific global function. Both have quite a number of callers, so I'd like to schedule that after my pending qom-cpu-9 refactorings to avoid conflicts, and cpu_get_phys_page_debug() feels like a candidate for a CPUState callback to avoid target-specific globals. Independently, the function needs to stay in exec.c since it uses TARGET_PAGE_{SIZE,MASK} for its implementation. I appreciate you thinking of this though! Thanks, applied to qom-cpu-next (with extended commit message): https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next Regards, Andreas >> >> f = fopen(filename, "wb"); >> if (!f) { -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg