On Tue, Jul 26, 2022 at 5:05 PM Janosch Frank <fran...@linux.ibm.com> wrote:
> On 7/26/22 13:35, Marc-André Lureau wrote:
> > On Tue, Jul 26, 2022 at 1:23 PM Janosch Frank <fran...@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 <fran...@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.
>
> Right, I'll add them in the last s390 dump patch and make them static
>
> > If you can't split the introduction and related refactoring, then
> > let's have a single patch.
>
> So squashing this with the next one but leave the other refactoring
> patches (dump_calculate_size() and get_start_block()) as they are?
>
>
Right, if you can't split it further.
--
Marc-André Lureau
