From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34570) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJkVL-0004PM-8P for qemu-devel@nongnu.org; Thu, 14 Jan 2016 11:12:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJkVE-0000SD-NO for qemu-devel@nongnu.org; Thu, 14 Jan 2016 11:12:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36949) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJkVE-0000Ro-Gc for qemu-devel@nongnu.org; Thu, 14 Jan 2016 11:12:00 -0500 References: <1452761307-57200-1-git-send-email-frankja@linux.vnet.ibm.com> <1452761307-57200-5-git-send-email-frankja@linux.vnet.ibm.com> From: Laszlo Ersek Message-ID: <5697C8CD.4080805@redhat.com> Date: Thu, 14 Jan 2016 17:11:57 +0100 MIME-Version: 1.0 In-Reply-To: <1452761307-57200-5-git-send-email-frankja@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 4/5] scripts/dump-guest-memory.py: Cleanup functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Janosch Frank , qemu-devel@nongnu.org Cc: pbonzini@redhat.com 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 > --- > 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 >