[Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Janosch Frank]

The dump guest memory script for extracting a Linux core from a qemu
core is currently limited to amd64 and python 2.

With this series we add support for python 3 (while maintaining python
2 support) and add the possibility to extract dumps from VMs with the
most common architectures.

This was tested on a s390 s12 guest only, I'd appreciate tests for the
other architectures.

Janosch Frank (5):
  scripts/dump-guest-memory.py: Move constants to the top
  scripts/dump-guest-memory.py: Make methods functions
  scripts/dump-guest-memory.py: Improve python 3 compatibility
  scripts/dump-guest-memory.py: Cleanup functions
  scripts/dump-guest-memory.py: Introduce multi-arch support

 scripts/dump-guest-memory.py | 717 +++++++++++++++++++++++++++----------------
 1 file changed, 453 insertions(+), 264 deletions(-)

-- 
2.3.0

[Qemu-devel] [RFC 1/5] scripts/dump-guest-memory.py: Move constants to the top

[Janosch Frank]

The constants bloated the class definition and were therefore moved to
the top.

Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 scripts/dump-guest-memory.py | 126 +++++++++++++++++++++----------------------
 1 file changed, 63 insertions(+), 63 deletions(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 08796ff..e49c835 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -17,6 +17,55 @@
 
 import struct
 
+TARGET_PAGE_SIZE = 0x1000
+TARGET_PAGE_MASK = 0xFFFFFFFFFFFFF000
+
+# Various ELF constants
+EM_X86_64   = 62        # AMD x86-64 target machine
+ELFDATA2LSB = 1         # little endian
+ELFCLASS64  = 2
+ELFMAG      = "\x7FELF"
+EV_CURRENT  = 1
+ET_CORE     = 4
+PT_LOAD     = 1
+PT_NOTE     = 4
+
+# Special value for e_phnum. This indicates that the real number of
+# program headers is too large to fit into e_phnum. Instead the real
+# value is in the field sh_info of section 0.
+PN_XNUM = 0xFFFF
+
+# Format strings for packing and header size calculation.
+ELF64_EHDR = ("4s" # e_ident/magic
+              "B"  # e_ident/class
+              "B"  # e_ident/data
+              "B"  # e_ident/version
+              "B"  # e_ident/osabi
+              "8s" # e_ident/pad
+              "H"  # e_type
+              "H"  # e_machine
+              "I"  # e_version
+              "Q"  # e_entry
+              "Q"  # e_phoff
+              "Q"  # e_shoff
+              "I"  # e_flags
+              "H"  # e_ehsize
+              "H"  # e_phentsize
+              "H"  # e_phnum
+              "H"  # e_shentsize
+              "H"  # e_shnum
+              "H"  # e_shstrndx
+          )
+ELF64_PHDR = ("I"  # p_type
+              "I"  # p_flags
+              "Q"  # p_offset
+              "Q"  # p_vaddr
+              "Q"  # p_paddr
+              "Q"  # p_filesz
+              "Q"  # p_memsz
+              "Q"  # p_align
+          )
+
 class DumpGuestMemory(gdb.Command):
     """Extract guest vmcore from qemu process coredump.
 
@@ -47,62 +96,13 @@ deliberately called abort(), or it was dumped in response to a signal at
 a halfway fortunate point, then its coredump should be in reasonable
 shape and this command should mostly work."""
 
-    TARGET_PAGE_SIZE = 0x1000
-    TARGET_PAGE_MASK = 0xFFFFFFFFFFFFF000
-
-    # Various ELF constants
-    EM_X86_64   = 62        # AMD x86-64 target machine
-    ELFDATA2LSB = 1         # little endian
-    ELFCLASS64  = 2
-    ELFMAG      = "\x7FELF"
-    EV_CURRENT  = 1
-    ET_CORE     = 4
-    PT_LOAD     = 1
-    PT_NOTE     = 4
-
-    # Special value for e_phnum. This indicates that the real number of
-    # program headers is too large to fit into e_phnum. Instead the real
-    # value is in the field sh_info of section 0.
-    PN_XNUM = 0xFFFF
-
-    # Format strings for packing and header size calculation.
-    ELF64_EHDR = ("4s" # e_ident/magic
-                  "B"  # e_ident/class
-                  "B"  # e_ident/data
-                  "B"  # e_ident/version
-                  "B"  # e_ident/osabi
-                  "8s" # e_ident/pad
-                  "H"  # e_type
-                  "H"  # e_machine
-                  "I"  # e_version
-                  "Q"  # e_entry
-                  "Q"  # e_phoff
-                  "Q"  # e_shoff
-                  "I"  # e_flags
-                  "H"  # e_ehsize
-                  "H"  # e_phentsize
-                  "H"  # e_phnum
-                  "H"  # e_shentsize
-                  "H"  # e_shnum
-                  "H"  # e_shstrndx
-                 )
-    ELF64_PHDR = ("I"  # p_type
-                  "I"  # p_flags
-                  "Q"  # p_offset
-                  "Q"  # p_vaddr
-                  "Q"  # p_paddr
-                  "Q"  # p_filesz
-                  "Q"  # p_memsz
-                  "Q"  # p_align
-                 )
-
     def __init__(self):
         super(DumpGuestMemory, self).__init__("dump-guest-memory",
                                               gdb.COMMAND_DATA,
                                               gdb.COMPLETE_FILENAME)
         self.uintptr_t     = gdb.lookup_type("uintptr_t")
-        self.elf64_ehdr_le = struct.Struct("<%s" % self.ELF64_EHDR)
-        self.elf64_phdr_le = struct.Struct("<%s" % self.ELF64_PHDR)
+        self.elf64_ehdr_le = struct.Struct("<%s" % ELF64_EHDR)
+        self.elf64_phdr_le = struct.Struct("<%s" % ELF64_PHDR)
 
     def int128_get64(self, val):
         assert (val["hi"] == 0)
@@ -130,7 +130,7 @@ shape and this command should mostly work."""
         if (mr["alias"] != 0):
             return (self.memory_region_get_ram_ptr(mr["alias"].dereference()) +
                     mr["alias_offset"])
-        return self.qemu_get_ram_ptr(mr["ram_addr"] & self.TARGET_PAGE_MASK)
+        return self.qemu_get_ram_ptr(mr["ram_addr"] & TARGET_PAGE_MASK)
 
     def guest_phys_blocks_init(self):
         self.guest_phys_blocks = []
@@ -198,21 +198,21 @@ shape and this command should mostly work."""
         # most common values. This also means that instruction pointer
         # etc. will be bogus in the dump, but at least the RAM contents
         # should be valid.
-        self.dump_info = {"d_machine": self.EM_X86_64,
-                          "d_endian" : self.ELFDATA2LSB,
-                          "d_class"  : self.ELFCLASS64}
+        self.dump_info = {"d_machine": EM_X86_64,
+                          "d_endian" : ELFDATA2LSB,
+                          "d_class"  : ELFCLASS64}
 
     def encode_elf64_ehdr_le(self):
         return self.elf64_ehdr_le.pack(
-                                 self.ELFMAG,                 # e_ident/magic
+                                 ELFMAG,                      # e_ident/magic
                                  self.dump_info["d_class"],   # e_ident/class
                                  self.dump_info["d_endian"],  # e_ident/data
-                                 self.EV_CURRENT,             # e_ident/version
+                                 EV_CURRENT,                  # e_ident/version
                                  0,                           # e_ident/osabi
                                  "",                          # e_ident/pad
-                                 self.ET_CORE,                # e_type
+                                 ET_CORE,                     # e_type
                                  self.dump_info["d_machine"], # e_machine
-                                 self.EV_CURRENT,             # e_version
+                                 EV_CURRENT,                  # e_version
                                  0,                           # e_entry
                                  self.elf64_ehdr_le.size,     # e_phoff
                                  0,                           # e_shoff
@@ -226,7 +226,7 @@ shape and this command should mostly work."""
                                 )
 
     def encode_elf64_note_le(self):
-        return self.elf64_phdr_le.pack(self.PT_NOTE,         # p_type
+        return self.elf64_phdr_le.pack(PT_NOTE,              # p_type
                                        0,                    # p_flags
                                        (self.memory_offset -
                                         len(self.note)),     # p_offset
@@ -238,7 +238,7 @@ shape and this command should mostly work."""
                                       )
 
     def encode_elf64_load_le(self, offset, start_hwaddr, range_size):
-        return self.elf64_phdr_le.pack(self.PT_LOAD, # p_type
+        return self.elf64_phdr_le.pack(PT_LOAD,      # p_type
                                        0,            # p_flags
                                        offset,       # p_offset
                                        0,            # p_vaddr
@@ -276,7 +276,7 @@ shape and this command should mostly work."""
         # We should never reach PN_XNUM for paging=false dumps: there's
         # just a handful of discontiguous ranges after merging.
         self.phdr_num += len(self.guest_phys_blocks)
-        assert (self.phdr_num < self.PN_XNUM)
+        assert (self.phdr_num < PN_XNUM)
 
         # Calculate the ELF file offset where the memory dump commences:
         #
@@ -312,7 +312,7 @@ shape and this command should mostly work."""
             print ("dumping range at %016x for length %016x" %
                    (cur.cast(self.uintptr_t), left))
             while (left > 0):
-                chunk_size = min(self.TARGET_PAGE_SIZE, left)
+                chunk_size = min(TARGET_PAGE_SIZE, left)
                 chunk = qemu_core.read_memory(cur, chunk_size)
                 vmcore.write(chunk)
                 cur  += chunk_size
-- 
2.3.0

[Qemu-devel] [RFC 2/5] scripts/dump-guest-memory.py: Make methods functions

[Janosch Frank]

The functions dealing with qemu components rarely used parts of the
class, so they were moved out of the class.

As the uintptr_t variable is needed both within and outside the class,
it was made a constant and moved to the top.

Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 scripts/dump-guest-memory.py | 184 ++++++++++++++++++++++---------------------
 1 file changed, 93 insertions(+), 91 deletions(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index e49c835..76a6ecb 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -17,6 +17,8 @@
 
 import struct
 
+UINTPTR_T = gdb.lookup_type("uintptr_t")
+
 TARGET_PAGE_SIZE = 0x1000
 TARGET_PAGE_MASK = 0xFFFFFFFFFFFFF000
 
@@ -66,6 +68,94 @@ ELF64_PHDR = ("I"  # p_type
               "Q"  # p_align
           )
 
+def int128_get64(val):
+    assert (val["hi"] == 0)
+    return val["lo"]
+
+def qlist_foreach(head, field_str):
+    var_p = head["lh_first"]
+    while (var_p != 0):
+        var = var_p.dereference()
+        yield var
+        var_p = var[field_str]["le_next"]
+
+def qemu_get_ram_block(ram_addr):
+    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"]):
+            return block
+    raise gdb.GdbError("Bad ram offset %x" % ram_addr)
+
+def qemu_get_ram_ptr(ram_addr):
+    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 get_guest_phys_blocks():
+    guest_phys_blocks = []
+    print "guest RAM blocks:"
+    print ("target_start     target_end       host_addr        message "
+           "count")
+    print ("---------------- ---------------- ---------------- ------- "
+           "-----")
+
+    current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
+    current_map = current_map_p.dereference()
+    for cur in range(current_map["nr"]):
+        flat_range   = (current_map["ranges"] + cur).dereference()
+        mr           = flat_range["mr"].dereference()
+
+        # we only care about RAM
+        if (not mr["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"])
+        predecessor = None
+
+        # find continuity in guest physical address space
+        if (len(guest_phys_blocks) > 0):
+            predecessor = guest_phys_blocks[-1]
+            predecessor_size = (predecessor["target_end"] -
+                                    predecessor["target_start"])
+
+            # the memory API guarantees monotonically increasing
+            # traversal
+            assert (predecessor["target_end"] <= target_start)
+
+            # we want continuity in both guest-physical and
+            # host-virtual memory
+            if (predecessor["target_end"] < target_start or
+                predecessor["host_addr"] + predecessor_size != host_addr):
+                predecessor = 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})
+            message = "added"
+        else:
+            # expand predecessor until @target_end; predecessor's
+            # start doesn't change
+            predecessor["target_end"] = target_end
+            message = "joined"
+
+        print ("%016x %016x %016x %-7s %5u" %
+               (target_start, target_end, host_addr.cast(UINTPTR_T),
+                message, len(guest_phys_blocks)))
+
+        return guest_phys_blocks
+
+
 class DumpGuestMemory(gdb.Command):
     """Extract guest vmcore from qemu process coredump.
 
@@ -100,96 +190,9 @@ shape and this command should mostly work."""
         super(DumpGuestMemory, self).__init__("dump-guest-memory",
                                               gdb.COMMAND_DATA,
                                               gdb.COMPLETE_FILENAME)
-        self.uintptr_t     = gdb.lookup_type("uintptr_t")
         self.elf64_ehdr_le = struct.Struct("<%s" % ELF64_EHDR)
         self.elf64_phdr_le = struct.Struct("<%s" % ELF64_PHDR)
-
-    def int128_get64(self, val):
-        assert (val["hi"] == 0)
-        return val["lo"]
-
-    def qlist_foreach(self, head, field_str):
-        var_p = head["lh_first"]
-        while (var_p != 0):
-            var = var_p.dereference()
-            yield var
-            var_p = var[field_str]["le_next"]
-
-    def qemu_get_ram_block(self, ram_addr):
-        ram_blocks = gdb.parse_and_eval("ram_list.blocks")
-        for block in self.qlist_foreach(ram_blocks, "next"):
-            if (ram_addr - block["offset"] < block["used_length"]):
-                return block
-        raise gdb.GdbError("Bad ram offset %x" % ram_addr)
-
-    def qemu_get_ram_ptr(self, ram_addr):
-        block = self.qemu_get_ram_block(ram_addr)
-        return block["host"] + (ram_addr - block["offset"])
-
-    def memory_region_get_ram_ptr(self, mr):
-        if (mr["alias"] != 0):
-            return (self.memory_region_get_ram_ptr(mr["alias"].dereference()) +
-                    mr["alias_offset"])
-        return self.qemu_get_ram_ptr(mr["ram_addr"] & TARGET_PAGE_MASK)
-
-    def guest_phys_blocks_init(self):
-        self.guest_phys_blocks = []
-
-    def guest_phys_blocks_append(self):
-        print "guest RAM blocks:"
-        print ("target_start     target_end       host_addr        message "
-               "count")
-        print ("---------------- ---------------- ---------------- ------- "
-               "-----")
-
-        current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
-        current_map = current_map_p.dereference()
-        for cur in range(current_map["nr"]):
-            flat_range   = (current_map["ranges"] + cur).dereference()
-            mr           = flat_range["mr"].dereference()
-
-            # we only care about RAM
-            if (not mr["ram"]):
-                continue
-
-            section_size = self.int128_get64(flat_range["addr"]["size"])
-            target_start = self.int128_get64(flat_range["addr"]["start"])
-            target_end   = target_start + section_size
-            host_addr    = (self.memory_region_get_ram_ptr(mr) +
-                            flat_range["offset_in_region"])
-            predecessor = None
-
-            # find continuity in guest physical address space
-            if (len(self.guest_phys_blocks) > 0):
-                predecessor = self.guest_phys_blocks[-1]
-                predecessor_size = (predecessor["target_end"] -
-                                    predecessor["target_start"])
-
-                # the memory API guarantees monotonically increasing
-                # traversal
-                assert (predecessor["target_end"] <= target_start)
-
-                # we want continuity in both guest-physical and
-                # host-virtual memory
-                if (predecessor["target_end"] < target_start or
-                    predecessor["host_addr"] + predecessor_size != host_addr):
-                    predecessor = None
-
-            if (predecessor is None):
-                # isolated mapping, add it to the list
-                self.guest_phys_blocks.append({"target_start": target_start,
-                                               "target_end"  : target_end,
-                                               "host_addr"   : host_addr})
-                message = "added"
-            else:
-                # expand predecessor until @target_end; predecessor's
-                # start doesn't change
-                predecessor["target_end"] = target_end
-                message = "joined"
-
-            print ("%016x %016x %016x %-7s %5u" %
-                   (target_start, target_end, host_addr.cast(self.uintptr_t),
-                    message, len(self.guest_phys_blocks)))
+        self.guest_phys_blocks = None
 
     def cpu_get_dump_info(self):
         # We can't synchronize the registers with KVM post-mortem, and
@@ -263,8 +266,7 @@ shape and this command should mostly work."""
                                 len(name) + 1, len(desc), type, name, desc)
 
     def dump_init(self):
-        self.guest_phys_blocks_init()
-        self.guest_phys_blocks_append()
+        self.guest_phys_blocks = get_guest_phys_blocks()
         self.cpu_get_dump_info()
         # we have no way to retrieve the VCPU status from KVM
         # post-mortem
@@ -310,7 +312,7 @@ shape and this command should mostly work."""
             cur  = block["host_addr"]
             left = block["target_end"] - block["target_start"]
             print ("dumping range at %016x for length %016x" %
-                   (cur.cast(self.uintptr_t), left))
+                   (cur.cast(UINTPTR_T), left))
             while (left > 0):
                 chunk_size = min(TARGET_PAGE_SIZE, left)
                 chunk = qemu_core.read_memory(cur, chunk_size)
-- 
2.3.0

[Qemu-devel] [RFC 3/5] scripts/dump-guest-memory.py: Improve python 3 compatibility

[Janosch Frank]

This commit does not make the script python 3 compatible, it is a
preparation that fixes the easy and common incompatibilities.

Print is a function in python 3 and therefore needs braces around its
arguments.

Range does not cast a gdb.Value object to int in python 3, we have to
do it ourselves.

Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 scripts/dump-guest-memory.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 76a6ecb..fe93135 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -98,15 +98,15 @@ def memory_region_get_ram_ptr(mr):
 
 def get_guest_phys_blocks():
     guest_phys_blocks = []
-    print "guest RAM blocks:"
-    print ("target_start     target_end       host_addr        message "
-           "count")
-    print ("---------------- ---------------- ---------------- ------- "
-           "-----")
+    print("guest RAM blocks:")
+    print("target_start     target_end       host_addr        message "
+          "count")
+    print("---------------- ---------------- ---------------- ------- "
+          "-----")
 
     current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
     current_map = current_map_p.dereference()
-    for cur in range(current_map["nr"]):
+    for cur in range(int(current_map["nr"])):
         flat_range   = (current_map["ranges"] + cur).dereference()
         mr           = flat_range["mr"].dereference()
 
@@ -149,9 +149,9 @@ def get_guest_phys_blocks():
             predecessor["target_end"] = target_end
             message = "joined"
 
-        print ("%016x %016x %016x %-7s %5u" %
-               (target_start, target_end, host_addr.cast(UINTPTR_T),
-                message, len(guest_phys_blocks)))
+        print("%016x %016x %016x %-7s %5u" %
+              (target_start, target_end, host_addr.cast(UINTPTR_T),
+               message, len(guest_phys_blocks)))
 
         return guest_phys_blocks
 
@@ -311,8 +311,8 @@ shape and this command should mostly work."""
         for block in self.guest_phys_blocks:
             cur  = block["host_addr"]
             left = block["target_end"] - block["target_start"]
-            print ("dumping range at %016x for length %016x" %
-                   (cur.cast(UINTPTR_T), left))
+            print("dumping range at %016x for length %016x" %
+                  (cur.cast(UINTPTR_T), left))
             while (left > 0):
                 chunk_size = min(TARGET_PAGE_SIZE, left)
                 chunk = qemu_core.read_memory(cur, chunk_size)
-- 
2.3.0

Re: [Qemu-devel] [RFC 3/5] scripts/dump-guest-memory.py: Improve python 3 compatibility

[Laszlo Ersek]

On 01/14/16 09:48, Janosch Frank wrote:
> This commit does not make the script python 3 compatible, it is a
> preparation that fixes the easy and common incompatibilities.
> 
> Print is a function in python 3 and therefore needs braces around its
> arguments.
> 
> Range does not cast a gdb.Value object to int in python 3, we have to
> do it ourselves.
> 
> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
>  scripts/dump-guest-memory.py | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 76a6ecb..fe93135 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -98,15 +98,15 @@ def memory_region_get_ram_ptr(mr):
>  
>  def get_guest_phys_blocks():
>      guest_phys_blocks = []
> -    print "guest RAM blocks:"
> -    print ("target_start     target_end       host_addr        message "
> -           "count")
> -    print ("---------------- ---------------- ---------------- ------- "
> -           "-----")
> +    print("guest RAM blocks:")
> +    print("target_start     target_end       host_addr        message "
> +          "count")
> +    print("---------------- ---------------- ---------------- ------- "
> +          "-----")
>  
>      current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
>      current_map = current_map_p.dereference()
> -    for cur in range(current_map["nr"]):
> +    for cur in range(int(current_map["nr"])):

FlatView.nr has type "unsigned" in C -- is this int cast safe in python?
(I don't expect a >= 2G value in "nr", but still.)

Thanks
Laszlo

>          flat_range   = (current_map["ranges"] + cur).dereference()
>          mr           = flat_range["mr"].dereference()
>  
> @@ -149,9 +149,9 @@ def get_guest_phys_blocks():
>              predecessor["target_end"] = target_end
>              message = "joined"
>  
> -        print ("%016x %016x %016x %-7s %5u" %
> -               (target_start, target_end, host_addr.cast(UINTPTR_T),
> -                message, len(guest_phys_blocks)))
> +        print("%016x %016x %016x %-7s %5u" %
> +              (target_start, target_end, host_addr.cast(UINTPTR_T),
> +               message, len(guest_phys_blocks)))
>  
>          return guest_phys_blocks
>  
> @@ -311,8 +311,8 @@ shape and this command should mostly work."""
>          for block in self.guest_phys_blocks:
>              cur  = block["host_addr"]
>              left = block["target_end"] - block["target_start"]
> -            print ("dumping range at %016x for length %016x" %
> -                   (cur.cast(UINTPTR_T), left))
> +            print("dumping range at %016x for length %016x" %
> +                  (cur.cast(UINTPTR_T), left))
>              while (left > 0):
>                  chunk_size = min(TARGET_PAGE_SIZE, left)
>                  chunk = qemu_core.read_memory(cur, chunk_size)
> 

Re: [Qemu-devel] [RFC 3/5] scripts/dump-guest-memory.py: Improve python 3 compatibility

[Janosch Frank]

On 01/14/2016 05:03 PM, Laszlo Ersek wrote:
> On 01/14/16 09:48, Janosch Frank wrote:
>> This commit does not make the script python 3 compatible, it is a
>> preparation that fixes the easy and common incompatibilities.
>>
>> Print is a function in python 3 and therefore needs braces around its
>> arguments.
>>
>> Range does not cast a gdb.Value object to int in python 3, we have to
>> do it ourselves.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> ---
>>  scripts/dump-guest-memory.py | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index 76a6ecb..fe93135 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -98,15 +98,15 @@ def memory_region_get_ram_ptr(mr):
>>  
>>  def get_guest_phys_blocks():
>>      guest_phys_blocks = []
>> -    print "guest RAM blocks:"
>> -    print ("target_start     target_end       host_addr        message "
>> -           "count")
>> -    print ("---------------- ---------------- ---------------- ------- "
>> -           "-----")
>> +    print("guest RAM blocks:")
>> +    print("target_start     target_end       host_addr        message "
>> +          "count")
>> +    print("---------------- ---------------- ---------------- ------- "
>> +          "-----")
>>  
>>      current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
>>      current_map = current_map_p.dereference()
>> -    for cur in range(current_map["nr"]):
>> +    for cur in range(int(current_map["nr"])):
> 
> FlatView.nr has type "unsigned" in C -- is this int cast safe in python?
> (I don't expect a >= 2G value in "nr", but still.)

>From the python point of view it shouldn't matter, short int is at least
32bit, long is arbitrary sized:

The function int() will return a short or a long int depending on
the argument value.
pep-0237

But I do not have enough cpython knowledge to validate what gdb does in
gdb/python/py-value.c as the value is a gdb.Value that gets cast to
integer or long.

> 
> Thanks
> Laszlo
> 
>>          flat_range   = (current_map["ranges"] + cur).dereference()
>>          mr           = flat_range["mr"].dereference()
>>  
>> @@ -149,9 +149,9 @@ def get_guest_phys_blocks():
>>              predecessor["target_end"] = target_end
>>              message = "joined"
>>  
>> -        print ("%016x %016x %016x %-7s %5u" %
>> -               (target_start, target_end, host_addr.cast(UINTPTR_T),
>> -                message, len(guest_phys_blocks)))
>> +        print("%016x %016x %016x %-7s %5u" %
>> +              (target_start, target_end, host_addr.cast(UINTPTR_T),
>> +               message, len(guest_phys_blocks)))
>>  
>>          return guest_phys_blocks
>>  
>> @@ -311,8 +311,8 @@ shape and this command should mostly work."""
>>          for block in self.guest_phys_blocks:
>>              cur  = block["host_addr"]
>>              left = block["target_end"] - block["target_start"]
>> -            print ("dumping range at %016x for length %016x" %
>> -                   (cur.cast(UINTPTR_T), left))
>> +            print("dumping range at %016x for length %016x" %
>> +                  (cur.cast(UINTPTR_T), left))
>>              while (left > 0):
>>                  chunk_size = min(TARGET_PAGE_SIZE, left)
>>                  chunk = qemu_core.read_memory(cur, chunk_size)
>>
> 

Re: [Qemu-devel] [RFC 3/5] scripts/dump-guest-memory.py: Improve python 3 compatibility

[Paolo Bonzini]

On 14/01/2016 09:48, Janosch Frank wrote:
> This commit does not make the script python 3 compatible, it is a
> preparation that fixes the easy and common incompatibilities.
> 
> Print is a function in python 3 and therefore needs braces around its
> arguments.
> 
> Range does not cast a gdb.Value object to int in python 3, we have to
> do it ourselves.

Would it make sense to make kvm_stat Py3-compatible too?

Paolo

> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
>  scripts/dump-guest-memory.py | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 76a6ecb..fe93135 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -98,15 +98,15 @@ def memory_region_get_ram_ptr(mr):
>  
>  def get_guest_phys_blocks():
>      guest_phys_blocks = []
> -    print "guest RAM blocks:"
> -    print ("target_start     target_end       host_addr        message "
> -           "count")
> -    print ("---------------- ---------------- ---------------- ------- "
> -           "-----")
> +    print("guest RAM blocks:")
> +    print("target_start     target_end       host_addr        message "
> +          "count")
> +    print("---------------- ---------------- ---------------- ------- "
> +          "-----")
>  
>      current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
>      current_map = current_map_p.dereference()
> -    for cur in range(current_map["nr"]):
> +    for cur in range(int(current_map["nr"])):
>          flat_range   = (current_map["ranges"] + cur).dereference()
>          mr           = flat_range["mr"].dereference()
>  
> @@ -149,9 +149,9 @@ def get_guest_phys_blocks():
>              predecessor["target_end"] = target_end
>              message = "joined"
>  
> -        print ("%016x %016x %016x %-7s %5u" %
> -               (target_start, target_end, host_addr.cast(UINTPTR_T),
> -                message, len(guest_phys_blocks)))
> +        print("%016x %016x %016x %-7s %5u" %
> +              (target_start, target_end, host_addr.cast(UINTPTR_T),
> +               message, len(guest_phys_blocks)))
>  
>          return guest_phys_blocks
>  
> @@ -311,8 +311,8 @@ shape and this command should mostly work."""
>          for block in self.guest_phys_blocks:
>              cur  = block["host_addr"]
>              left = block["target_end"] - block["target_start"]
> -            print ("dumping range at %016x for length %016x" %
> -                   (cur.cast(UINTPTR_T), left))
> +            print("dumping range at %016x for length %016x" %
> +                  (cur.cast(UINTPTR_T), left))
>              while (left > 0):
>                  chunk_size = min(TARGET_PAGE_SIZE, left)
>                  chunk = qemu_core.read_memory(cur, chunk_size)
> 

Re: [Qemu-devel] [RFC 3/5] scripts/dump-guest-memory.py: Improve python 3 compatibility

[Janosch Frank]

On 01/20/2016 12:18 PM, Paolo Bonzini wrote:
> 
> 
> On 14/01/2016 09:48, Janosch Frank wrote:
>> This commit does not make the script python 3 compatible, it is a
>> preparation that fixes the easy and common incompatibilities.
>>
>> Print is a function in python 3 and therefore needs braces around its
>> arguments.
>>
>> Range does not cast a gdb.Value object to int in python 3, we have to
>> do it ourselves.
> 
> Would it make sense to make kvm_stat Py3-compatible too?

Well, if you don't enforce it for any patches afterwards it might get
broken and therefore unnecessary. But I do not expect a huge number of
patches for those small scripts.

For compatibility we need to (quick check and test):
 fix all prints
 exchange dict.iteritems() with dict.items().
 decode the regex to utf-8

I'll add a patch for it into my next patch series for kvm_stat if you
want. I still need to add functionality and documentation to it.

Initially I did not chose to make dump guest memory compatible to both
versions. As my gdb was compiled with py 3, I decided to fix it until it
worked.
Surprisingly it then was also py 2 compatible.

> 
> Paolo
> 
>> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> ---
>>  scripts/dump-guest-memory.py | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index 76a6ecb..fe93135 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -98,15 +98,15 @@ def memory_region_get_ram_ptr(mr):
>>  
>>  def get_guest_phys_blocks():
>>      guest_phys_blocks = []
>> -    print "guest RAM blocks:"
>> -    print ("target_start     target_end       host_addr        message "
>> -           "count")
>> -    print ("---------------- ---------------- ---------------- ------- "
>> -           "-----")
>> +    print("guest RAM blocks:")
>> +    print("target_start     target_end       host_addr        message "
>> +          "count")
>> +    print("---------------- ---------------- ---------------- ------- "
>> +          "-----")
>>  
>>      current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
>>      current_map = current_map_p.dereference()
>> -    for cur in range(current_map["nr"]):
>> +    for cur in range(int(current_map["nr"])):
>>          flat_range   = (current_map["ranges"] + cur).dereference()
>>          mr           = flat_range["mr"].dereference()
>>  
>> @@ -149,9 +149,9 @@ def get_guest_phys_blocks():
>>              predecessor["target_end"] = target_end
>>              message = "joined"
>>  
>> -        print ("%016x %016x %016x %-7s %5u" %
>> -               (target_start, target_end, host_addr.cast(UINTPTR_T),
>> -                message, len(guest_phys_blocks)))
>> +        print("%016x %016x %016x %-7s %5u" %
>> +              (target_start, target_end, host_addr.cast(UINTPTR_T),
>> +               message, len(guest_phys_blocks)))
>>  
>>          return guest_phys_blocks
>>  
>> @@ -311,8 +311,8 @@ shape and this command should mostly work."""
>>          for block in self.guest_phys_blocks:
>>              cur  = block["host_addr"]
>>              left = block["target_end"] - block["target_start"]
>> -            print ("dumping range at %016x for length %016x" %
>> -                   (cur.cast(UINTPTR_T), left))
>> +            print("dumping range at %016x for length %016x" %
>> +                  (cur.cast(UINTPTR_T), left))
>>              while (left > 0):
>>                  chunk_size = min(TARGET_PAGE_SIZE, left)
>>                  chunk = qemu_core.read_memory(cur, chunk_size)
>>
> 

[Qemu-devel] [RFC 4/5] scripts/dump-guest-memory.py: Cleanup functions

[Janosch Frank]

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 <frankja@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.
     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"])
         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"])
 
             # 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
-- 
2.3.0

Re: [Qemu-devel] [RFC 4/5] scripts/dump-guest-memory.py: Cleanup functions

[Laszlo Ersek]

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 <frankja@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
> 

[Qemu-devel] [RFC 5/5] scripts/dump-guest-memory.py: Introduce multi-arch support

[Janosch Frank]

By modelling the ELF with ctypes we not only gain full python 3
support but can also create dumps for different architectures more easily.

Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 scripts/dump-guest-memory.py | 484 ++++++++++++++++++++++++++++---------------
 1 file changed, 321 insertions(+), 163 deletions(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 2110494..7aed8a7 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -15,58 +15,303 @@
 # "help data" summary), and it should match how other help texts look in
 # gdb.
 
-import struct
+import ctypes
+from math import ceil
 
 UINTPTR_T = gdb.lookup_type("uintptr_t")
 
 TARGET_PAGE_SIZE = 0x1000
 TARGET_PAGE_MASK = 0xFFFFFFFFFFFFF000
 
-# Various ELF constants
-EM_X86_64   = 62        # AMD x86-64 target machine
-ELFDATA2LSB = 1         # little endian
-ELFCLASS64  = 2
-ELFMAG      = "\x7FELF"
-EV_CURRENT  = 1
-ET_CORE     = 4
-PT_LOAD     = 1
-PT_NOTE     = 4
-
 # Special value for e_phnum. This indicates that the real number of
 # program headers is too large to fit into e_phnum. Instead the real
 # value is in the field sh_info of section 0.
 PN_XNUM = 0xFFFF
 
-# Format strings for packing and header size calculation.
-ELF64_EHDR = ("4s" # e_ident/magic
-              "B"  # e_ident/class
-              "B"  # e_ident/data
-              "B"  # e_ident/version
-              "B"  # e_ident/osabi
-              "8s" # e_ident/pad
-              "H"  # e_type
-              "H"  # e_machine
-              "I"  # e_version
-              "Q"  # e_entry
-              "Q"  # e_phoff
-              "Q"  # e_shoff
-              "I"  # e_flags
-              "H"  # e_ehsize
-              "H"  # e_phentsize
-              "H"  # e_phnum
-              "H"  # e_shentsize
-              "H"  # e_shnum
-              "H"  # e_shstrndx
-          )
-ELF64_PHDR = ("I"  # p_type
-              "I"  # p_flags
-              "Q"  # p_offset
-              "Q"  # p_vaddr
-              "Q"  # p_paddr
-              "Q"  # p_filesz
-              "Q"  # p_memsz
-              "Q"  # p_align
-          )
+EV_CURRENT = 1
+
+ELFCLASS32 = 1
+ELFCLASS64 = 2
+
+ELFDATA2LSB = 1
+ELFDATA2MSB = 2
+
+ET_CORE = 4
+
+PT_LOAD = 1
+PT_NOTE = 4
+
+EM_386 = 3
+EM_PPC = 20
+EM_PPC64 = 21
+EM_S390 = 22
+EM_AARCH = 183
+EM_X86_64 = 62
+
+class ELF(object):
+    """Representation of a ELF file."""
+
+    def __init__(self, arch):
+        self.ehdr = None
+        self.notes = []
+        self.segments = []
+        self.notes_size = 0
+        self.endianess = None
+        self.elfclass = ELFCLASS64
+
+        if arch == 'aarch64-le':
+            self.endianess = ELFDATA2LSB
+            self.elfclass = ELFCLASS64
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_AARCH
+
+        elif arch == 'aarch64-be':
+            self.endianess = ELFDATA2MSB
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_AARCH
+
+        elif arch == 'X86_64':
+            self.endianess = ELFDATA2LSB
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_X86_64
+
+        elif arch == '386':
+            self.endianess = ELFDATA2LSB
+            self.elfclass = ELFCLASS32
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_386
+
+        elif arch == 's390':
+            self.endianess = ELFDATA2MSB
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_S390
+
+        elif arch == 'ppc64-le':
+            self.endianess = ELFDATA2LSB
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_PPC64
+
+        elif arch == 'ppc64-be':
+            self.endianess = ELFDATA2MSB
+            self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+            self.ehdr.e_machine = EM_PPC64
+
+        else:
+            raise gdb.GdbError("No valid arch type specified.\n"
+                               "Currently supported types:"
+                               "aarch64 be/le, X86_64, 386, s390, ppc64 be/le")
+
+        self.add_segment(PT_NOTE, 0, 0)
+
+    def add_note(self, n_name, n_desc, n_type):
+        """Adds a note to the ELF."""
+
+        note = get_arch_note(self.endianess, len(n_name), len(n_desc))
+        note.n_namesz = len(n_name) + 1
+        note.n_descsz = len(n_desc)
+        note.n_name = n_name.encode()
+        note.n_type = n_type
+
+        # Desc needs to be 4 byte aligned (although the 64bit spec
+        # specifies 8 byte). When defining n_desc as uint32 it will be
+        # automatically aligned but we need the memmove to copy the
+        # string into it.
+        ctypes.memmove(note.n_desc, n_desc.encode(), len(n_desc))
+
+        self.notes.append(note)
+        self.segments[0].p_filesz += ctypes.sizeof(note)
+        self.segments[0].p_memsz += ctypes.sizeof(note)
+
+    def add_segment(self, p_type, p_paddr, p_size):
+        """Adds a segment to the elf."""
+
+        phdr = get_arch_phdr(self.endianess, self.elfclass)
+        phdr.p_type = p_type
+        phdr.p_paddr = p_paddr
+        phdr.p_filesz = p_size
+        phdr.p_memsz = p_size
+        self.segments.append(phdr)
+        self.ehdr.e_phnum += 1
+
+    def to_file(self, elf_file):
+        """Writes all ELF structures to the the passed file.
+
+        Structure:
+        Ehdr
+        Segment 0:PT_NOTE
+        Segment 1:PT_LOAD
+        Segment N:PT_LOAD
+        Note    0..N
+        Dump contents
+        """
+        elf_file.write(self.ehdr)
+        off = ctypes.sizeof(self.ehdr) + \
+              len(self.segments) * ctypes.sizeof(self.segments[0])
+
+        for phdr in self.segments:
+            phdr.p_offset = off
+            elf_file.write(phdr)
+            off += phdr.p_filesz
+
+        for note in self.notes:
+            elf_file.write(note)
+
+
+def get_arch_note(endianess, len_name, len_desc):
+    """Returns a Note class with the specified endianess."""
+
+    if endianess == ELFDATA2LSB:
+        superclass = ctypes.LittleEndianStructure
+    else:
+        superclass = ctypes.BigEndianStructure
+
+    len_name = len_name + 1
+
+    class Note(superclass):
+        """Represents an ELF note, includes the content."""
+
+        _fields_ = [("n_namesz", ctypes.c_uint32),
+                    ("n_descsz", ctypes.c_uint32),
+                    ("n_type", ctypes.c_uint32),
+                    ("n_name", ctypes.c_char * len_name),
+                    ("n_desc", ctypes.c_uint32 * int(ceil(len_desc / 4.0)))]
+    return Note()
+
+
+class Ident(ctypes.Structure):
+    """Represents the ELF ident array in the ehdr structure."""
+
+    _fields_ = [('ei_mag0', ctypes.c_ubyte),
+                ('ei_mag1', ctypes.c_ubyte),
+                ('ei_mag2', ctypes.c_ubyte),
+                ('ei_mag3', ctypes.c_ubyte),
+                ('ei_class', ctypes.c_ubyte),
+                ('ei_data', ctypes.c_ubyte),
+                ('ei_version', ctypes.c_ubyte),
+                ('ei_osabi', ctypes.c_ubyte),
+                ('ei_abiversion', ctypes.c_ubyte),
+                ('ei_pad', ctypes.c_ubyte * 7)]
+
+    def __init__(self, endianess, elfclass):
+        self.ei_mag0 = 0x7F
+        self.ei_mag1 = ord('E')
+        self.ei_mag2 = ord('L')
+        self.ei_mag3 = ord('F')
+        self.ei_class = elfclass
+        self.ei_data = endianess
+        self.ei_version = EV_CURRENT
+
+
+def get_arch_ehdr(endianess, elfclass):
+    """Returns a EHDR64 class with the specified endianess."""
+
+    if endianess == ELFDATA2LSB:
+        superclass = ctypes.LittleEndianStructure
+    else:
+        superclass = ctypes.BigEndianStructure
+
+    class EHDR64(superclass):
+        """Represents the 64 bit ELF header struct."""
+
+        _fields_ = [('e_ident', Ident),
+                    ('e_type', ctypes.c_uint16),
+                    ('e_machine', ctypes.c_uint16),
+                    ('e_version', ctypes.c_uint32),
+                    ('e_entry', ctypes.c_uint64),
+                    ('e_phoff', ctypes.c_uint64),
+                    ('e_shoff', ctypes.c_uint64),
+                    ('e_flags', ctypes.c_uint32),
+                    ('e_ehsize', ctypes.c_uint16),
+                    ('e_phentsize', ctypes.c_uint16),
+                    ('e_phnum', ctypes.c_uint16),
+                    ('e_shentsize', ctypes.c_uint16),
+                    ('e_shnum', ctypes.c_uint16),
+                    ('e_shstrndx', ctypes.c_uint16)]
+
+        def __init__(self):
+            super(superclass, self).__init__()
+            self.e_ident = Ident(endianess, elfclass)
+            self.e_type = ET_CORE
+            self.e_version = EV_CURRENT
+            self.e_ehsize = ctypes.sizeof(self)
+            self.e_phoff = ctypes.sizeof(self)
+            self.e_phentsize = ctypes.sizeof(get_arch_phdr(endianess, elfclass))
+            self.e_phnum = 0
+
+
+    class EHDR32(superclass):
+        """Represents the 32 bit ELF header struct."""
+
+        _fields_ = [('e_ident', Ident),
+                    ('e_type', ctypes.c_uint16),
+                    ('e_machine', ctypes.c_uint16),
+                    ('e_version', ctypes.c_uint32),
+                    ('e_entry', ctypes.c_uint32),
+                    ('e_phoff', ctypes.c_uint32),
+                    ('e_shoff', ctypes.c_uint32),
+                    ('e_flags', ctypes.c_uint32),
+                    ('e_ehsize', ctypes.c_uint16),
+                    ('e_phentsize', ctypes.c_uint16),
+                    ('e_phnum', ctypes.c_uint16),
+                    ('e_shentsize', ctypes.c_uint16),
+                    ('e_shnum', ctypes.c_uint16),
+                    ('e_shstrndx', ctypes.c_uint16)]
+
+        def __init__(self):
+            super(superclass, self).__init__()
+            self.e_ident = Ident(endianess, elfclass)
+            self.e_type = ET_CORE
+            self.e_version = EV_CURRENT
+            self.e_ehsize = ctypes.sizeof(self)
+            self.e_phoff = ctypes.sizeof(self)
+            self.e_phentsize = ctypes.sizeof(get_arch_phdr(endianess, elfclass))
+            self.e_phnum = 0
+
+    # End get_arch_ehdr
+    if elfclass == ELFCLASS64:
+        return EHDR64()
+    else:
+        return EHDR32()
+
+
+def get_arch_phdr(endianess, elfclass):
+    """Returns a 32 or 64 bit PHDR class with the specified endianess."""
+
+    if endianess == ELFDATA2LSB:
+        superclass = ctypes.LittleEndianStructure
+    else:
+        superclass = ctypes.BigEndianStructure
+
+    class PHDR64(superclass):
+        """Represents the 64 bit ELF program header struct."""
+
+        _fields_ = [('p_type', ctypes.c_uint32),
+                    ('p_flags', ctypes.c_uint32),
+                    ('p_offset', ctypes.c_uint64),
+                    ('p_vaddr', ctypes.c_uint64),
+                    ('p_paddr', ctypes.c_uint64),
+                    ('p_filesz', ctypes.c_uint64),
+                    ('p_memsz', ctypes.c_uint64),
+                    ('p_align', ctypes.c_uint64)]
+
+    class PHDR32(superclass):
+        """Represents the 32 bit ELF program header struct."""
+
+        _fields_ = [('p_type', ctypes.c_uint32),
+                    ('p_offset', ctypes.c_uint32),
+                    ('p_vaddr', ctypes.c_uint32),
+                    ('p_paddr', ctypes.c_uint32),
+                    ('p_filesz', ctypes.c_uint32),
+                    ('p_memsz', ctypes.c_uint32),
+                    ('p_flags', ctypes.c_uint32),
+                    ('p_align', ctypes.c_uint32)]
+
+    # End get_arch_phdr
+    if elfclass == ELFCLASS64:
+        return PHDR64()
+    else:
+        return PHDR32()
+
 
 def int128_get64(val):
     """Returns low 64bit part of Int128 struct."""
@@ -219,152 +464,65 @@ shape and this command should mostly work."""
         super(DumpGuestMemory, self).__init__("dump-guest-memory",
                                               gdb.COMMAND_DATA,
                                               gdb.COMPLETE_FILENAME)
-        self.elf64_ehdr_le = struct.Struct("<%s" % ELF64_EHDR)
-        self.elf64_phdr_le = struct.Struct("<%s" % ELF64_PHDR)
+        self.elf = None
         self.guest_phys_blocks = None
 
-    def cpu_get_dump_info(self):
-        # We can't synchronize the registers with KVM post-mortem, and
-        # the bits in (first_x86_cpu->env.hflags) seem to be stale; they
-        # may not reflect long mode for example. Hence just assume the
-        # most common values. This also means that instruction pointer
-        # etc. will be bogus in the dump, but at least the RAM contents
-        # should be valid.
-        self.dump_info = {"d_machine": EM_X86_64,
-                          "d_endian" : ELFDATA2LSB,
-                          "d_class"  : ELFCLASS64}
+    def dump_init(self, vmcore):
+        """Prepares and writes ELF structures to core file."""
 
-    def encode_elf64_ehdr_le(self):
-        return self.elf64_ehdr_le.pack(
-                                 ELFMAG,                      # e_ident/magic
-                                 self.dump_info["d_class"],   # e_ident/class
-                                 self.dump_info["d_endian"],  # e_ident/data
-                                 EV_CURRENT,                  # e_ident/version
-                                 0,                           # e_ident/osabi
-                                 "",                          # e_ident/pad
-                                 ET_CORE,                     # e_type
-                                 self.dump_info["d_machine"], # e_machine
-                                 EV_CURRENT,                  # e_version
-                                 0,                           # e_entry
-                                 self.elf64_ehdr_le.size,     # e_phoff
-                                 0,                           # e_shoff
-                                 0,                           # e_flags
-                                 self.elf64_ehdr_le.size,     # e_ehsize
-                                 self.elf64_phdr_le.size,     # e_phentsize
-                                 self.phdr_num,               # e_phnum
-                                 0,                           # e_shentsize
-                                 0,                           # e_shnum
-                                 0                            # e_shstrndx
-                                )
+        # Needed to make crash happy, data for more useful notes is
+        # not available in a qemu core.
+        self.elf.add_note("NONE", "EMPTY", 0)
 
-    def encode_elf64_note_le(self):
-        return self.elf64_phdr_le.pack(PT_NOTE,              # p_type
-                                       0,                    # p_flags
-                                       (self.memory_offset -
-                                        len(self.note)),     # p_offset
-                                       0,                    # p_vaddr
-                                       0,                    # p_paddr
-                                       len(self.note),       # p_filesz
-                                       len(self.note),       # p_memsz
-                                       0                     # p_align
-                                      )
+        # We should never reach PN_XNUM for paging=false dumps,
+        # there's just a handful of discontiguous ranges after
+        # merging.
+        # The constant is needed to account for the PT_NOTE segment.
+        phdr_num = len(self.guest_phys_blocks) + 1
+        assert phdr_num < PN_XNUM
 
-    def encode_elf64_load_le(self, offset, start_hwaddr, range_size):
-        return self.elf64_phdr_le.pack(PT_LOAD,      # p_type
-                                       0,            # p_flags
-                                       offset,       # p_offset
-                                       0,            # p_vaddr
-                                       start_hwaddr, # p_paddr
-                                       range_size,   # p_filesz
-                                       range_size,   # p_memsz
-                                       0             # p_align
-                                      )
-
-    def note_init(self, name, desc, type):
-        # name must include a trailing NUL
-        namesz = (len(name) + 1 + 3) / 4 * 4
-        descsz = (len(desc)     + 3) / 4 * 4
-        fmt = ("<"   # little endian
-               "I"   # n_namesz
-               "I"   # n_descsz
-               "I"   # n_type
-               "%us" # name
-               "%us" # desc
-               % (namesz, descsz))
-        self.note = struct.pack(fmt,
-                                len(name) + 1, len(desc), type, name, desc)
-
-    def dump_init(self):
-        self.guest_phys_blocks = get_guest_phys_blocks()
-        self.cpu_get_dump_info()
-        # we have no way to retrieve the VCPU status from KVM
-        # post-mortem
-        self.note_init("NONE", "EMPTY", 0)
-
-        # Account for PT_NOTE.
-        self.phdr_num = 1
-
-        # We should never reach PN_XNUM for paging=false dumps: there's
-        # just a handful of discontiguous ranges after merging.
-        self.phdr_num += len(self.guest_phys_blocks)
-        assert (self.phdr_num < PN_XNUM)
-
-        # Calculate the ELF file offset where the memory dump commences:
-        #
-        #   ELF header
-        #   PT_NOTE
-        #   PT_LOAD: 1
-        #   PT_LOAD: 2
-        #   ...
-        #   PT_LOAD: len(self.guest_phys_blocks)
-        #   ELF note
-        #   memory dump
-        self.memory_offset = (self.elf64_ehdr_le.size +
-                              self.elf64_phdr_le.size * self.phdr_num +
-                              len(self.note))
-
-    def dump_begin(self, vmcore):
-        vmcore.write(self.encode_elf64_ehdr_le())
-        vmcore.write(self.encode_elf64_note_le())
-        running = self.memory_offset
         for block in self.guest_phys_blocks:
-            range_size = block["target_end"] - block["target_start"]
-            vmcore.write(self.encode_elf64_load_le(running,
-                                                   block["target_start"],
-                                                   range_size))
-            running += range_size
-        vmcore.write(self.note)
+            block_size = block["target_end"] - block["target_start"]
+            self.elf.add_segment(PT_LOAD, block["target_start"], block_size)
+
+        self.elf.to_file(vmcore)
 
     def dump_iterate(self, vmcore):
+        """Writes guest core to file."""
+
         qemu_core = gdb.inferiors()[0]
         for block in self.guest_phys_blocks:
-            cur  = block["host_addr"]
+
+            cur = block["host_addr"]
             left = block["target_end"] - block["target_start"]
             print("dumping range at %016x for length %016x" %
                   (cur.cast(UINTPTR_T), left))
-            while (left > 0):
+
+            while left > 0:
                 chunk_size = min(TARGET_PAGE_SIZE, left)
                 chunk = qemu_core.read_memory(cur, chunk_size)
                 vmcore.write(chunk)
-                cur  += chunk_size
+                cur += chunk_size
                 left -= chunk_size
 
-    def create_vmcore(self, filename):
-        vmcore = open(filename, "wb")
-        self.dump_begin(vmcore)
-        self.dump_iterate(vmcore)
-        vmcore.close()
-
     def invoke(self, args, from_tty):
+        """Handles command invocation from gdb."""
+
         # Unwittingly pressing the Enter key after the command should
         # not dump the same multi-gig coredump to the same file.
         self.dont_repeat()
 
         argv = gdb.string_to_argv(args)
-        if (len(argv) != 1):
-            raise gdb.GdbError("usage: dump-guest-memory FILE")
+        if len(argv) != 2:
+            raise gdb.GdbError("usage: dump-guest-memory FILE ARCH")
 
-        self.dump_init()
-        self.create_vmcore(argv[0])
+        self.elf = ELF(argv[1])
+        self.guest_phys_blocks = get_guest_phys_blocks()
+
+        with open(argv[0], "wb") as vmcore:
+            # Writes ELF headers
+            self.dump_init(vmcore)
+            # Writes ELF contents
+            self.dump_iterate(vmcore)
 
 DumpGuestMemory()
-- 
2.3.0

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Laszlo Ersek]

On 01/14/16 09:48, Janosch Frank wrote:
> The dump guest memory script for extracting a Linux core from a qemu
> core is currently limited to amd64 and python 2.
> 
> With this series we add support for python 3 (while maintaining python
> 2 support) and add the possibility to extract dumps from VMs with the
> most common architectures.
> 
> This was tested on a s390 s12 guest only, I'd appreciate tests for the
> other architectures.
> 
> Janosch Frank (5):
>   scripts/dump-guest-memory.py: Move constants to the top
>   scripts/dump-guest-memory.py: Make methods functions
>   scripts/dump-guest-memory.py: Improve python 3 compatibility
>   scripts/dump-guest-memory.py: Cleanup functions
>   scripts/dump-guest-memory.py: Introduce multi-arch support
> 
>  scripts/dump-guest-memory.py | 717 +++++++++++++++++++++++++++----------------
>  1 file changed, 453 insertions(+), 264 deletions(-)
> 

So, I had a few notes for patches 1-4, but those are just insignificant
nits, so address them or not, I'm fine.

Also, I'm not a Python programmer (you can probably tell from the
source). For every three lines I wrote for this script, I had to stare
at basic Python documentation, and PEP-8, for five minutes. :)

