Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Crash Utility (http://www.linux-archive.org/crash-utility/)
-   -   add support to "virsh dump-guest-memory"(qemu memory dump) (http://www.linux-archive.org/crash-utility/685521-add-support-virsh-dump-guest-memory-qemu-memory-dump.html)

HATAYAMA Daisuke 07-20-2012 08:43 AM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
From: qiaonuohan <qiaonuohan@cn.fujitsu.com>
Subject: [Crash-utility] [PATCH] add support to "virsh dump-guest-memory"(qemu memory dump)
Date: Fri, 20 Jul 2012 14:25:58 +0800

> Hello Dave,
>
> I made this patch to make crash can analyze core dump file created by
> "virsh dump-guest-memory"(I will call it "qemu memory dump" below). In
> my test(guest OS: RHEL6.2 x86 & x86_64), the patch works well with
> dump
> files created by "qemu memory dump".
>
> However, after some investigation, I think I need to discuss further
> works with you.
>
> The core dump created by qemu memory dump is similar to kdump. The
> distinctness only focuses on note sections. The former one gets
> note sections with a name called "QEMU".
>
> 1. Some registers' information stored in "CORE" note sections, needed
> by crash, also stores in "QEMU" note sections. I think it's not
> reasonable to replace them. What do you think?
>
> 2. Other registers which are only stored in "QEMU" note sections are
> not directly used in crash. I will continue investigating the use of
> these registers. And if you give some suggestion, it will be helpful.
>

qemu memory dump can be triggered when guest machine is in the kdump
2nd kernel. kdump moves the first 640kB in the reserved area, so then
in order to show users the 1st kernel's view, crash needs to interpret
read requests to the first 640kB properly. See sadump implementation,
which does this so seems helpful.

I also think it useful if we can see dump image from the 2nd kernel's
view. Because we can pass phys_base value via command-line, this
amounts to how to calculate phys_base for the 2nd kernel, and this
needs some meaningful 2nd kernel's address that points at
straight-mapping region, and this is IDT value, which is also included
in QEMU values.

Even if there's no commands that explicitly use QEMU values, it's
useful at least to be able to dump them. Users that know what each
value means can take advantage of them in some senses; at least IDT
value is needed in the above case. Though I don't know the best place
to add the feature. help -D is natural?

Thanks.
HATAYAMA, Daisuke

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

07-20-2012 02:32 PM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
> Hello Dave,
>
> I made this patch to make crash can analyze core dump file created by
> "virsh dump-guest-memory"(I will call it "qemu memory dump" below). In
> my test(guest OS: RHEL6.2 x86 & x86_64), the patch works well with dump
> files created by "qemu memory dump".
>
> However, after some investigation, I think I need to discuss further
> works with you.
>
> The core dump created by qemu memory dump is similar to kdump. The
> distinctness only focuses on note sections. The former one gets
> note sections with a name called "QEMU".
>
> 1. Some registers' information stored in "CORE" note sections, needed
> by crash, also stores in "QEMU" note sections. I think it's not
> reasonable to replace them. What do you think?
>
> 2. Other registers which are only stored in "QEMU" note sections are
> not directly used in crash. I will continue investigating the use of
> these registers. And if you give some suggestion, it will be helpful.
>
> --
> --
> Regards
> Qiao Nuohan

Qiao,

I'm sending this from my home email address -- I am going to be
on vacation for the next two weeks, and will have limited internet
access during that time. So I won't be able to look at this
patch in detail until the week of August 6th.

Anyway, one thing about the patch that I don't like is this
flags/flags2 usage:

#define NETDUMP_DUMPFILE() (pc->flags & (NETDUMP|REM_NETDUMP))
#define DISKDUMP_DUMPFILE() (pc->flags & DISKDUMP)
#define KDUMP_DUMPFILE() (pc->flags & KDUMP)
+#define QEMU_MEM_DUMP_DUMPFILE() ((pc->flags & NETDUMP) && (pc->flags2 &
QEMU_MEM_DUMP))
#define XENDUMP_DUMPFILE() (pc->flags & XENDUMP)
#define XEN_HYPER_MODE() (pc->flags & XEN_HYPER)

None of the other individual dumpfile types "share" a flag with
any other dumpfile type. And so having *both* the NETDUMP flag
*and* the QEMU_MEM_DUMP_DUMPFILE flags set at the same time makes
no sense. It's simply *not* a NETDUMP, and all of the locations
where you do something like this are needlessly confusing:

+ else if (pc->flags & NETDUMP) {
+ if (pc->flags2 & QEMU_MEM_DUMP)
+ retval = qemu_mem_dump_memory_dump(fp);
+ else
+ retval = netdump_memory_dump(fp);
+ }

It may have somehow made your patch easier, but please re-work it
such that QEMU_MEM_DUMP_DUMPFILE() only relies on a single flag bit.

Also, when I get back to work, perhaps you can also make a small sample
dumpfile available to me to test your patch with.

Thanks,
Dave Anderson

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

