| Message ID | 1370794247-28267-14-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On Sun, 9 Jun 2013 18:10:42 +0200 Andreas Färber <afaerber@suse.de> wrote: > We relied on the CPUClass::get_memory_mapping() implementation being a > no-op when paging is disabled for that CPU. Therefore we can just as well > iterate over all CPUs to retrieve mappings. > > This allows to use qemu_for_each_cpu() instead of open-coding CPU loops. > > Pass any Error out into dump_init() and have it actually stop on errors. > Whether it is unsupported on a certain CPU can be checked by looking for > a NULL CPUClass::get_memory_mapping field. > > Signed-off-by: Andreas Färber <afaerber@suse.de> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > dump.c | 7 +++++- > include/sysemu/memory_mapping.h | 8 +------ > memory_mapping.c | 52 ++++++++++++++++++++++++----------------- > 3 files changed, 38 insertions(+), 29 deletions(-) > > diff --git a/dump.c b/dump.c > index c0d3da5..b44dafc 100644 > --- a/dump.c > +++ b/dump.c > @@ -706,6 +706,7 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter, > { > CPUArchState *env; > int nr_cpus; > + Error *err = NULL; > int ret; > > if (runstate_is_running()) { > @@ -756,7 +757,11 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter, > /* get memory mapping */ > memory_mapping_list_init(&s->list); > if (paging) { > - qemu_get_guest_memory_mapping(&s->list); > + qemu_get_guest_memory_mapping(&s->list, &err); > + if (err != NULL) { > + error_propagate(errp, err); > + goto cleanup; > + } > } else { > qemu_get_guest_simple_memory_mapping(&s->list); > } > diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h > index c47e6ee..6dfb68d 100644 > --- a/include/sysemu/memory_mapping.h > +++ b/include/sysemu/memory_mapping.h > @@ -45,13 +45,7 @@ void memory_mapping_list_free(MemoryMappingList *list); > > void memory_mapping_list_init(MemoryMappingList *list); > > -/* > - * Return value: > - * 0: success > - * -1: failed > - * -2: unsupported > - */ > -int qemu_get_guest_memory_mapping(MemoryMappingList *list); > +void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp); > > /* get guest's memory mapping without do paging(virtual address is 0). */ > void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list); > diff --git a/memory_mapping.c b/memory_mapping.c > index 9bd24ce..a19be54 100644 > --- a/memory_mapping.c > +++ b/memory_mapping.c > @@ -165,36 +165,48 @@ void memory_mapping_list_init(MemoryMappingList *list) > QTAILQ_INIT(&list->head); > } > > -static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu) > +static void find_paging_enabled_cpu(CPUState *cpu, void *data) > { > - CPUArchState *env; > + bool *ret = data; > > - for (env = start_cpu; env != NULL; env = env->next_cpu) { > - if (cpu_paging_enabled(ENV_GET_CPU(env))) { > - return env; > - } > + if (*ret) { > + return; > } > + *ret = cpu_paging_enabled(cpu); > +} > + > +typedef struct GetGuestMemoryMappingData { > + MemoryMappingList *list; > + Error *err; > +} GetGuestMemoryMappingData; > + > +static void qemu_get_one_guest_memory_mapping(CPUState *cpu, void *data) > +{ > + GetGuestMemoryMappingData *s = data; > > - return NULL; > + if (s->err != NULL || !cpu_paging_enabled(cpu)) { > + return; > + } > + cpu_get_memory_mapping(cpu, s->list, &s->err); > } > > -int qemu_get_guest_memory_mapping(MemoryMappingList *list) > +void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp) > { > - CPUArchState *env, *first_paging_enabled_cpu; > + GetGuestMemoryMappingData s = { > + .list = list, > + .err = NULL, > + }; > + bool paging_enabled = false; > RAMBlock *block; > ram_addr_t offset, length; > > - first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu); > - if (first_paging_enabled_cpu) { > - for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) { > - Error *err = NULL; > - cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err); > - if (err) { > - error_free(err); > - return -1; > - } > + qemu_for_each_cpu(find_paging_enabled_cpu, &paging_enabled); > + if (paging_enabled) { > + qemu_for_each_cpu(qemu_get_one_guest_memory_mapping, &s); > + if (s.err != NULL) { > + error_propagate(errp, s.err); > } > - return 0; > + return; > } > > /* > @@ -206,8 +218,6 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list) > length = block->length; > create_new_memory_mapping(list, offset, offset, length); > } > - > - return 0; > } > > void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
Am 11.06.2013 17:52, schrieb Luiz Capitulino: > On Sun, 9 Jun 2013 18:10:42 +0200 > Andreas Färber <afaerber@suse.de> wrote: > >> We relied on the CPUClass::get_memory_mapping() implementation being a >> no-op when paging is disabled for that CPU. Therefore we can just as well >> iterate over all CPUs to retrieve mappings. >> >> This allows to use qemu_for_each_cpu() instead of open-coding CPU loops. >> >> Pass any Error out into dump_init() and have it actually stop on errors. >> Whether it is unsupported on a certain CPU can be checked by looking for >> a NULL CPUClass::get_memory_mapping field. >> >> Signed-off-by: Andreas Färber <afaerber@suse.de> > > Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> Thanks, reverted CPU loop changes and applied to qom-cpu as pure error handling improvement: https://github.com/afaerber/qemu-cpu/commits/qom-cpu Andreas
diff --git a/dump.c b/dump.c index c0d3da5..b44dafc 100644 --- a/dump.c +++ b/dump.c @@ -706,6 +706,7 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter, { CPUArchState *env; int nr_cpus; + Error *err = NULL; int ret; if (runstate_is_running()) { @@ -756,7 +757,11 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter, /* get memory mapping */ memory_mapping_list_init(&s->list); if (paging) { - qemu_get_guest_memory_mapping(&s->list); + qemu_get_guest_memory_mapping(&s->list, &err); + if (err != NULL) { + error_propagate(errp, err); + goto cleanup; + } } else { qemu_get_guest_simple_memory_mapping(&s->list); } diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h index c47e6ee..6dfb68d 100644 --- a/include/sysemu/memory_mapping.h +++ b/include/sysemu/memory_mapping.h @@ -45,13 +45,7 @@ void memory_mapping_list_free(MemoryMappingList *list); void memory_mapping_list_init(MemoryMappingList *list); -/* - * Return value: - * 0: success - * -1: failed - * -2: unsupported - */ -int qemu_get_guest_memory_mapping(MemoryMappingList *list); +void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp); /* get guest's memory mapping without do paging(virtual address is 0). */ void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list); diff --git a/memory_mapping.c b/memory_mapping.c index 9bd24ce..a19be54 100644 --- a/memory_mapping.c +++ b/memory_mapping.c @@ -165,36 +165,48 @@ void memory_mapping_list_init(MemoryMappingList *list) QTAILQ_INIT(&list->head); } -static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu) +static void find_paging_enabled_cpu(CPUState *cpu, void *data) { - CPUArchState *env; + bool *ret = data; - for (env = start_cpu; env != NULL; env = env->next_cpu) { - if (cpu_paging_enabled(ENV_GET_CPU(env))) { - return env; - } + if (*ret) { + return; } + *ret = cpu_paging_enabled(cpu); +} + +typedef struct GetGuestMemoryMappingData { + MemoryMappingList *list; + Error *err; +} GetGuestMemoryMappingData; + +static void qemu_get_one_guest_memory_mapping(CPUState *cpu, void *data) +{ + GetGuestMemoryMappingData *s = data; - return NULL; + if (s->err != NULL || !cpu_paging_enabled(cpu)) { + return; + } + cpu_get_memory_mapping(cpu, s->list, &s->err); } -int qemu_get_guest_memory_mapping(MemoryMappingList *list) +void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp) { - CPUArchState *env, *first_paging_enabled_cpu; + GetGuestMemoryMappingData s = { + .list = list, + .err = NULL, + }; + bool paging_enabled = false; RAMBlock *block; ram_addr_t offset, length; - first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu); - if (first_paging_enabled_cpu) { - for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) { - Error *err = NULL; - cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err); - if (err) { - error_free(err); - return -1; - } + qemu_for_each_cpu(find_paging_enabled_cpu, &paging_enabled); + if (paging_enabled) { + qemu_for_each_cpu(qemu_get_one_guest_memory_mapping, &s); + if (s.err != NULL) { + error_propagate(errp, s.err); } - return 0; + return; } /* @@ -206,8 +218,6 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list) length = block->length; create_new_memory_mapping(list, offset, offset, length); } - - return 0; } void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
We relied on the CPUClass::get_memory_mapping() implementation being a no-op when paging is disabled for that CPU. Therefore we can just as well iterate over all CPUs to retrieve mappings. This allows to use qemu_for_each_cpu() instead of open-coding CPU loops. Pass any Error out into dump_init() and have it actually stop on errors. Whether it is unsupported on a certain CPU can be checked by looking for a NULL CPUClass::get_memory_mapping field. Signed-off-by: Andreas Färber <afaerber@suse.de> --- dump.c | 7 +++++- include/sysemu/memory_mapping.h | 8 +------ memory_mapping.c | 52 ++++++++++++++++++++++++----------------- 3 files changed, 38 insertions(+), 29 deletions(-)