Moving out a bunch of stuff to global namespace (from classes) in the
initial patches is fine I guess; but maybe keeping then in the class
helps with avoiding namespace collisions if a user loads other
extensions into gdb. IIRC that was my main motivation to keep those
things within the class. But, I don't feel strongly about this at all.

Patch 5 is mostly over my head ("class ELF" --> Laszlo stops reading,
almost).

I do notice that you import "ceil" from math, for a simple rounded-up
division. I think that's a bad idea (although I'm unsure about Python's
conversions between floating point and integers, and its floats in
general). Such rounding is not hard to do purely with integers; please
leave floating point out of the picture if possible.

In any case, if you have kept the script working for the x86_64 target
(I trust you regression tested it), in patch 5, then I don't object,
generally speaking. I actually welcome the aarch64 addition.

(Drew, can you perhaps check that out? IIRC you worked on the QMP
dump-guest-memory for aarch64.)

So, for patches 1-4, with the nits fixed or not:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

For patch 5, *if* you remove floating point (--> math / ceil), *and* you
confirm that you regression-tested it for the x86_64 target (which
testing includes looking briefly, with the "crash" utility, at the
extracted kernel vmcore), then you can add my:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Andrew Jones]

On Thu, Jan 14, 2016 at 05:24:23PM +0100, Laszlo Ersek wrote:
> On 01/14/16 09:48, Janosch Frank wrote:
> > The dump guest memory script for extracting a Linux core from a qemu
> > core is currently limited to amd64 and python 2.
> > 
> > With this series we add support for python 3 (while maintaining python
> > 2 support) and add the possibility to extract dumps from VMs with the
> > most common architectures.
> > 
> > This was tested on a s390 s12 guest only, I'd appreciate tests for the
> > other architectures.
> > 
> > Janosch Frank (5):
> >   scripts/dump-guest-memory.py: Move constants to the top
> >   scripts/dump-guest-memory.py: Make methods functions
> >   scripts/dump-guest-memory.py: Improve python 3 compatibility
> >   scripts/dump-guest-memory.py: Cleanup functions
> >   scripts/dump-guest-memory.py: Introduce multi-arch support
> > 
> >  scripts/dump-guest-memory.py | 717 +++++++++++++++++++++++++++----------------
> >  1 file changed, 453 insertions(+), 264 deletions(-)
> > 
> 
> So, I had a few notes for patches 1-4, but those are just insignificant
> nits, so address them or not, I'm fine.
> 
> Also, I'm not a Python programmer (you can probably tell from the
> source). For every three lines I wrote for this script, I had to stare
> at basic Python documentation, and PEP-8, for five minutes. :)
> 
> Moving out a bunch of stuff to global namespace (from classes) in the
> initial patches is fine I guess; but maybe keeping then in the class
> helps with avoiding namespace collisions if a user loads other
> extensions into gdb. IIRC that was my main motivation to keep those
> things within the class. But, I don't feel strongly about this at all.
> 
> Patch 5 is mostly over my head ("class ELF" --> Laszlo stops reading,
> almost).
> 
> I do notice that you import "ceil" from math, for a simple rounded-up
> division. I think that's a bad idea (although I'm unsure about Python's
> conversions between floating point and integers, and its floats in
> general). Such rounding is not hard to do purely with integers; please
> leave floating point out of the picture if possible.
> 
> In any case, if you have kept the script working for the x86_64 target
> (I trust you regression tested it), in patch 5, then I don't object,
> generally speaking. I actually welcome the aarch64 addition.
> 
> (Drew, can you perhaps check that out? IIRC you worked on the QMP
> dump-guest-memory for aarch64.)

