On Thu, Jun 01, 2017 at 05:03:23PM +0400, Marc-André Lureau wrote:
> Read vmcoreinfo note from guest memory when dump_info provides the
> address, and write it as an ELF note in the dump.
>
> NUMBER(phys_base) in vmcoreinfo has only been recently introduced in
> Linux 4.10 ("kexec: export the value of phys_base instead of symbol
> address"). To accomadate for older kernels, modify the vmcoreinfo to add
> the new fields and help newer crash that will use it.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> include/sysemu/dump.h | 2 +
> dump.c | 133
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 135 insertions(+)
>
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index 2672a15f8b..b8a7a1e41d 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -192,6 +192,8 @@ typedef struct DumpState {
> * this could be used to calculate
> * how much work we have
> * finished. */
> + uint8_t *vmcoreinfo;
> + size_t vmcoreinfo_size;
> } DumpState;
>
> uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
> diff --git a/dump.c b/dump.c
> index bdf3270f02..6911ffad8b 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -27,6 +27,7 @@
> #include "qapi/qmp/qerror.h"
> #include "qmp-commands.h"
> #include "qapi-event.h"
> +#include "qemu/error-report.h"
>
> #include <zlib.h>
> #ifdef CONFIG_LZO
> @@ -88,6 +89,8 @@ static int dump_cleanup(DumpState *s)
> qemu_mutex_unlock_iothread();
> }
> }
> + g_free(s->vmcoreinfo);
> + s->vmcoreinfo = NULL;
>
> return 0;
> }
> @@ -238,6 +241,19 @@ static inline int cpu_index(CPUState *cpu)
> return cpu->cpu_index + 1;
> }
>
> +static void write_vmcoreinfo_note(WriteCoreDumpFunction f, DumpState *s,
> + Error **errp)
> +{
> + int ret;
> +
> + if (s->vmcoreinfo) {
> + ret = f(s->vmcoreinfo, s->vmcoreinfo_size, s);
> + if (ret < 0) {
> + error_setg(errp, "dump: failed to write vmcoreinfo");
> + }
> + }
> +}
> +
> static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
> Error **errp)
> {
> @@ -261,6 +277,8 @@ static void write_elf64_notes(WriteCoreDumpFunction f,
> DumpState *s,
> return;
> }
> }
> +
> + write_vmcoreinfo_note(f, s, errp);
> }
>
> static void write_elf32_note(DumpState *s, Error **errp)
> @@ -306,6 +324,8 @@ static void write_elf32_notes(WriteCoreDumpFunction f,
> DumpState *s,
> return;
> }
> }
> +
> + write_vmcoreinfo_note(f, s, errp);
> }
>
> static void write_elf_section(DumpState *s, int type, Error **errp)
> @@ -717,6 +737,50 @@ static int buf_write_note(const void *buf, size_t size,
> void *opaque)
> return 0;
> }
>
> +static void get_note_sizes(DumpState *s, const void *note,
> + uint64_t *note_head_size,
> + uint64_t *name_size,
> + uint64_t *desc_size)
> +{
> + uint64_t note_head_sz;
> + uint64_t name_sz;
> + uint64_t desc_sz;
> +
> + if (s->dump_info.d_class == ELFCLASS64) {
> + const Elf64_Nhdr *hdr = note;
> + note_head_sz = sizeof(Elf64_Nhdr);
> + name_sz = hdr->n_namesz;
> + desc_sz = hdr->n_descsz;
> + } else {
> + const Elf32_Nhdr *hdr = note;
> + note_head_sz = sizeof(Elf32_Nhdr);
> + name_sz = hdr->n_namesz;
> + desc_sz = hdr->n_descsz;
> + }
> +
> + if (note_head_size) {
> + *note_head_size = note_head_sz;
> + }
> + if (name_size) {
> + *name_size = name_sz;
> + }
> + if (desc_size) {
> + *desc_size = desc_sz;
> + }
> +}
> +
> +static void set_note_desc_size(DumpState *s, void *note,
> + uint64_t desc_size)
> +{
> + if (s->dump_info.d_class == ELFCLASS64) {
> + Elf64_Nhdr *hdr = note;
> + hdr->n_descsz = desc_size;
> + } else {
> + Elf32_Nhdr *hdr = note;
> + hdr->n_descsz = desc_size;
> + }
> +}
> +
> /* write common header, sub header and elf note to vmcore */
> static void create_header32(DumpState *s, Error **errp)
> {
> @@ -1491,6 +1555,42 @@ static int64_t dump_calculate_size(DumpState *s)
> return total;
> }
>
> +static void vmcoreinfo_add_phys_base(DumpState *s)
> +{
> + uint64_t size, note_head_size, name_size;
> + char **lines, *physbase = NULL;
> + uint8_t *newvmci, *vmci;
> + size_t i;
> +
> + get_note_sizes(s, s->vmcoreinfo, ¬e_head_size, &name_size, &size);
> + note_head_size = ((note_head_size + 3) / 4) * 4;
> + name_size = ((name_size + 3) / 4) * 4;
I'd prefer to replace all the + 3 / 4 * 4 stuff with ROUND_UP()
> + vmci = s->vmcoreinfo + note_head_size + name_size;
How about using another size variable (e.g. sz) = note_head_size + name_size
> + *(vmci + size) = '\0';
> + lines = g_strsplit((char *)vmci, "\n", -1);
> + for (i = 0; lines[i]; i++) {
> + if (g_str_has_prefix(lines[i], "NUMBER(phys_base)=")) {
> + goto end;
> + }
> + }
I don't think it should be necessary to split the info first. We can
just search the entire info for the substring '\nNUMBER(phys_base)='.
> +
> + physbase = g_strdup_printf("\nNUMBER(phys_base)=%ld",
> + s->dump_info.phys_base);
sz += size
> + s->vmcoreinfo_size =
> + ((note_head_size + name_size + size + strlen(physbase) + 3) / 4) * 4;
> +
> + newvmci = g_malloc(s->vmcoreinfo_size);
> + memcpy(newvmci, s->vmcoreinfo, note_head_size + name_size + size - 1);
> + memcpy(newvmci + note_head_size + name_size + size - 1, physbase,
> + strlen(physbase) + 1);
> + g_free(s->vmcoreinfo);
> + s->vmcoreinfo = newvmci;
How about using g_realloc() instead?
> + set_note_desc_size(s, s->vmcoreinfo, size + strlen(physbase));
> +
> +end:
> + g_strfreev(lines);
> +}
> +
> static void dump_init(DumpState *s, int fd, bool has_format,
> DumpGuestMemoryFormat format, bool paging, bool
> has_filter,
> int64_t begin, int64_t length, Error **errp)
> @@ -1566,6 +1666,39 @@ static void dump_init(DumpState *s, int fd, bool
> has_format,
> goto cleanup;
> }
>
> + if (dump_info.has_phys_base) {
> + s->dump_info.phys_base = dump_info.phys_base;
> + }
> + if (dump_info.vmcoreinfo) {
> + uint64_t addr, size, note_head_size, name_size, desc_size;
> + int count = sscanf(dump_info.vmcoreinfo, "%" PRIx64 " %" PRIx64,
> + &addr, &size);
> + if (count != 2) {
> + /* non fatal error */
> + error_report("Failed to parse vmcoreinfo");
> + } else {
> + assert(!s->vmcoreinfo);
> + s->vmcoreinfo = g_malloc(size);
> + cpu_physical_memory_read(addr, s->vmcoreinfo, size);
> +
> + get_note_sizes(s, s->vmcoreinfo,
> + ¬e_head_size, &name_size, &desc_size);
> + s->vmcoreinfo_size = ((note_head_size + 3) / 4 +
> + (name_size + 3) / 4 +
> + (desc_size + 3) / 4) * 4;
> + if (s->vmcoreinfo_size > size) {
> + error_report("Invalid vmcoreinfo header, size mismatch");
> + g_free(s->vmcoreinfo);
> + s->vmcoreinfo = NULL;
> + } else {
> + if (dump_info.has_phys_base) {
> + vmcoreinfo_add_phys_base(s);
> + }
> + s->note_size += s->vmcoreinfo_size;
> + }
> + }
> + }
> +
> /* get memory mapping */
> if (paging) {
> qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, &err);
> --
> 2.13.0.91.g00982b8dd
>
>
Thanks,
drew
