Re: [PATCH] dump: enhance win_dump_available to report properly

["Daniel P. Berrangé" <berrange@redhat.com>]

On 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 :|