I gave this a test run on AArch64 (LE). It worked, thus

Tested-by: Andrew Jones <drjones@redhat.com>


But the help text needs help. I'll paste the ones I think need changes
here in order to point out my suggestions

>  raise gdb.GdbError("No valid arch type specified.\n"
>                     "Currently supported types:"
>                     "aarch64 be/le, X86_64, 386, s390, ppc64 be/le")
                              ^ missing '-'                   ^ missing '-'

Actually it might be better to spell out aarch64-be, aarch64-le and
ppc64-be, ppc64-le as well.

>  class DumpGuestMemory(gdb.Command):
>      """Extract guest vmcore from qemu process coredump.
>
>  The sole argument is FILE, identifying the target file to write the

The two required arguments are FILE and ARCH. FILE identifies... ARCH
selects the architecture for which the core will be generated.

>  guest vmcore to.
>
>  This GDB command reimplements the dump-guest-memory QMP command in
>  python, using the representation of guest memory as captured in the qemu
>  coredump. The qemu process that has been dumped must have had the
>  command line option "-machine dump-guest-core=on".

Add one more sentence: "By default dump-guest-core is on."

>  
>  For simplicity, the "paging", "begin" and "end" parameters of the QMP
>  command are not supported -- no attempt is made to get the guest's
>  internal paging structures (ie. paging=false is hard-wired), and guest
>  memory is always fully dumped.
>  
>  Only x86_64 guests are supported.

aarch64-be, aarch64-le, X86_64, 386, s390, ppc64-be, ppc64-le guests are
supported.

>  
>  The CORE/NT_PRSTATUS and QEMU notes (that is, the VCPUs' statuses) are
>  not written to the vmcore. Preparing these would require context that is
>  only present in the KVM host kernel module when the guest is alive. A
>  fake ELF note is written instead, only to keep the ELF parser of "crash"
>  happy.
>  
>  Dependent on how busted the qemu process was at the time of the
>  coredump, this command might produce unpredictable results. If qemu
>  deliberately called abort(), or it was dumped in response to a signal at
>  a halfway fortunate point, then its coredump should be in reasonable
>  shape and this command should mostly work."""


