Re: [Crash-utility] [PATCH] add support to "virsh dump-guest-memory"(qemu memory dump)
----- Original Message -----
From: qiaonuohan <qiaonuohan(a)cn.fujitsu.com>
Subject: Re: [Crash-utility] [PATCH] add support to "virsh
dump-guest-memory"(qemu memory dump)
Date: Tue, 21 Aug 2012 14:05:25 +0800
>>From c2f4d203d1610213ce2af9754d857f7f2192f121 Mon Sep 17 00:00:00 2001
> From: qiaonuohan <qiaonuohan(a)cn.fujitsu.com>
> Date: Tue, 21 Aug 2012 13:35:54 +0800
> Subject: [PATCH 2/3] support core dump file when kdump is operating
>
> diff --git a/main.c b/main.c
> index 9dced6e..033bfa1 100755
> --- a/main.c
> +++ b/main.c
> @@ -640,6 +640,8 @@ main_loop(void)
> if (!(pc->flags & GDB_INIT)) {
> gdb_session_init();
> show_untrusted_files();
> + if (pc->flags2 & QEMU_MEM_DUMP)
> + qemu_mem_dump_kdump_backup_region_init();
> if (SADUMP_DUMPFILE())
> sadump_kdump_backup_region_init();
> if (XEN_HYPER_MODE()) {
This is different from what Dave suggests. He suggests it's useful if
this conversion is applied also to Xen part. Now the conversion is
done for vmcore with qemu note only. The previous patch sets
QEMU_MEM_DUMP if QEMU note exists. To identify Xen's vmcore, some kind
of the sign that indicates "this vmcore is Xen's" is needed.
I'm now thinking that this backup region processing should be written
commonly among crash dump mechanisms running independently of
kdump. They are now qemu's dump-guest-memory, xen's dump mechanism and
sadump.
Wait a minute -- I don't understand how "xen's dump mechanism" has
anything
to do with this backup region processing?
-> He suggests it's useful if this conversion is applied also to Xen part.
No, when I referred to Xen with respect to the read_kdump() function, I was only
indicating that there is an "if xen" statement there that translates a Xen
psuedo-physical address into a Xen machine address -- where the incoming
"paddr"
argument gets modified by xen_kdump_p2m() before being passed to the common
read_netump() function. And given that Qiao's patch is:
(1) also changing the incoming "paddr" argument on the fly, and
(2) it's a kdump clone,
then it makes more sense that his check be done in read_kdump() instead of
in read_netdump().
Anyway, now looking at sadump.c, I see that sadump_kdump_backup_region_init()
and Qiao's qemu_mem_dump_kdump_backup_region_init() function are pretty much
equivalent except for:
(1) qemu_mem_dump_kdump_backup_region_init() supports 32-bit ELF vmcores,
based upon the setting of the nd->flags KDUMP_ELF32/KDUMP_ELF64 bits, and
(2) qemu_mem_dump_kdump_backup_region_init() uses the pre-existing vmcore_data
structure from netdump.c, and sadump.c uses its own sadump_data structure,
where both structures have the same thing at the end of the struct:
...
/* Backup Region, First 640K of System RAM. */
#define KEXEC_BACKUP_SRC_END 0x0009ffff
ulong backup_src_start;
ulong backup_src_size;
ulonglong backup_offset;
};
So I agree that they could be a common function, and we could
have the main_loop() function do something like this:
if (!(pc->flags & GDB_INIT)) {
gdb_session_init();
show_untrusted_files();
+ kdump_backup_region_init()
- if (SADUMP_DUMPFILE())
- sadump_kdump_backup_region_init();
if (XEN_HYPER_MODE()) {
The common kdump_backup_region_init() could be put in netdump.c, where
it can check for SADUMP_DUMPFILE() or (pc->flags2 & QEMU_MEM_DUMP), and
if either is set, do its thing. The main issue would be dealing with the
usage of the two different structures. netdump.c can safely #include
sadump.h, and there would only be a need for a new global function in
sadump.c similar to this one:
struct vmcore_data *get_kdump_vmcore_data(void);
but which passes a pointer to the sadump_data structure back to the
common function in netdump.c.
What do you guys think?
Dave