On Thu, 2022-08-11 at 12:10 +0000, Janosch Frank wrote:
> get_start_block() returns the start address of the first memory block
> or -1.
>
> With the GuestPhysBlock iterator conversion we don't need to set the
> start address and can therefore remove that code and the "start"
> DumpState struct member. The only functionality left is the validation
> of the start block so it only makes sense to re-name the function to
> validate_start_block()
Nit, since you don't return an address anymore, I find retaining the -
1/0 return value instead of true/false weird.
>
> Signed-off-by: Janosch Frank <fran...@linux.ibm.com>
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> Reviewed-by: Janis Schoetterl-Glausch <s...@linux.ibm.com>
> ---
> dump/dump.c | 20 ++++++--------------
> include/sysemu/dump.h | 2 --
> 2 files changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 340de5a1e7..e204912a89 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -1500,30 +1500,22 @@ static void create_kdump_vmcore(DumpState *s, Error
> **errp)
> }
> }
>
> -static ram_addr_t get_start_block(DumpState *s)
> +static int validate_start_block(DumpState *s)
> {
> GuestPhysBlock *block;
>
> if (!s->has_filter) {
> - s->next_block = QTAILQ_FIRST(&s->guest_phys_blocks.head);
> return 0;
> }
>
> QTAILQ_FOREACH(block, &s->guest_phys_blocks.head, next) {
> + /* This block is out of the range */
> if (block->target_start >= s->begin + s->length ||
> block->target_end <= s->begin) {
> - /* This block is out of the range */
> continue;
> }
> -
> - s->next_block = block;
> - if (s->begin > block->target_start) {
> - s->start = s->begin - block->target_start;
> - } else {
> - s->start = 0;
> - }
> - return s->start;
> - }
> + return 0;
> + }
>
> return -1;
> }
> @@ -1670,8 +1662,8 @@ static void dump_init(DumpState *s, int fd, bool
> has_format,
> goto cleanup;
> }
>
> - s->start = get_start_block(s);
> - if (s->start == -1) {
> + /* Is the filter filtering everything? */
> + if (validate_start_block(s) == -1) {
> error_setg(errp, QERR_INVALID_PARAMETER, "begin");
> goto cleanup;
> }
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index ffc2ea1072..7fce1d4af6 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -166,8 +166,6 @@ typedef struct DumpState {
> hwaddr memory_offset;
> int fd;
>
> - GuestPhysBlock *next_block;
> - ram_addr_t start;
> bool has_filter;
> int64_t begin;
> int64_t length;