Additionally, as this was a pretty full rewrite of the script, then I
think it warrants an additional Authors line under Laszlo's name.

Thanks,
drew


> 
> So, for patches 1-4, with the nits fixed or not:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> For patch 5, *if* you remove floating point (--> math / ceil), *and* you
> confirm that you regression-tested it for the x86_64 target (which
> testing includes looking briefly, with the "crash" utility, at the
> extracted kernel vmcore), then you can add my:
> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks
> Laszlo
> 

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Laszlo Ersek]

On 01/18/16 17:31, Andrew Jones wrote:
> On Thu, Jan 14, 2016 at 05:24:23PM +0100, Laszlo Ersek wrote:
>> On 01/14/16 09:48, Janosch Frank wrote:
>>> The dump guest memory script for extracting a Linux core from a qemu
>>> core is currently limited to amd64 and python 2.
>>>
>>> With this series we add support for python 3 (while maintaining python
>>> 2 support) and add the possibility to extract dumps from VMs with the
>>> most common architectures.
>>>
>>> This was tested on a s390 s12 guest only, I'd appreciate tests for the
>>> other architectures.
>>>
>>> Janosch Frank (5):
>>>   scripts/dump-guest-memory.py: Move constants to the top
>>>   scripts/dump-guest-memory.py: Make methods functions
>>>   scripts/dump-guest-memory.py: Improve python 3 compatibility
>>>   scripts/dump-guest-memory.py: Cleanup functions
>>>   scripts/dump-guest-memory.py: Introduce multi-arch support
>>>
>>>  scripts/dump-guest-memory.py | 717 +++++++++++++++++++++++++++----------------
>>>  1 file changed, 453 insertions(+), 264 deletions(-)
>>>
>>
>> So, I had a few notes for patches 1-4, but those are just insignificant
>> nits, so address them or not, I'm fine.
>>
>> Also, I'm not a Python programmer (you can probably tell from the
>> source). For every three lines I wrote for this script, I had to stare
>> at basic Python documentation, and PEP-8, for five minutes. :)
>>
>> Moving out a bunch of stuff to global namespace (from classes) in the
>> initial patches is fine I guess; but maybe keeping then in the class
>> helps with avoiding namespace collisions if a user loads other
>> extensions into gdb. IIRC that was my main motivation to keep those
>> things within the class. But, I don't feel strongly about this at all.
>>
>> Patch 5 is mostly over my head ("class ELF" --> Laszlo stops reading,
>> almost).
>>
>> I do notice that you import "ceil" from math, for a simple rounded-up
>> division. I think that's a bad idea (although I'm unsure about Python's
>> conversions between floating point and integers, and its floats in
>> general). Such rounding is not hard to do purely with integers; please
>> leave floating point out of the picture if possible.
>>
>> In any case, if you have kept the script working for the x86_64 target
>> (I trust you regression tested it), in patch 5, then I don't object,
>> generally speaking. I actually welcome the aarch64 addition.
>>
>> (Drew, can you perhaps check that out? IIRC you worked on the QMP
>> dump-guest-memory for aarch64.)
> 
> I gave this a test run on AArch64 (LE). It worked, thus
> 
> Tested-by: Andrew Jones <drjones@redhat.com>
> 
> 
> But the help text needs help. I'll paste the ones I think need changes
> here in order to point out my suggestions
> 
>>  raise gdb.GdbError("No valid arch type specified.\n"
>>                     "Currently supported types:"
>>                     "aarch64 be/le, X86_64, 386, s390, ppc64 be/le")
>                               ^ missing '-'                   ^ missing '-'
> 
> Actually it might be better to spell out aarch64-be, aarch64-le and
> ppc64-be, ppc64-le as well.
> 
>>  class DumpGuestMemory(gdb.Command):
>>      """Extract guest vmcore from qemu process coredump.
>>
>>  The sole argument is FILE, identifying the target file to write the
> 
> The two required arguments are FILE and ARCH. FILE identifies... ARCH
> selects the architecture for which the core will be generated.
> 
>>  guest vmcore to.
>>
>>  This GDB command reimplements the dump-guest-memory QMP command in
>>  python, using the representation of guest memory as captured in the qemu
>>  coredump. The qemu process that has been dumped must have had the
>>  command line option "-machine dump-guest-core=on".
> 
> Add one more sentence: "By default dump-guest-core is on."
> 
>>  
>>  For simplicity, the "paging", "begin" and "end" parameters of the QMP
>>  command are not supported -- no attempt is made to get the guest's
>>  internal paging structures (ie. paging=false is hard-wired), and guest
>>  memory is always fully dumped.
>>  
>>  Only x86_64 guests are supported.
> 
> aarch64-be, aarch64-le, X86_64, 386, s390, ppc64-be, ppc64-le guests are
> supported.
> 
>>  
>>  The CORE/NT_PRSTATUS and QEMU notes (that is, the VCPUs' statuses) are
>>  not written to the vmcore. Preparing these would require context that is
>>  only present in the KVM host kernel module when the guest is alive. A
>>  fake ELF note is written instead, only to keep the ELF parser of "crash"
>>  happy.
>>  
>>  Dependent on how busted the qemu process was at the time of the
>>  coredump, this command might produce unpredictable results. If qemu
>>  deliberately called abort(), or it was dumped in response to a signal at
>>  a halfway fortunate point, then its coredump should be in reasonable
>>  shape and this command should mostly work."""
> 
> 
> Additionally, as this was a pretty full rewrite of the script, then I
> think it warrants an additional Authors line under Laszlo's name.