qiaonuohan 08-03-2012 02:20 AM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
Hello Dave,

The patch has been modified. For every bit of flag has been occupied, I
use flag2 to indicate qemu memory dump.

According to HATAYAMA's suggest, I made another two patch. The 0002
patch is used to deal with the reserved 640KB area. And the 0003 is
used to display register in qemu note section.

Now, I am still working on "see dump image from the 2nd kernel's
view"(See HATAYAMA's mail). As HATAYAMA suggested, only hint message
about calculating phys_base is needed. Then what do think? Is it
suitable add some brief explanation in program_usage_info?

BTW, the following files are what you want, including the core of
RHEL6.2 x86_64 & RHEL6.2 i386

https://skydrive.live.com/redir?resid=62F21E6013CBFCB9!111&authkey=!AF1s2nd-xYJnL3w

https://skydrive.live.com/redir?resid=62F21E6013CBFCB9!112&authkey=!AIp0uoev nNiJLYY

https://skydrive.live.com/redir?resid=62F21E6013CBFCB9!113&authkey=!ACnO0HEh UQmhz4M

https://skydrive.live.com/redir?resid=62F21E6013CBFCB9!114&authkey=!AD58FzZr Gwz3H0E

--
--
Regards
Qiao Nuohan


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

qiaonuohan 08-08-2012 08:03 AM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
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?


> new macro to replace all the usages of "(pc->flags& MEMORY_SOURCES)".
>
> My mistake, Qian -- sorry about that...
>
> 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).


>
> Dave
>
>
>
>
>
>
>


--
--
Regards
Qiao Nuohan



--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 08-08-2012 01:26 PM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
----- 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.

> > 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?

Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

qiaonuohan 08-10-2012 07:50 AM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
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.



Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility





--
--
Regards
Qiao Nuohan


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 08-10-2012 06:46 PM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
----- 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@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

qiaonuohan 08-20-2012 08:32 AM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
Hello Dave,

I have modified the patches, and they are based on crash 6.0.9.

--
--
Regards
Qiao Nuohan



--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

qiaonuohan 08-20-2012 09:19 AM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
At 2012-8-20 16:32, qiaonuohan wrote:


I have modified the patches, and they are based on crash 6.0.9.


I find some mistakes in my former patches, please ignore them and refer
to the attachments of this mail.

--
--
Regards
Qiao Nuohan


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 08-20-2012 07:47 PM

add support to "virsh dump-guest-memory"(qemu memory dump)
 
----- Original Message -----
> At 2012-8-20 16:32, qiaonuohan wrote:
> >
> > I have modified the patches, and they are based on crash 6.0.9.
>
> I find some mistakes in my former patches, please ignore them and refer
> to the attachments of this mail.
>
> --
> --
> Regards
> Qiao Nuohan

The patch is looking better, but a few issues still remain to be
cleaned up.

I'm wondering about the correctness of this addition to read_netdump()
for 32-bit dumpfiles:

int
read_netdump(int fd, void *bufptr, int cnt, ulong addr, physaddr_t paddr)
{
off_t offset;
struct pt_load_segment *pls;
int i;

+ if (nd->flags & QEMU_MEM_DUMP_KDUMP_BACKUP &&
+ paddr >= nd->backup_src_start &&
+ paddr < nd->backup_src_start + nd->backup_src_size) {
+ ulong orig_paddr;
+
+ orig_paddr = paddr;
+ paddr += nd->backup_offset - nd->backup_src_start;
+
+ if (CRASHDEBUG(1))
+ error(INFO,
+ "qemu_mem_dump: kdump backup region: %#lx => %#lx
",
+ orig_paddr, paddr);
+ }

The incoming "paddr" parameter is type physaddr_t, which is declared as:

typedef uint64_t physaddr_t;

I see that you've pretty much copied the "if" statement from read_sadump(),
but I'm not sure whether SADUMP supports 32-bit dumpfiles?

Since nd->backup_src_start is a physical address, maybe it should be
declared as a physaddr_t as well? Or if the incoming paddr is 4GB or
greater, then perhaps it shouldn't be checked against nd->backup_src_start.
In other words, I'm not sure what happens when you do this:

paddr >= nd->backup_src_start

When running on a 32-bit machine, does paddr get truncated to a 32-bit value,
or does nd->backup_src_start get promoted to a 64-bit value?

In any case, whatever you decide, please move the complete "if" statement
above from read_netdump() into read_kdump(). It makes much more sense for
it to be located in read_kdump(), where for example, the incoming "paddr"
is also modified for Xen ELF core files. You can also use the "paddr_in"
that is declared there instead of the 32-bit-invalid "ulong orig_paddr"
that you currently use within your "if" statement.

Lastly, "help -n" should show the new QEMU_MEM_DUMP_KDUMP_BACKUP
vmcore_data.flag, as well as the 3 new "backup_xxx" fields that
you have added to the vmcore_data structure.

Other than that, it looks pretty good.

Thanks,
Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility


All times are GMT. The time now is 03:41 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.