On 7/26/22 13:35, Marc-André Lureau wrote:
> 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.
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.