Great points; thanks, Drew!
Laszlo

> 
> Thanks,
> drew
> 
> 
>>
>> So, for patches 1-4, with the nits fixed or not:
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> For patch 5, *if* you remove floating point (--> math / ceil), *and* you
>> confirm that you regression-tested it for the x86_64 target (which
>> testing includes looking briefly, with the "crash" utility, at the
>> extracted kernel vmcore), then you can add my:
>>
>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Thanks
>> Laszlo
>>

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Janosch Frank]

On 01/18/2016 06:57 PM, Laszlo Ersek wrote:
> On 01/18/16 17:31, Andrew Jones wrote:
>> On Thu, Jan 14, 2016 at 05:24:23PM +0100, Laszlo Ersek wrote:
>>> On 01/14/16 09:48, Janosch Frank wrote:
>>>> The dump guest memory script for extracting a Linux core from a qemu
>>>> core is currently limited to amd64 and python 2.
>>>>
>>>> With this series we add support for python 3 (while maintaining python
>>>> 2 support) and add the possibility to extract dumps from VMs with the
>>>> most common architectures.
>>>>
>>>> This was tested on a s390 s12 guest only, I'd appreciate tests for the
>>>> other architectures.
>>>>
>>>> Janosch Frank (5):
>>>>   scripts/dump-guest-memory.py: Move constants to the top
>>>>   scripts/dump-guest-memory.py: Make methods functions
>>>>   scripts/dump-guest-memory.py: Improve python 3 compatibility
>>>>   scripts/dump-guest-memory.py: Cleanup functions
>>>>   scripts/dump-guest-memory.py: Introduce multi-arch support
>>>>
>>>>  scripts/dump-guest-memory.py | 717 +++++++++++++++++++++++++++----------------
>>>>  1 file changed, 453 insertions(+), 264 deletions(-)
>>>>
>>>
>>> So, I had a few notes for patches 1-4, but those are just insignificant
>>> nits, so address them or not, I'm fine.
>>>
>>> Also, I'm not a Python programmer (you can probably tell from the
>>> source). For every three lines I wrote for this script, I had to stare
>>> at basic Python documentation, and PEP-8, for five minutes. :)
>>>
>>> Moving out a bunch of stuff to global namespace (from classes) in the
>>> initial patches is fine I guess; but maybe keeping then in the class
>>> helps with avoiding namespace collisions if a user loads other
>>> extensions into gdb. IIRC that was my main motivation to keep those
>>> things within the class. But, I don't feel strongly about this at all.
>>>
>>> Patch 5 is mostly over my head ("class ELF" --> Laszlo stops reading,
>>> almost).
>>>
>>> I do notice that you import "ceil" from math, for a simple rounded-up
>>> division. I think that's a bad idea (although I'm unsure about Python's
>>> conversions between floating point and integers, and its floats in
>>> general). Such rounding is not hard to do purely with integers; please
>>> leave floating point out of the picture if possible.

