Re: [PATCH v4 02/17] dump: Introduce GuestPhysBlock offset and length filter functions

[Marc-André Lureau]

On Tue, Jul 26, 2022 at 1:23 PM Janosch Frank <frankja@linux.ibm.com> wrote:
>
> The iteration over the memblocks is hard to understand so it's about
> time to clean it up. Instead of manually grabbing the next memblock we
> can use QTAILQ_FOREACH to iterate over all memblocks.
>
> Additionally we move the calculation of the offset and length out by
> using the dump_get_memblock_*() functions.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  dump/dump.c           | 37 +++++++++++++++++++++++++++++++++++++
>  include/sysemu/dump.h |  5 +++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 0ed7cf9c7b..0fd7c76c1e 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -591,6 +591,43 @@ static void dump_begin(DumpState *s, Error **errp)
>      write_elf_notes(s, errp);
>  }
>
> +int64_t dump_get_memblock_size(GuestPhysBlock *block, int64_t 
> filter_area_start,
> +                               int64_t filter_area_length)
> +{
> +    int64_t size, left, right;
> +
> +    /* No filter, return full size */
> +    if (!filter_area_length) {
> +        return block->target_end - block->target_start;
> +    }
> +
> +    /* calculate the overlapped region. */
> +    left = MAX(filter_area_start, block->target_start);
> +    right = MIN(filter_area_start + filter_area_length, block->target_end);
> +    size = right - left;
> +    size = size > 0 ? size : 0;
> +
> +    return size;
> +}
> +
> +int64_t dump_get_memblock_start(GuestPhysBlock *block, int64_t 
> filter_area_start,
> +                                int64_t filter_area_length)
> +{
> +    if (filter_area_length) {
> +        /* return -1 if the block is not within filter area */
> +        if (block->target_start >= filter_area_start + filter_area_length ||
> +            block->target_end <= filter_area_start) {
> +            return -1;
> +        }
> +
> +        if (filter_area_start > block->target_start) {
> +            return filter_area_start - block->target_start;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static int get_next_block(DumpState *s, GuestPhysBlock *block)
>  {
>      while (1) {
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index ffc2ea1072..6ce3c24197 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -203,4 +203,9 @@ typedef struct DumpState {
>  uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
>  uint32_t cpu_to_dump32(DumpState *s, uint32_t val);
>  uint64_t cpu_to_dump64(DumpState *s, uint64_t val);
> +
> +int64_t dump_get_memblock_size(GuestPhysBlock *block, int64_t 
> filter_area_start,
> +                               int64_t filter_area_length);
> +int64_t dump_get_memblock_start(GuestPhysBlock *block, int64_t 
> filter_area_start,
> +                                int64_t filter_area_length);

The functions don't need to be exported. You probably need to
introduce them back with their usage, to avoid some compiler warning.
If you can't split the introduction and related refactoring, then
let's have a single patch.

Thanks

>  #endif
> --
> 2.34.1
>