1:46 p.m.
----- 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