Leaving floating point out in python is difficult, read pep 238.
https://www.python.org/dev/peps/pep-0238/

In python 3:
1/2 == 0.5
1//2 == 0
but a // b == floor(a/b), i.e. a cast is made.

Anyway, I got rid of the import with:
-(-len_desc // 4)

>>>
>>> In any case, if you have kept the script working for the x86_64 target
>>> (I trust you regression tested it), in patch 5, then I don't object,
>>> generally speaking. I actually welcome the aarch64 addition.
>>>
>>> (Drew, can you perhaps check that out? IIRC you worked on the QMP
>>> dump-guest-memory for aarch64.)
>>
>> I gave this a test run on AArch64 (LE). It worked, thus
>>
>> Tested-by: Andrew Jones <drjones@redhat.com>

Thanks for testing, I'm currently setting up a Intel system to test
X86_64. Unfortunately I didn't have the system at hand before sending
the RFC.

>>
>>
>> But the help text needs help. I'll paste the ones I think need changes
>> here in order to point out my suggestions
>>
>>>  raise gdb.GdbError("No valid arch type specified.\n"
>>>                     "Currently supported types:"
>>>                     "aarch64 be/le, X86_64, 386, s390, ppc64 be/le")
>>                               ^ missing '-'                   ^ missing '-'
>>
>> Actually it might be better to spell out aarch64-be, aarch64-le and
>> ppc64-be, ppc64-le as well.
>>
>>>  class DumpGuestMemory(gdb.Command):
>>>      """Extract guest vmcore from qemu process coredump.
>>>
>>>  The sole argument is FILE, identifying the target file to write the
>>
>> The two required arguments are FILE and ARCH. FILE identifies... ARCH
>> selects the architecture for which the core will be generated.
>>
>>>  guest vmcore to.
>>>
>>>  This GDB command reimplements the dump-guest-memory QMP command in
>>>  python, using the representation of guest memory as captured in the qemu
>>>  coredump. The qemu process that has been dumped must have had the
>>>  command line option "-machine dump-guest-core=on".
>>
>> Add one more sentence: "By default dump-guest-core is on."
>>
>>>  
>>>  For simplicity, the "paging", "begin" and "end" parameters of the QMP
>>>  command are not supported -- no attempt is made to get the guest's
>>>  internal paging structures (ie. paging=false is hard-wired), and guest
>>>  memory is always fully dumped.
>>>  
>>>  Only x86_64 guests are supported.
>>
>> aarch64-be, aarch64-le, X86_64, 386, s390, ppc64-be, ppc64-le guests are
>> supported.
>>
>>>  
>>>  The CORE/NT_PRSTATUS and QEMU notes (that is, the VCPUs' statuses) are
>>>  not written to the vmcore. Preparing these would require context that is
>>>  only present in the KVM host kernel module when the guest is alive. A
>>>  fake ELF note is written instead, only to keep the ELF parser of "crash"
>>>  happy.
>>>  
>>>  Dependent on how busted the qemu process was at the time of the
>>>  coredump, this command might produce unpredictable results. If qemu
>>>  deliberately called abort(), or it was dumped in response to a signal at
>>>  a halfway fortunate point, then its coredump should be in reasonable
>>>  shape and this command should mostly work."""
>>
>>
>> Additionally, as this was a pretty full rewrite of the script, then I
>> think it warrants an additional Authors line under Laszlo's name.
> 
> Great points; thanks, Drew!
> Laszlo
> 

All mentioned changes will land in the patch series, thanks for
reviewing/testing to both of you.

>>
>> Thanks,
>> drew
>>
>>
>>>
>>> So, for patches 1-4, with the nits fixed or not:
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>
>>> For patch 5, *if* you remove floating point (--> math / ceil), *and* you
>>> confirm that you regression-tested it for the x86_64 target (which
>>> testing includes looking briefly, with the "crash" utility, at the
>>> extracted kernel vmcore), then you can add my:
>>>
>>> Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks

>>>
>>> Thanks
>>> Laszlo
>>>
> 

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Paolo Bonzini]

