dump/win_dump.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
QMP query-dump-guest-memory-capability reports win dump as available for
any x86 VM, which is false.
This patch implements proper query of vmcoreinfo and calculation of
guest note size. Based on that we can surely report whether win dump
available or not.
For further reference one may review this libvirt discussion:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB/#HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB
[PATCH 0/4] Allow xml-configured coredump format on VM crash
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
---
dump/win_dump.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/dump/win_dump.c b/dump/win_dump.c
index 3162e8bd48..4bb1b28e63 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -14,14 +14,74 @@
#include "qemu/error-report.h"
#include "exec/cpu-defs.h"
#include "hw/core/cpu.h"
+#include "hw/misc/vmcoreinfo.h"
#include "qemu/win_dump_defs.h"
#include "win_dump.h"
#include "cpu.h"
+#include "elf.h"
#if defined(TARGET_X86_64)
+#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size) \
+ ((DIV_ROUND_UP((hdr_size), 4) + \
+ DIV_ROUND_UP((name_size), 4) + \
+ DIV_ROUND_UP((desc_size), 4)) * 4)
+
bool win_dump_available(Error **errp)
{
+ uint64_t addr, note_head_size, name_size, desc_size;
+ uint32_t size;
+ uint16_t guest_format;
+ uint8_t *guest_note = NULL;
+ size_t guest_note_size = 0;
+ VMCoreInfoState *vmci = vmcoreinfo_find();
+ ArchDumpInfo dump_info = {};
+ GuestPhysBlockList blocks = {};
+ int ret;
+
+ if (!vmci || !vmci->has_vmcoreinfo)
+ return false;
+
+ ret = cpu_get_dump_info(&dump_info, &blocks);
+ if (ret < 0)
+ return false;
+
+ guest_format = le16_to_cpu(vmci->vmcoreinfo.guest_format);
+ if (guest_format != FW_CFG_VMCOREINFO_FORMAT_ELF)
+ return false;
+
+ size = le32_to_cpu(vmci->vmcoreinfo.size);
+ addr = le64_to_cpu(vmci->vmcoreinfo.paddr);
+ note_head_size = dump_info.d_class == ELFCLASS64 ?
+ sizeof(Elf64_Nhdr) : sizeof(Elf32_Nhdr);
+
+ guest_note = g_malloc(size + 1);
+ cpu_physical_memory_read(addr, guest_note, size);
+ if (dump_info.d_class == ELFCLASS64) {
+ const Elf64_Nhdr *hdr = (void *)guest_note;
+ if (dump_info.d_endian == ELFDATA2LSB) {
+ name_size = cpu_to_le64(hdr->n_namesz);
+ desc_size = cpu_to_le64(hdr->n_descsz);
+ } else {
+ name_size = cpu_to_be64(hdr->n_namesz);
+ desc_size = cpu_to_be64(hdr->n_descsz);
+ }
+ } else {
+ const Elf32_Nhdr *hdr = (void *)guest_note;
+ if (dump_info.d_endian == ELFDATA2LSB) {
+ name_size = cpu_to_le32(hdr->n_namesz);
+ desc_size = cpu_to_le32(hdr->n_descsz);
+ } else {
+ name_size = cpu_to_be32(hdr->n_namesz);
+ desc_size = cpu_to_be32(hdr->n_descsz);
+ }
+ }
+
+ guest_note_size = ELF_NOTE_SIZE(note_head_size, name_size, desc_size);
+ if (guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE64 &&
+ guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE32)
+ return false;
+
return true;
}
--
2.47.3On Wed, Jul 23, 2025 at 08:04:02PM +0300, Nikolai Barybin wrote:
> QMP query-dump-guest-memory-capability reports win dump as available for
> any x86 VM, which is false.
>
> This patch implements proper query of vmcoreinfo and calculation of
> guest note size. Based on that we can surely report whether win dump
> available or not.
>
> For further reference one may review this libvirt discussion:
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB/#HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB
> [PATCH 0/4] Allow xml-configured coredump format on VM crash
>
> Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
> ---
> dump/win_dump.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index 3162e8bd48..4bb1b28e63 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -14,14 +14,74 @@
> #include "qemu/error-report.h"
> #include "exec/cpu-defs.h"
> #include "hw/core/cpu.h"
> +#include "hw/misc/vmcoreinfo.h"
> #include "qemu/win_dump_defs.h"
> #include "win_dump.h"
> #include "cpu.h"
> +#include "elf.h"
>
> #if defined(TARGET_X86_64)
>
> +#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size) \
> + ((DIV_ROUND_UP((hdr_size), 4) + \
> + DIV_ROUND_UP((name_size), 4) + \
> + DIV_ROUND_UP((desc_size), 4)) * 4)
> +
> bool win_dump_available(Error **errp)
> {
> + uint64_t addr, note_head_size, name_size, desc_size;
> + uint32_t size;
> + uint16_t guest_format;
> + uint8_t *guest_note = NULL;
> + size_t guest_note_size = 0;
> + VMCoreInfoState *vmci = vmcoreinfo_find();
> + ArchDumpInfo dump_info = {};
> + GuestPhysBlockList blocks = {};
> + int ret;
> +
> + if (!vmci || !vmci->has_vmcoreinfo)
> + return false;
> +
> + ret = cpu_get_dump_info(&dump_info, &blocks);
> + if (ret < 0)
> + return false;
> +
> + guest_format = le16_to_cpu(vmci->vmcoreinfo.guest_format);
> + if (guest_format != FW_CFG_VMCOREINFO_FORMAT_ELF)
> + return false;
> +
> + size = le32_to_cpu(vmci->vmcoreinfo.size);
> + addr = le64_to_cpu(vmci->vmcoreinfo.paddr);
> + note_head_size = dump_info.d_class == ELFCLASS64 ?
> + sizeof(Elf64_Nhdr) : sizeof(Elf32_Nhdr);
> +
> + guest_note = g_malloc(size + 1);
> + cpu_physical_memory_read(addr, guest_note, size);
> + if (dump_info.d_class == ELFCLASS64) {
> + const Elf64_Nhdr *hdr = (void *)guest_note;
> + if (dump_info.d_endian == ELFDATA2LSB) {
> + name_size = cpu_to_le64(hdr->n_namesz);
> + desc_size = cpu_to_le64(hdr->n_descsz);
> + } else {
> + name_size = cpu_to_be64(hdr->n_namesz);
> + desc_size = cpu_to_be64(hdr->n_descsz);
> + }
> + } else {
> + const Elf32_Nhdr *hdr = (void *)guest_note;
> + if (dump_info.d_endian == ELFDATA2LSB) {
> + name_size = cpu_to_le32(hdr->n_namesz);
> + desc_size = cpu_to_le32(hdr->n_descsz);
> + } else {
> + name_size = cpu_to_be32(hdr->n_namesz);
> + desc_size = cpu_to_be32(hdr->n_descsz);
> + }
> + }
> +
> + guest_note_size = ELF_NOTE_SIZE(note_head_size, name_size, desc_size);
It feels like there is overlap between what this method has to do upto
here, with what the existing 'dump_init' has to do. Any possibility to
have a common helper to share logic ?
> + if (guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE64 &&
> + guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE32)
> + return false;
This dupes a check in create_win_dump, but misses the extra sanity
check from check_header. I think we should move the guest_note_size
check out of 'create_win_dump' and into 'check_header', then call
that from this code.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/23/25 19:04, Nikolai Barybin wrote:
> QMP query-dump-guest-memory-capability reports win dump as available for
> any x86 VM, which is false.
>
> This patch implements proper query of vmcoreinfo and calculation of
> guest note size. Based on that we can surely report whether win dump
> available or not.
>
> For further reference one may review this libvirt discussion:
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB/#HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB
> [PATCH 0/4] Allow xml-configured coredump format on VM crash
>
> Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
> ---
> dump/win_dump.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index 3162e8bd48..4bb1b28e63 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -14,14 +14,74 @@
> #include "qemu/error-report.h"
> #include "exec/cpu-defs.h"
> #include "hw/core/cpu.h"
> +#include "hw/misc/vmcoreinfo.h"
> #include "qemu/win_dump_defs.h"
> #include "win_dump.h"
> #include "cpu.h"
> +#include "elf.h"
>
> #if defined(TARGET_X86_64)
>
> +#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size) \
> + ((DIV_ROUND_UP((hdr_size), 4) + \
> + DIV_ROUND_UP((name_size), 4) + \
> + DIV_ROUND_UP((desc_size), 4)) * 4)
> +
> bool win_dump_available(Error **errp)
> {
> + uint64_t addr, note_head_size, name_size, desc_size;
> + uint32_t size;
> + uint16_t guest_format;
> + uint8_t *guest_note = NULL;
> + size_t guest_note_size = 0;
> + VMCoreInfoState *vmci = vmcoreinfo_find();
> + ArchDumpInfo dump_info = {};
> + GuestPhysBlockList blocks = {};
> + int ret;
> +
> + if (!vmci || !vmci->has_vmcoreinfo)
> + return false;
> +
> + ret = cpu_get_dump_info(&dump_info, &blocks);
This will not work IMHO. cpu_get_dump_info tries to operate
with a real guest memory, which is not provided. This means
that guest executable is impossible to find.
For me this is looking like Windows dump could not be enabled
after the patch.
> + if (ret < 0)
> + return false;
> +
> + guest_format = le16_to_cpu(vmci->vmcoreinfo.guest_format);
> + if (guest_format != FW_CFG_VMCOREINFO_FORMAT_ELF)
> + return false;
> +
> + size = le32_to_cpu(vmci->vmcoreinfo.size);
> + addr = le64_to_cpu(vmci->vmcoreinfo.paddr);
> + note_head_size = dump_info.d_class == ELFCLASS64 ?
> + sizeof(Elf64_Nhdr) : sizeof(Elf32_Nhdr);
note_head_size assignment could go to if below saving conditional
> +
> + guest_note = g_malloc(size + 1);
leaked at the end of the call
> + cpu_physical_memory_read(addr, guest_note, size);
> + if (dump_info.d_class == ELFCLASS64) {
> + const Elf64_Nhdr *hdr = (void *)guest_note;
> + if (dump_info.d_endian == ELFDATA2LSB) {
> + name_size = cpu_to_le64(hdr->n_namesz);
> + desc_size = cpu_to_le64(hdr->n_descsz);
you are going to use these values as integers below,
thus it is required to do conversion from the guest
endianity to the local one, i.e. le64_to_cpu
> + } else {
> + name_size = cpu_to_be64(hdr->n_namesz);
> + desc_size = cpu_to_be64(hdr->n_descsz);
same as above
> + }
> + } else {
> + const Elf32_Nhdr *hdr = (void *)guest_note;
> + if (dump_info.d_endian == ELFDATA2LSB) {
> + name_size = cpu_to_le32(hdr->n_namesz);
> + desc_size = cpu_to_le32(hdr->n_descsz);
same as above
> + } else {
> + name_size = cpu_to_be32(hdr->n_namesz);
> + desc_size = cpu_to_be32(hdr->n_descsz);
same as above
> + }
> + }
> +
> + guest_note_size = ELF_NOTE_SIZE(note_head_size, name_size, desc_size);
> + if (guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE64 &&
> + guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE32)
> + return false;
> +
personally I prefer
return guest_note_size == VMCOREINFO_WIN_DUMP_NOTE_SIZE64 ||
guest_note_size == VMCOREINFO_WIN_DUMP_NOTE_SIZE32;
> return true;
> }
>
On 7/23/25 19:31, Denis V. Lunev wrote:
> On 7/23/25 19:04, Nikolai Barybin wrote:
>> QMP query-dump-guest-memory-capability reports win dump as available for
>> any x86 VM, which is false.
>>
>> This patch implements proper query of vmcoreinfo and calculation of
>> guest note size. Based on that we can surely report whether win dump
>> available or not.
>>
>> For further reference one may review this libvirt discussion:
>> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB/#HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB
>>
>> [PATCH 0/4] Allow xml-configured coredump format on VM crash
>>
>> Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
>> ---
>> dump/win_dump.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 60 insertions(+)
>>
>> diff --git a/dump/win_dump.c b/dump/win_dump.c
>> index 3162e8bd48..4bb1b28e63 100644
>> --- a/dump/win_dump.c
>> +++ b/dump/win_dump.c
>> @@ -14,14 +14,74 @@
>> #include "qemu/error-report.h"
>> #include "exec/cpu-defs.h"
>> #include "hw/core/cpu.h"
>> +#include "hw/misc/vmcoreinfo.h"
>> #include "qemu/win_dump_defs.h"
>> #include "win_dump.h"
>> #include "cpu.h"
>> +#include "elf.h"
>> #if defined(TARGET_X86_64)
>> +#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size) \
>> + ((DIV_ROUND_UP((hdr_size), 4) + \
>> + DIV_ROUND_UP((name_size), 4) + \
>> + DIV_ROUND_UP((desc_size), 4)) * 4)
>> +
>> bool win_dump_available(Error **errp)
>> {
>> + uint64_t addr, note_head_size, name_size, desc_size;
>> + uint32_t size;
>> + uint16_t guest_format;
>> + uint8_t *guest_note = NULL;
>> + size_t guest_note_size = 0;
>> + VMCoreInfoState *vmci = vmcoreinfo_find();
>> + ArchDumpInfo dump_info = {};
>> + GuestPhysBlockList blocks = {};
>> + int ret;
>> +
>> + if (!vmci || !vmci->has_vmcoreinfo)
>> + return false;
>> +
>> + ret = cpu_get_dump_info(&dump_info, &blocks);
> This will not work IMHO. cpu_get_dump_info tries to operate
> with a real guest memory, which is not provided. This means
> that guest executable is impossible to find.
>
> For me this is looking like Windows dump could not be enabled
> after the patch.
>
Correction: current code will not work on 32bit guest with
more than 4Gb RAM on i386. More problems are definitely
possible on other platforms.
Den
On 7/23/25 19:04, Nikolai Barybin wrote:
> QMP query-dump-guest-memory-capability reports win dump as available for
> any x86 VM, which is false.
>
> This patch implements proper query of vmcoreinfo and calculation of
> guest note size. Based on that we can surely report whether win dump
> available or not.
>
> For further reference one may review this libvirt discussion:
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB/#HJ3JRLWLGN3IKIC22OQ3PMZ4J3EFG5XB
> [PATCH 0/4] Allow xml-configured coredump format on VM crash
>
> Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
> ---
> dump/win_dump.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index 3162e8bd48..4bb1b28e63 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -14,14 +14,74 @@
> #include "qemu/error-report.h"
> #include "exec/cpu-defs.h"
> #include "hw/core/cpu.h"
> +#include "hw/misc/vmcoreinfo.h"
> #include "qemu/win_dump_defs.h"
> #include "win_dump.h"
> #include "cpu.h"
> +#include "elf.h"
>
> #if defined(TARGET_X86_64)
>
> +#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size) \
> + ((DIV_ROUND_UP((hdr_size), 4) + \
> + DIV_ROUND_UP((name_size), 4) + \
> + DIV_ROUND_UP((desc_size), 4)) * 4)
> +
> bool win_dump_available(Error **errp)
> {
> + uint64_t addr, note_head_size, name_size, desc_size;
> + uint32_t size;
> + uint16_t guest_format;
> + uint8_t *guest_note = NULL;
> + size_t guest_note_size = 0;
> + VMCoreInfoState *vmci = vmcoreinfo_find();
> + ArchDumpInfo dump_info = {};
> + GuestPhysBlockList blocks = {};
> + int ret;
> +
> + if (!vmci || !vmci->has_vmcoreinfo)
> + return false;
> +
> + ret = cpu_get_dump_info(&dump_info, &blocks);
> + if (ret < 0)
> + return false;
> +
> + guest_format = le16_to_cpu(vmci->vmcoreinfo.guest_format);
> + if (guest_format != FW_CFG_VMCOREINFO_FORMAT_ELF)
> + return false;
> +
> + size = le32_to_cpu(vmci->vmcoreinfo.size);
> + addr = le64_to_cpu(vmci->vmcoreinfo.paddr);
> + note_head_size = dump_info.d_class == ELFCLASS64 ?
> + sizeof(Elf64_Nhdr) : sizeof(Elf32_Nhdr);
> +
> + guest_note = g_malloc(size + 1);
> + cpu_physical_memory_read(addr, guest_note, size);
> + if (dump_info.d_class == ELFCLASS64) {
> + const Elf64_Nhdr *hdr = (void *)guest_note;
> + if (dump_info.d_endian == ELFDATA2LSB) {
> + name_size = cpu_to_le64(hdr->n_namesz);
> + desc_size = cpu_to_le64(hdr->n_descsz);
> + } else {
> + name_size = cpu_to_be64(hdr->n_namesz);
> + desc_size = cpu_to_be64(hdr->n_descsz);
> + }
> + } else {
> + const Elf32_Nhdr *hdr = (void *)guest_note;
> + if (dump_info.d_endian == ELFDATA2LSB) {
> + name_size = cpu_to_le32(hdr->n_namesz);
> + desc_size = cpu_to_le32(hdr->n_descsz);
> + } else {
> + name_size = cpu_to_be32(hdr->n_namesz);
> + desc_size = cpu_to_be32(hdr->n_descsz);
> + }
> + }
> +
> + guest_note_size = ELF_NOTE_SIZE(note_head_size, name_size, desc_size);
> + if (guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE64 &&
> + guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE32)
> + return false;
> +
> return true;
> }
>
please start from ./scripts/check-patch.pl
© 2016 - 2025 Red Hat, Inc.