On 01/14/16 09:48, Janosch Frank wrote:
> Increase readability by adding newlines and comments, as well as
> removing wrong whitespaces and C style braces around conditionals and
> loops.
>
> Signed-off-by: Janosch Frank <fran...@linux.vnet.ibm.com>
> ---
> scripts/dump-guest-memory.py | 71
> +++++++++++++++++++++++++++++++-------------
> 1 file changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index fe93135..2110494 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -69,35 +69,60 @@ ELF64_PHDR = ("I" # p_type
> )
>
> def int128_get64(val):
> - assert (val["hi"] == 0)
> + """Returns low 64bit part of Int128 struct."""
> +
> + assert val["hi"] == 0
> return val["lo"]
>
> +
> def qlist_foreach(head, field_str):
> + """Generator for qlists."""
> +
> var_p = head["lh_first"]
> - while (var_p != 0):
> + while var_p != 0:
> var = var_p.dereference()
> - yield var
> var_p = var[field_str]["le_next"]
> + yield var
> +
>
> def qemu_get_ram_block(ram_addr):
> + """Returns the RAMBlock struct to which the given address belongs."""
> +
> ram_blocks = gdb.parse_and_eval("ram_list.blocks")
> +
> for block in qlist_foreach(ram_blocks, "next"):
> - if (ram_addr - block["offset"] < block["used_length"]):
> + if (ram_addr - block["offset"]) < block["used_length"]:
> return block
> +
> raise gdb.GdbError("Bad ram offset %x" % ram_addr)
>
> +
> def qemu_get_ram_ptr(ram_addr):
> + """Returns qemu vaddr for given guest physical address."""
> +
> block = qemu_get_ram_block(ram_addr)
> return block["host"] + (ram_addr - block["offset"])
>
> -def memory_region_get_ram_ptr(mr):
> - if (mr["alias"] != 0):
> - return (memory_region_get_ram_ptr(mr["alias"].dereference()) +
> - mr["alias_offset"])
> - return qemu_get_ram_ptr(mr["ram_addr"] & TARGET_PAGE_MASK)
> +
> +def memory_region_get_ram_ptr(memory_region):
> + if memory_region["alias"] != 0:
> + return
> (memory_region_get_ram_ptr(memory_region["alias"].dereference())
> + + memory_region["alias_offset"])
> +
> + return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
> +
>
> def get_guest_phys_blocks():
> + """Returns a list of ram blocks.
> +
> + Each block entry contains:
> + 'target_start': guest block phys start address
> + 'target_end': guest block phys end address
> + 'host_addr': qemu vaddr of the block's start
> + """
> +
> guest_phys_blocks = []
> +
> print("guest RAM blocks:")
> print("target_start target_end host_addr message "
> "count")
> @@ -106,30 +131,34 @@ def get_guest_phys_blocks():
>
> current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
> current_map = current_map_p.dereference()
> +
> + # Conversion to int is needed for python 3
> + # compatibility. Otherwise range doesn't cast the value itself and
> + # breaks.
This comment probably belongs to the previous patch.
> for cur in range(int(current_map["nr"])):
> - flat_range = (current_map["ranges"] + cur).dereference()
> - mr = flat_range["mr"].dereference()
> + flat_range = (current_map["ranges"] + cur).dereference()
> + memory_region = flat_range["mr"].dereference()
>
> # we only care about RAM
> - if (not mr["ram"]):
> + if not memory_region["ram"]:
> continue
>
> section_size = int128_get64(flat_range["addr"]["size"])
> target_start = int128_get64(flat_range["addr"]["start"])
> - target_end = target_start + section_size
> - host_addr = (memory_region_get_ram_ptr(mr) +
> - flat_range["offset_in_region"])
> + target_end = target_start + section_size
> + host_addr = (memory_region_get_ram_ptr(memory_region) +
> + flat_range["offset_in_region"])
The way you preserved the line wrapping here (i.e., operator at the end
of the line) is inconsistent with the change you employed above, in
memory_region_get_ram_ptr(). There you moved the operator to the start
of the next line.
> predecessor = None
>
> # find continuity in guest physical address space
> - if (len(guest_phys_blocks) > 0):
> + if len(guest_phys_blocks) > 0:
> predecessor = guest_phys_blocks[-1]
> predecessor_size = (predecessor["target_end"] -
> - predecessor["target_start"])
> + predecessor["target_start"])
Ditto.
>
> # the memory API guarantees monotonically increasing
> # traversal
> - assert (predecessor["target_end"] <= target_start)
> + assert predecessor["target_end"] <= target_start
>
> # we want continuity in both guest-physical and
> # host-virtual memory
> @@ -137,11 +166,11 @@ def get_guest_phys_blocks():
> predecessor["host_addr"] + predecessor_size != host_addr):
> predecessor = None
>
> - if (predecessor is None):
> + if predecessor is None:
> # isolated mapping, add it to the list
> guest_phys_blocks.append({"target_start": target_start,
> - "target_end" : target_end,
> - "host_addr" : host_addr})
> + "target_end": target_end,
> + "host_addr": host_addr})
> message = "added"
> else:
> # expand predecessor until @target_end; predecessor's
>