On 20/01/2016 11:03, Janosch Frank wrote:
> 
> In python 3:
> 1/2 == 0.5
> 1//2 == 0
> but a // b == floor(a/b), i.e. a cast is made.
> 
> Anyway, I got rid of the import with:
> -(-len_desc // 4)

I would change that to either:

    def ceil_div(a, b)
        return -(-a // b)

    ...

    ceil_div(len_desc, 4)

or

    (len_desc + 3) / 4

Paolo

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 20/01/2016 11:03, Janosch Frank wrote:
>> 
>> In python 3:
>> 1/2 == 0.5
>> 1//2 == 0
>> but a // b == floor(a/b), i.e. a cast is made.
>> 
>> Anyway, I got rid of the import with:
>> -(-len_desc // 4)
>
> I would change that to either:
>
>     def ceil_div(a, b)
>         return -(-a // b)
>
>     ...
>
>     ceil_div(len_desc, 4)
>
> or
>
>     (len_desc + 3) / 4

The latter is *far* easier on my eyes.  But you'd still have to truncate
for Python 3.

Re: [Qemu-devel] [RFC 0/5] scripts/dump-guest-memory.py: Add multi-arch support

[Laszlo Ersek]

On 01/20/16 14:50, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 20/01/2016 11:03, Janosch Frank wrote:
>>>
>>> In python 3:
>>> 1/2 == 0.5
>>> 1//2 == 0
>>> but a // b == floor(a/b), i.e. a cast is made.
>>>
>>> Anyway, I got rid of the import with:
>>> -(-len_desc // 4)
>>
>> I would change that to either:
>>
>>     def ceil_div(a, b)
>>         return -(-a // b)
>>
>>     ...
>>
>>     ceil_div(len_desc, 4)
>>
>> or
>>
>>     (len_desc + 3) / 4
> 
> The latter is *far* easier on my eyes.  But you'd still have to truncate
> for Python 3.

Yes, I think I had

  (len_desc + 3) // 4

in mind (except I didn't know about "//" just yet :))

Thanks
Laszlo