----- Original Message ----- > At 2012-8-8 21:26, Dave Anderson wrote: > > > > > > ----- Original Message ----- > >> At 2012-8-8 2:59, Dave Anderson wrote: > >> > >> > > >> > Anyway, it was surprising to note is that the two dumpfiles can already > >> > can be read with no problem by the crash utility -- with no additional > >> > patches to support it. The crash utility just thinks that it's a kdump > >> > with some unknown "QEMU" ELF notes: > >> > >> That's right. The formats are extremely similar, except the qemu note > >> section. > >> > >> > I should have suggested moving one of the currently-existing pc->flags bits into > >> > pc->flags2, and keeping all the dumpfile types in pc->flags. Or at least create a > >> > >> I think it's a better choice. But encountering a new type of dump file, > >> creating a patch used to move bits in pc->flags can easily put things > >> into a mess. Why not use a new flag only used to keep dumpfile types? > > > > Yes, that's probably a better idea. When the next "real" dumpfile type > > comes along, perhaps we can go that route. > > I see. And I reworked the patches to treat qemu memory dump as kdump. > > > >> > But -- it seems that we can just leave the dumpfile type as a "KDUMP_ELF32/KDUMP_ELF64", > >> > and then based upon the existence of a "QEMU" note, just set a QEMU_MEM_DUMP pc->flags2 > >> > bit that can be used everywhere that you're interested in accessing the "QEMU" > >> > notes/registers section? Wouldn't that be far simpler? > >> > >> Treat it as a kdump files seems cool to me. The only problem is I still > >> need to distinguish kdump and qemu memory dump, not only using qemu note > >> section, but also the first 640K is special(when doing qemu memory dump, > >> the second kernel is working). > > > > Right, and you will be able to set the new QEMU_MEM_DUMP flag very early > > on during the dumpfile determination phase so that later you will have > > that knowledge at hand later on when you want to deal with the notes. > > > > With respect to the "second-kernel" issue, I would presume that you would > > have to use the currently-existing "--machdep phys_base=<physaddr>" capability > > for x86_64? > > Yes, that's right. To see dump image from the 2nd kernel's view, > phys_base should be set. > > P.S. > I will suspend the work for about one week because of personal affairs. Hello Qiao, Here are my comments on your latest patch. First, I have a couple problems with your check_elf_note() implementation. It gets called regardless whether it's a QEMU-generated dumpfile or not. You call the function here in is_netdump(): if ((load32->p_offset & (MIN_PAGE_SIZE-1)) && (load32->p_align == 0)) { tmp_flags |= KDUMP_ELF32; if (check_elf_note(eheader, fd, file, 0)) pc->flags2 |= QEMU_MEM_DUMP; } else if ((load64->p_offset & (MIN_PAGE_SIZE-1)) && (load64->p_align == 0)) { tmp_flags |= KDUMP_ELF64; if (check_elf_note(eheader, fd, file, 1)) pc->flags2 |= QEMU_MEM_DUMP; } else That forces *all* non-QEMU netdumps and kdumps to run through the complete check_elf_note() function for no reason at all. But more importantly, the check_elf_note() is a huge duplication of effort. It completely parses the ELF header looking for ELF notes, and when it finds a "QEMU" note string: if (STREQ(name, "QEMU")) { qemu_memory_dump = TRUE; break; } it ends up returning TRUE back to is_netdump(), which sets the QEMU_MEM_DUMP flag. But then later on, is_netdump() calls either dump_Elf32_Nhdr() or dump_Elf64_Nhdr for every ELF note. And in those two functions you've added this: qemu_info = STRNEQ(buf, "QEMU"); in order to decide whether to display the register data. But given that you are again just checking for the "QEMU" string in those two functions above, then check_elf_note() is completely unnecessary. Just set the QEMU_MEM_DUMP flag in those two locations in dump_Elf32_Nhdr() and dump_Elf64_Nhdr() -- that's what those two function are there for. And lastly, the output of "crash -d1" and "help -n" is really kind of ugly. Plus this looks to be a bug in both dump_Elf32_Nhdr() and dump_Elf64_Nhdr() where you've got: if (CRASHDEBUG(1) & qemu_info) { It should be "&&" so that "help -n" does not display the register data unless CRASHDEBUG(x) is turned on. But speaking to the "ugliness" issue, it really is confusing to see the "normal" PT_NOTE raw data display interspersed with your new QEMU register translation. How about separating it into a unique display? Maybe something like "help -q" could do something like: crash> help q CPU 0: <register data> CPU 1: <register data> ... Thanks, Dave -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility