Add vmcoreinfo ELF note if vmcoreinfo device is ready.
To help the python script, add a little global vmcoreinfo_gdb
structure, that is populated with vmcoreinfo_gdb_update().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/dump-guest-memory.py | 32 ++++++++++++++++++++++++++++++++
include/hw/acpi/vmcoreinfo.h | 1 +
hw/acpi/vmcoreinfo.c | 16 ++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index f7c6635f15..16c3d7cb10 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -120,6 +120,20 @@ class ELF(object):
self.segments[0].p_filesz += ctypes.sizeof(note)
self.segments[0].p_memsz += ctypes.sizeof(note)
+
+ def add_vmcoreinfo_note(self, vmcoreinfo):
+ """Adds a vmcoreinfo note to the ELF dump."""
+ chead = type(get_arch_note(self.endianness, 0, 0))
+ header = chead.from_buffer_copy(vmcoreinfo[0:ctypes.sizeof(chead)])
+ note = get_arch_note(self.endianness,
+ header.n_namesz - 1, header.n_descsz)
+ ctypes.memmove(ctypes.pointer(note), vmcoreinfo, ctypes.sizeof(note))
+ header_size = ctypes.sizeof(note) - header.n_descsz
+
+ 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."""
@@ -505,6 +519,23 @@ shape and this command should mostly work."""
cur += chunk_size
left -= chunk_size
+ def add_vmcoreinfo(self):
+ qemu_core = gdb.inferiors()[0]
+
+ gdb.execute("call vmcoreinfo_gdb_update()")
+ avail = gdb.parse_and_eval("vmcoreinfo_gdb.available")
+ if not avail:
+ return;
+
+ addr = gdb.parse_and_eval("vmcoreinfo_gdb.paddr")
+ size = gdb.parse_and_eval("vmcoreinfo_gdb.size")
+ for block in self.guest_phys_blocks:
+ if block["target_start"] <= addr < block["target_end"]:
+ haddr = block["host_addr"] + (addr - block["target_start"])
+ vmcoreinfo = qemu_core.read_memory(haddr, size)
+ self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
+ return
+
def invoke(self, args, from_tty):
"""Handles command invocation from gdb."""
@@ -518,6 +549,7 @@ shape and this command should mostly work."""
self.elf = ELF(argv[1])
self.guest_phys_blocks = get_guest_phys_blocks()
+ self.add_vmcoreinfo()
with open(argv[0], "wb") as vmcore:
self.dump_init(vmcore)
diff --git a/include/hw/acpi/vmcoreinfo.h b/include/hw/acpi/vmcoreinfo.h
index 40fe99c3ed..4efa678237 100644
--- a/include/hw/acpi/vmcoreinfo.h
+++ b/include/hw/acpi/vmcoreinfo.h
@@ -32,5 +32,6 @@ void vmcoreinfo_build_acpi(VmcoreinfoState *vis, GArray *table_data,
void vmcoreinfo_add_fw_cfg(VmcoreinfoState *vis, FWCfgState *s, GArray *vmci);
bool vmcoreinfo_get(VmcoreinfoState *vis, uint64_t *paddr, uint64_t *size,
Error **errp);
+void vmcoreinfo_gdb_update(void);
#endif
diff --git a/hw/acpi/vmcoreinfo.c b/hw/acpi/vmcoreinfo.c
index 216e0bb83a..75e3330813 100644
--- a/hw/acpi/vmcoreinfo.c
+++ b/hw/acpi/vmcoreinfo.c
@@ -145,6 +145,22 @@ bool vmcoreinfo_get(VmcoreinfoState *vis,
return true;
}
+struct vmcoreinfo_gdb {
+ bool available;
+ uint64_t paddr;
+ uint64_t size;
+} vmcoreinfo_gdb;
+
+void vmcoreinfo_gdb_update(void)
+{
+ Object *vmci = find_vmcoreinfo_dev();
+
+ vmcoreinfo_gdb.available = vmci ?
+ vmcoreinfo_get(VMCOREINFO(vmci),
+ &vmcoreinfo_gdb.paddr, &vmcoreinfo_gdb.size, NULL)
+ : false;
+}
+
static const VMStateDescription vmstate_vmcoreinfo = {
.name = "vmcoreinfo",
.version_id = 1,
--
2.13.1.395.gf7b71de06
On 06/29/17 15:23, Marc-André Lureau wrote:
> Add vmcoreinfo ELF note if vmcoreinfo device is ready.
>
> To help the python script, add a little global vmcoreinfo_gdb
> structure, that is populated with vmcoreinfo_gdb_update().
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> scripts/dump-guest-memory.py | 32 ++++++++++++++++++++++++++++++++
> include/hw/acpi/vmcoreinfo.h | 1 +
> hw/acpi/vmcoreinfo.c | 16 ++++++++++++++++
> 3 files changed, 49 insertions(+)
>
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index f7c6635f15..16c3d7cb10 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -120,6 +120,20 @@ class ELF(object):
> self.segments[0].p_filesz += ctypes.sizeof(note)
> self.segments[0].p_memsz += ctypes.sizeof(note)
>
> +
> + def add_vmcoreinfo_note(self, vmcoreinfo):
> + """Adds a vmcoreinfo note to the ELF dump."""
> + chead = type(get_arch_note(self.endianness, 0, 0))
> + header = chead.from_buffer_copy(vmcoreinfo[0:ctypes.sizeof(chead)])
> + note = get_arch_note(self.endianness,
> + header.n_namesz - 1, header.n_descsz)
> + ctypes.memmove(ctypes.pointer(note), vmcoreinfo, ctypes.sizeof(note))
> + header_size = ctypes.sizeof(note) - header.n_descsz
> +
> + 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."""
>
> @@ -505,6 +519,23 @@ shape and this command should mostly work."""
> cur += chunk_size
> left -= chunk_size
>
> + def add_vmcoreinfo(self):
> + qemu_core = gdb.inferiors()[0]
> +
> + gdb.execute("call vmcoreinfo_gdb_update()")
I think it's a bad idea to call a function from a process that's just
crashed.
If this feature is so important, maybe we can simply set a global
pointer variable at the end of vmcoreinfo_realize(); something like:
static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
{
static VmcoreinfoState * volatile vmcoreinfo_gdb_helper;
[...]
vmcoreinfo_gdb_helper = VMCOREINFO(dev);
}
- vmcoreinfo_gdb_helper has function scope, so no other code can abuse
it
- it has static storage duration so gdb can access it at any time
- the pointer (not the pointed-to object) is qualified volatile, so gcc
cannot optimize out the pointer assignment (which it might be tempted
to do otherwise, due to the pointer never being read within QEMU)
Then you can use "vmcoreinfo_gdb_helper->vmcoreinfo_addr_le" to
implement all the logic in "dump-guest-memory.py".
Just my two cents, of course.
Thanks
Laszlo
> + avail = gdb.parse_and_eval("vmcoreinfo_gdb.available")
> + if not avail:
> + return;
> +
> + addr = gdb.parse_and_eval("vmcoreinfo_gdb.paddr")
> + size = gdb.parse_and_eval("vmcoreinfo_gdb.size")
> + for block in self.guest_phys_blocks:
> + if block["target_start"] <= addr < block["target_end"]:
> + haddr = block["host_addr"] + (addr - block["target_start"])
> + vmcoreinfo = qemu_core.read_memory(haddr, size)
> + self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
> + return
> +
> def invoke(self, args, from_tty):
> """Handles command invocation from gdb."""
>
> @@ -518,6 +549,7 @@ shape and this command should mostly work."""
>
> self.elf = ELF(argv[1])
> self.guest_phys_blocks = get_guest_phys_blocks()
> + self.add_vmcoreinfo()
>
> with open(argv[0], "wb") as vmcore:
> self.dump_init(vmcore)
> diff --git a/include/hw/acpi/vmcoreinfo.h b/include/hw/acpi/vmcoreinfo.h
> index 40fe99c3ed..4efa678237 100644
> --- a/include/hw/acpi/vmcoreinfo.h
> +++ b/include/hw/acpi/vmcoreinfo.h
> @@ -32,5 +32,6 @@ void vmcoreinfo_build_acpi(VmcoreinfoState *vis, GArray *table_data,
> void vmcoreinfo_add_fw_cfg(VmcoreinfoState *vis, FWCfgState *s, GArray *vmci);
> bool vmcoreinfo_get(VmcoreinfoState *vis, uint64_t *paddr, uint64_t *size,
> Error **errp);
> +void vmcoreinfo_gdb_update(void);
>
> #endif
> diff --git a/hw/acpi/vmcoreinfo.c b/hw/acpi/vmcoreinfo.c
> index 216e0bb83a..75e3330813 100644
> --- a/hw/acpi/vmcoreinfo.c
> +++ b/hw/acpi/vmcoreinfo.c
> @@ -145,6 +145,22 @@ bool vmcoreinfo_get(VmcoreinfoState *vis,
> return true;
> }
>
> +struct vmcoreinfo_gdb {
> + bool available;
> + uint64_t paddr;
> + uint64_t size;
> +} vmcoreinfo_gdb;
> +
> +void vmcoreinfo_gdb_update(void)
> +{
> + Object *vmci = find_vmcoreinfo_dev();
> +
> + vmcoreinfo_gdb.available = vmci ?
> + vmcoreinfo_get(VMCOREINFO(vmci),
> + &vmcoreinfo_gdb.paddr, &vmcoreinfo_gdb.size, NULL)
> + : false;
> +}
> +
> static const VMStateDescription vmstate_vmcoreinfo = {
> .name = "vmcoreinfo",
> .version_id = 1,
>
Hi
On Wed, Jul 5, 2017 at 2:22 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 06/29/17 15:23, Marc-André Lureau wrote:
>> Add vmcoreinfo ELF note if vmcoreinfo device is ready.
>>
>> To help the python script, add a little global vmcoreinfo_gdb
>> structure, that is populated with vmcoreinfo_gdb_update().
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> scripts/dump-guest-memory.py | 32 ++++++++++++++++++++++++++++++++
>> include/hw/acpi/vmcoreinfo.h | 1 +
>> hw/acpi/vmcoreinfo.c | 16 ++++++++++++++++
>> 3 files changed, 49 insertions(+)
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index f7c6635f15..16c3d7cb10 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -120,6 +120,20 @@ class ELF(object):
>> self.segments[0].p_filesz += ctypes.sizeof(note)
>> self.segments[0].p_memsz += ctypes.sizeof(note)
>>
>> +
>> + def add_vmcoreinfo_note(self, vmcoreinfo):
>> + """Adds a vmcoreinfo note to the ELF dump."""
>> + chead = type(get_arch_note(self.endianness, 0, 0))
>> + header = chead.from_buffer_copy(vmcoreinfo[0:ctypes.sizeof(chead)])
>> + note = get_arch_note(self.endianness,
>> + header.n_namesz - 1, header.n_descsz)
>> + ctypes.memmove(ctypes.pointer(note), vmcoreinfo, ctypes.sizeof(note))
>> + header_size = ctypes.sizeof(note) - header.n_descsz
>> +
>> + 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."""
>>
>> @@ -505,6 +519,23 @@ shape and this command should mostly work."""
>> cur += chunk_size
>> left -= chunk_size
>>
>> + def add_vmcoreinfo(self):
>> + qemu_core = gdb.inferiors()[0]
>> +
>> + gdb.execute("call vmcoreinfo_gdb_update()")
>
> I think it's a bad idea to call a function from a process that's just
> crashed.
Yeah, if qemu crashed you can't use that script. But we are talking
about dump of guest kernel, so qemu didn't crash :)
>
> If this feature is so important, maybe we can simply set a global
> pointer variable at the end of vmcoreinfo_realize(); something like:
>
> static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
> {
> static VmcoreinfoState * volatile vmcoreinfo_gdb_helper;
> [...]
> vmcoreinfo_gdb_helper = VMCOREINFO(dev);
> }
>
> - vmcoreinfo_gdb_helper has function scope, so no other code can abuse
> it
> - it has static storage duration so gdb can access it at any time
> - the pointer (not the pointed-to object) is qualified volatile, so gcc
> cannot optimize out the pointer assignment (which it might be tempted
> to do otherwise, due to the pointer never being read within QEMU)
>
> Then you can use "vmcoreinfo_gdb_helper->vmcoreinfo_addr_le" to
> implement all the logic in "dump-guest-memory.py".
If necessary, I can try that.
>
> Just my two cents, of course.
>
> Thanks
> Laszlo
>
>> + avail = gdb.parse_and_eval("vmcoreinfo_gdb.available")
>> + if not avail:
>> + return;
>> +
>> + addr = gdb.parse_and_eval("vmcoreinfo_gdb.paddr")
>> + size = gdb.parse_and_eval("vmcoreinfo_gdb.size")
>> + for block in self.guest_phys_blocks:
>> + if block["target_start"] <= addr < block["target_end"]:
>> + haddr = block["host_addr"] + (addr - block["target_start"])
>> + vmcoreinfo = qemu_core.read_memory(haddr, size)
>> + self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
>> + return
>> +
>> def invoke(self, args, from_tty):
>> """Handles command invocation from gdb."""
>>
>> @@ -518,6 +549,7 @@ shape and this command should mostly work."""
>>
>> self.elf = ELF(argv[1])
>> self.guest_phys_blocks = get_guest_phys_blocks()
>> + self.add_vmcoreinfo()
>>
>> with open(argv[0], "wb") as vmcore:
>> self.dump_init(vmcore)
>> diff --git a/include/hw/acpi/vmcoreinfo.h b/include/hw/acpi/vmcoreinfo.h
>> index 40fe99c3ed..4efa678237 100644
>> --- a/include/hw/acpi/vmcoreinfo.h
>> +++ b/include/hw/acpi/vmcoreinfo.h
>> @@ -32,5 +32,6 @@ void vmcoreinfo_build_acpi(VmcoreinfoState *vis, GArray *table_data,
>> void vmcoreinfo_add_fw_cfg(VmcoreinfoState *vis, FWCfgState *s, GArray *vmci);
>> bool vmcoreinfo_get(VmcoreinfoState *vis, uint64_t *paddr, uint64_t *size,
>> Error **errp);
>> +void vmcoreinfo_gdb_update(void);
>>
>> #endif
>> diff --git a/hw/acpi/vmcoreinfo.c b/hw/acpi/vmcoreinfo.c
>> index 216e0bb83a..75e3330813 100644
>> --- a/hw/acpi/vmcoreinfo.c
>> +++ b/hw/acpi/vmcoreinfo.c
>> @@ -145,6 +145,22 @@ bool vmcoreinfo_get(VmcoreinfoState *vis,
>> return true;
>> }
>>
>> +struct vmcoreinfo_gdb {
>> + bool available;
>> + uint64_t paddr;
>> + uint64_t size;
>> +} vmcoreinfo_gdb;
>> +
>> +void vmcoreinfo_gdb_update(void)
>> +{
>> + Object *vmci = find_vmcoreinfo_dev();
>> +
>> + vmcoreinfo_gdb.available = vmci ?
>> + vmcoreinfo_get(VMCOREINFO(vmci),
>> + &vmcoreinfo_gdb.paddr, &vmcoreinfo_gdb.size, NULL)
>> + : false;
>> +}
>> +
>> static const VMStateDescription vmstate_vmcoreinfo = {
>> .name = "vmcoreinfo",
>> .version_id = 1,
>>
>
>
--
Marc-André Lureau
On 07/05/17 11:58, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jul 5, 2017 at 2:22 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 06/29/17 15:23, Marc-André Lureau wrote:
>>> Add vmcoreinfo ELF note if vmcoreinfo device is ready.
>>>
>>> To help the python script, add a little global vmcoreinfo_gdb
>>> structure, that is populated with vmcoreinfo_gdb_update().
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>> scripts/dump-guest-memory.py | 32 ++++++++++++++++++++++++++++++++
>>> include/hw/acpi/vmcoreinfo.h | 1 +
>>> hw/acpi/vmcoreinfo.c | 16 ++++++++++++++++
>>> 3 files changed, 49 insertions(+)
>>>
>>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>>> index f7c6635f15..16c3d7cb10 100644
>>> --- a/scripts/dump-guest-memory.py
>>> +++ b/scripts/dump-guest-memory.py
>>> @@ -120,6 +120,20 @@ class ELF(object):
>>> self.segments[0].p_filesz += ctypes.sizeof(note)
>>> self.segments[0].p_memsz += ctypes.sizeof(note)
>>>
>>> +
>>> + def add_vmcoreinfo_note(self, vmcoreinfo):
>>> + """Adds a vmcoreinfo note to the ELF dump."""
>>> + chead = type(get_arch_note(self.endianness, 0, 0))
>>> + header = chead.from_buffer_copy(vmcoreinfo[0:ctypes.sizeof(chead)])
>>> + note = get_arch_note(self.endianness,
>>> + header.n_namesz - 1, header.n_descsz)
>>> + ctypes.memmove(ctypes.pointer(note), vmcoreinfo, ctypes.sizeof(note))
>>> + header_size = ctypes.sizeof(note) - header.n_descsz
>>> +
>>> + 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."""
>>>
>>> @@ -505,6 +519,23 @@ shape and this command should mostly work."""
>>> cur += chunk_size
>>> left -= chunk_size
>>>
>>> + def add_vmcoreinfo(self):
>>> + qemu_core = gdb.inferiors()[0]
>>> +
>>> + gdb.execute("call vmcoreinfo_gdb_update()")
>>
>> I think it's a bad idea to call a function from a process that's just
>> crashed.
>
> Yeah, if qemu crashed you can't use that script. But we are talking
> about dump of guest kernel, so qemu didn't crash :)
I think we have a misunderstanding here. Extracting the guest kernel
core from the core dump of a crashed QEMU process is the *only* purpose
of the GDB extension implemented in "dump-guest-memory.py".
In other words, if you are loading "dump-guest-memory.py" into gdb, then
QEMU crashed *by definition*. Because otherwise you'd dump the guest
kernel core using the live monitor commands (HMP or QMP).
So, "dump-guest-memory.py" is really a last resort utility, for the case
when the guest kernel does something "interesting" that crashes QEMU
with hopefully localized damage, and most of the data structures
hopefully remain usable. It is not guaranteed at all that
"dump-guest-memory.py" will produce anything useful, dependent on how
corrupt the QEMU process memory is at the time of the SIGSEGV or SIGABRT
(or another fatal signal).
Please see the message on original QEMU commit 3e16d14fd93c
("Python-lang gdb script to extract x86_64 guest vmcore from qemu
coredump", 2013-12-17).
See also the original RFE -- I apologize to non-Red Hatters, the RHBZ is
private because it was filed by a customer --:
https://bugzilla.redhat.com/show_bug.cgi?id=826266
In my opinion, poking at possibly corrupt data structures with the
python script is OK, while executing code directly from the crashed
image is too much. But, again, that's just my opinion.
>
>>
>> If this feature is so important, maybe we can simply set a global
>> pointer variable at the end of vmcoreinfo_realize(); something like:
>>
>> static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
>> {
>> static VmcoreinfoState * volatile vmcoreinfo_gdb_helper;
>> [...]
>> vmcoreinfo_gdb_helper = VMCOREINFO(dev);
>> }
>>
>> - vmcoreinfo_gdb_helper has function scope, so no other code can abuse
>> it
>> - it has static storage duration so gdb can access it at any time
>> - the pointer (not the pointed-to object) is qualified volatile, so gcc
>> cannot optimize out the pointer assignment (which it might be tempted
>> to do otherwise, due to the pointer never being read within QEMU)
>>
>> Then you can use "vmcoreinfo_gdb_helper->vmcoreinfo_addr_le" to
>> implement all the logic in "dump-guest-memory.py".
>
> If necessary, I can try that.
Well, I can't claim it is "objectively necessary"; it's just that this
method would satisfy my preferences above (i.e., poking at process data
from python: OK; running code from the process: not OK).
Thanks,
Laszlo
© 2016 - 2026 Red Hat, Inc.