9:48 a.m.
Hi Dave,
Recently I used crash-tool for the first time and I was pleasantly surprised,
it really looks like a very useful and handy debugging tool ;)
And I was surprised again when I figured out that it can be used to debug the
live system on the same machine. Cool!
Now I am wondering if we can teach it to debug the live guests runnning under
qemu/kvm. This looks certainly possible, qemu supports gdb remote protocol.
But. this obviously needs more work. And. Afaics crash-tool needs some fixes
anyway. See 01/10-07/10, but probably it needs more changes, so far I only
tried to audit task.c and kernel.c. I tried to document every change, but I
am very new to this code so I can be easily wrong.
So can't we teach it to work with live/raw RAM dumpfiles for the start? See
09/10 and 10/10.
With these changes I can run qemu-kvm with the
-object memory-backend-file,id=MEM,size=128m,mem-path=/tmp/MEM,share=on
-numa node,memdev=MEM
options and then do
$ crash path-to-guests-vmlinux live:/tmp/MEM@0
to debug the live guest, No need to dump-guest-memory + restart /usr/bin/crash
which can be slow.
What do you think?
Oleg.
defs.h | 1 +
filesys.c | 8 ++++----
kernel.c | 7 +++----
main.c | 11 +++++++++++
ramdump.c | 49 ++++++++++++++++++++++++++++++++++---------------
task.c | 13 ++++++-------
tools.c | 2 +-
7 files changed, 60 insertions(+), 31 deletions(-)
9:48 a.m.
New subject: [PATCH 01/10] introduce LOCAL_ACTIVE() helper
Add the new simple helper, LOCAL_ACTIVE(). True if crash examines the live kernel
on the same machine.
I do not really like to use MEMSRC_LOCAL; it would be better to define this helper
as ACTIVE() && !REMOTE_ACTIVE(), the last patch in series could set
REM_LIVE_SYSTEM.
But this can't really work without other cleanups.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
defs.h | 1 +
filesys.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/defs.h b/defs.h
index 56ae06c..43c0d15 100644
--- a/defs.h
+++ b/defs.h
@@ -251,6 +251,7 @@ struct number_option {
#define PROC_KCORE (0x8000000000000000ULL)
#define ACTIVE() (pc->flags & LIVE_SYSTEM)
+#define LOCAL_ACTIVE() ((pc->flags & (LIVE_SYSTEM|MEMSRC_LOCAL)) ==
(LIVE_SYSTEM|MEMSRC_LOCAL))
#define DUMPFILE() (!(pc->flags & LIVE_SYSTEM))
#define LIVE() (pc->flags2 & LIVE_DUMP || pc->flags &
LIVE_SYSTEM)
#define MEMORY_SOURCES
(NETDUMP|KDUMP|MCLXCD|LKCD|DEVMEM|S390D|MEMMOD|DISKDUMP|XENDUMP|CRASHBUILTIN|KVMDUMP|PROC_KCORE|SADUMP|VMWARE_VMSS)
diff --git a/filesys.c b/filesys.c
index 9b59998..a9f54b1 100644
--- a/filesys.c
+++ b/filesys.c
@@ -123,7 +123,7 @@ fd_init(void)
}
if (!pc->dumpfile) {
- pc->flags |= LIVE_SYSTEM;
+ pc->flags |= (LIVE_SYSTEM|MEMSRC_LOCAL);
get_live_memory_source();
}
--
2.5.0
9:48 a.m.
New subject: [PATCH 02/10] check LOCAL_ACTIVE() rather than ACTIVE() in memory_source_init() path
Change fd_init() and memory_source_init() to check LOCAL_ACTIVE(). We need
this for the last patch in series.
The 1st change is obviously safe, fd_init() sets LIVE_SYSTEM | MEMSRC_LOCAL
and nobody else can set LIVE_SYSTEM if we get here.
The 2nd one should be correct even if REMOTE() is true, we check MEMSRC_LOCAL
at the start of memory_source_init().
---
filesys.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/filesys.c b/filesys.c
index a9f54b1..de04486 100644
--- a/filesys.c
+++ b/filesys.c
@@ -134,7 +134,7 @@ fd_init(void)
pc->nfd = -1;
}
- if (ACTIVE() && !(pc->namelist_debug || pc->system_map)) {
+ if (LOCAL_ACTIVE() && !(pc->namelist_debug || pc->system_map)) {
memory_source_init();
match_proc_version();
}
@@ -168,7 +168,7 @@ memory_source_init(void)
if (pc->flags & KERNEL_DEBUG_QUERY)
return;
- if (ACTIVE()) {
+ if (LOCAL_ACTIVE()) {
if (pc->mfd != -1) /* already been here */
return;
--
2.5.0
9:48 a.m.
New subject: [PATCH 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
Obviously the /proc/$tc->pid check can only make sense if LOCAL_ACTIVE().
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
kernel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel.c b/kernel.c
index a6fa7a6..6dfc43b 100644
--- a/kernel.c
+++ b/kernel.c
@@ -2900,7 +2900,7 @@ back_trace(struct bt_info *bt)
return;
}
- if (ACTIVE() && !INSTACK(esp, bt)) {
+ if (LOCAL_ACTIVE() && !INSTACK(esp, bt)) {
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
--
2.5.0
8:49 a.m.
New subject: [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
On 04/25, Oleg Nesterov wrote:
- if (ACTIVE() && !INSTACK(esp, bt)) {
+ if (LOCAL_ACTIVE() && !INSTACK(esp, bt)) {
Of course, this is wrong, please see v2 below.
-------------------------------------------------------------------------------
Subject: [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
Obviously the /proc/$tc->pid check can only make sense if LOCAL_ACTIVE().
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
kernel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel.c b/kernel.c
index a6fa7a6..cf0a6a0 100644
--- a/kernel.c
+++ b/kernel.c
@@ -2902,7 +2902,7 @@ back_trace(struct bt_info *bt)
if (ACTIVE() && !INSTACK(esp, bt)) {
sprintf(buf, "/proc/%ld", bt->tc->pid);
- if (!file_exists(buf, NULL))
+ if (!(LOCAL_ACTIVE() && file_exists(buf, NULL)))
error(INFO, "task no longer exists\n");
else
error(INFO,
--
2.5.0
9:23 a.m.
New subject: [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
----- Original Message -----
This doesn't make sense to me. If it's !LOCAL_ACTIVE() (i.e. hybrid-live-dump),
then
why would you want to call file_exists()?
Shouldn't it be: LOCAL_ACTIVE() and !file_exists()
Dave
10:30 a.m.
New subject: [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
On 04/28, Dave Anderson wrote:
It won't be called in this case, please see below.
Shouldn't it be: LOCAL_ACTIVE() and !file_exists()
This is what I did initially... then decided that error("task no longer
exists\n")
makes more sense if !LOCAL_ACTIVE() && !INSTACK(esp, bt).
IOW. with the patch above the code actually does
if (ACTIVE() && !INSTACK(...)) {
if (LOCAL_ACTIVE() && file_exists(...))
error("invalid/stale stack pointer");
else
error("task no longer exists\n");
}
is it wrong?
I thought that !INSTACK() here likely means the task has gone, but back_trace()
does the additional file_exists() to verify this, and "invalid/stale stack
pointer"
error means that something was wrong.
No?
Oleg.
10:45 a.m.
New subject: [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
----- Original Message -----
I can't even remember -- that code's been in place for so long I'd prefer to
just
leave it as-is, and for you to just add something like this:
if (ACTIVE() && !INSTACK(esp, bt)) {
+ if (!(LOCAL_ACTIVE()) }
+ error(INFO, "whatever error message you'd like\n");
+ return;
+ }
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
else
error(INFO,
"invalid/stale stack pointer for this task: %lx\n",
esp);
return;
}
Dave
11:58 a.m.
New subject: [PATCH v3 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
On 04/28, Dave Anderson wrote:
OK.
-------------------------------------------------------------------------------
Subject: [PATCH v3 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
Obviously the /proc/$tc->pid check can only make sense if LOCAL_ACTIVE().
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
kernel.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel.c b/kernel.c
index a6fa7a6..c82b854 100644
--- a/kernel.c
+++ b/kernel.c
@@ -2901,6 +2901,10 @@ back_trace(struct bt_info *bt)
}
if (ACTIVE() && !INSTACK(esp, bt)) {
+ if (!LOCAL_ACTIVE()) {
+ error(INFO, "task no longer exists\n");
+ return;
+ }
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
--
2.5.0
9:48 a.m.
New subject: [PATCH 04/10] task_init: don't abuse pc->program_pid if !LOCAL_ACTIVE()
Use pid == 1 and /sbin/init as tt->this_task instead, pc->program_pid can't
be used.
We could probably leave tt->this_task = NO_TASK, but we need to initialize
CURRENT_CONTEXT() anyway and we need something for refresh_context(), to me
"init" looks better than (say) FIRST_CONTEXT() == swapper/0.
However, this means that /init/sbin will be always reported as "active", see
the next patch.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
task.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/task.c b/task.c
index 7b01951..fd4b700 100644
--- a/task.c
+++ b/task.c
@@ -124,7 +124,6 @@ task_init(void)
long tss_offset, thread_offset;
long eip_offset, esp_offset, ksp_offset;
struct gnu_request req;
- ulong active_pid;
if (!(tt->idle_threads = (ulong *)calloc(NR_CPUS, sizeof(ulong))))
error(FATAL, "cannot malloc idle_threads array");
@@ -493,7 +492,9 @@ task_init(void)
tt->flags &= ~(TASK_REFRESH|TASK_REFRESH_OFF);
if (ACTIVE()) {
- active_pid = REMOTE() ? pc->server_pid : pc->program_pid;
+ ulong active_pid = REMOTE() ? pc->server_pid :
+ LOCAL_ACTIVE() ? pc->program_pid :
+ 1;
set_context(NO_TASK, active_pid);
tt->this_task = pid_to_task(active_pid);
}
--
2.5.0
9:49 a.m.
New subject: [PATCH 05/10] simplify the "tt->this_task should be active" logic
After the previous patch the "task == tt->this_task" checks in back_trace()
and
is_task_active() lead to falsely "active" state if ACTIVE() &&
!LOCAL_ACTIVE().
This is easy to fix, but it seems to me that we can just simplify this logic and
remove the tt->this_task check from back_trace(), although I'm afraid I missed
something.
It look really simple: if LOCAL_ACTIVE() than "this_task" is /usr/bin/crash and
it is obviously running, otherwise we can never assume this if !task_has_cpu().
Not to mention that "deprecation of usage of has_cpu on non-SMP systems" looks
confusing, task_has_cpu() can work without VALID_MEMBER(task_struct_has_cpu).
BTW, I think that task_init() should use "on_cpu" if
MEMBER_OFFSET_INIT(has_cpu)
fails, but this is off-topic.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
kernel.c | 3 +--
task.c | 8 +++-----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/kernel.c b/kernel.c
index 6dfc43b..e73d6fe 100644
--- a/kernel.c
+++ b/kernel.c
@@ -2800,8 +2800,7 @@ back_trace(struct bt_info *bt)
return;
}
- if (LIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
- ((bt->task == tt->this_task) || is_task_active(bt->task))) {
+ if (LIVE() && !(bt->flags & BT_EFRAME_SEARCH) &&
is_task_active(bt->task)) {
if (BT_REFERENCE_CHECK(bt) ||
bt->flags & (BT_TEXT_SYMBOLS_PRINT|BT_TEXT_SYMBOLS_NOPRINT))
diff --git a/task.c b/task.c
index fd4b700..a6bc9e7 100644
--- a/task.c
+++ b/task.c
@@ -5359,18 +5359,16 @@ is_task_active(ulong task)
{
int has_cpu;
+ if (LOCAL_ACTIVE() && (task == tt->this_task))
+ return TRUE;
if (DUMPFILE() && is_panic_thread(task))
return TRUE;
fill_task_struct(task);
- has_cpu = tt->last_task_read ?
+ has_cpu = tt->last_task_read ?
task_has_cpu(task, tt->task_struct) : 0;
- if (!(kt->flags & SMP) && !has_cpu && ACTIVE() &&
- (task == tt->this_task))
- has_cpu = TRUE;
-
return(has_cpu);
}
--
2.5.0
9:49 a.m.
New subject: [PATCH 06/10] panic_this_kernel: check LOCAL_ACTIVE() rather than DUMPFILE()
Obviously it can only work if LOCAL_ACTIVE() == T.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
kernel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel.c b/kernel.c
index e73d6fe..65e71ec 100644
--- a/kernel.c
+++ b/kernel.c
@@ -8448,7 +8448,7 @@ panic_this_kernel(void)
{
pid_t zero_pid = 0;
- if (DUMPFILE())
+ if (!LOCAL_ACTIVE())
error(FATAL, "cannot panic a dumpfile!\n");
if (!(pc->flags & MFD_RDWR) || (pc->flags & MEMMOD))
--
2.5.0
9:49 a.m.
New subject: [PATCH 07/10] cmd_set: use tt->this_task rather than pc->program_pid
pc->program_pid is only correct if LOCAL_ACTIVE(), tt->this_task is always
fine even if REMOTE().
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
tools.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools.c b/tools.c
index afc6cb3..fef262d 100644
--- a/tools.c
+++ b/tools.c
@@ -1787,7 +1787,7 @@ cmd_set(void)
return;
if (ACTIVE()) {
- set_context(NO_TASK, pc->program_pid);
+ set_context(tt->this_task, NO_PID);
show_context(CURRENT_CONTEXT());
return;
}
--
2.5.0
9:49 a.m.
New subject: [PATCH 08/10] shift ramdump_def.end_paddr calculation from alloc_program_headers() to is_ramdump()
Preparation. This also simplifies the code a little bit.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
ramdump.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/ramdump.c b/ramdump.c
index ab93767..c6a8a85 100644
--- a/ramdump.c
+++ b/ramdump.c
@@ -59,29 +59,19 @@ static void alloc_elf_header(Elf64_Ehdr *ehdr, ushort e_machine)
ehdr->e_shstrndx = 0;
}
-static int alloc_program_headers(Elf64_Phdr *phdr)
+static void alloc_program_headers(Elf64_Phdr *phdr)
{
unsigned int i;
- struct stat64 st;
for (i = 0; i < nodes; i++) {
phdr[i].p_type = PT_LOAD;
-
- if (0 > stat64(ramdump[i].path, &st)) {
- error(INFO, "ramdump stat failed\n");
- return -1;
- }
-
- phdr[i].p_filesz = st.st_size;
+ phdr[i].p_filesz = ramdump[i].end_paddr + 1 - ramdump[i].start_paddr;
phdr[i].p_memsz = phdr[i].p_filesz;
phdr[i].p_vaddr = 0;
phdr[i].p_paddr = ramdump[i].start_paddr;
- ramdump[i].end_paddr = ramdump[i].start_paddr + st.st_size - 1;
phdr[i].p_flags = PF_R | PF_W | PF_X;
phdr[i].p_align = 0;
}
-
- return 0;
}
static char *write_elf(Elf64_Phdr *load, Elf64_Ehdr *e_head, size_t data_offset)
@@ -219,8 +209,7 @@ char *ramdump_to_elf(void)
load = (Elf64_Phdr *)ptr;
- if (alloc_program_headers(load))
- goto end;
+ alloc_program_headers(load);
offset += sizeof(Elf64_Phdr) * nodes;
ptr += sizeof(Elf64_Phdr) * nodes;
@@ -248,6 +237,7 @@ int is_ramdump(char *p)
char *x = NULL, *y = NULL, *pat;
size_t len;
char *pattern;
+ struct stat64 st;
int err = 0;
if (nodes || !strchr(p, '@'))
@@ -277,6 +267,10 @@ int is_ramdump(char *p)
"ramdump %s open failed:%s\n",
ramdump[nodes - 1].path,
strerror(errno));
+ if (fstat64(ramdump[nodes - 1].rfd, &st) < 0)
+ error(FATAL, "ramdump stat failed\n");
+ ramdump[nodes - 1].end_paddr =
+ ramdump[nodes - 1].start_paddr + st.st_size - 1;
}
pat = NULL;
--
2.5.0
9:49 a.m.
New subject: [PATCH 09/10] teach crash to accept raw:MEMORY-IMAGE[@ADDRESS]
With this change /usr/bin/crash will try to work with ramdump files without
creating the temporary KDUMP elf header if MEMORY-IMAGE starts with the "raw:"
prefix. This looks useful to me, especially because ramdump_to_elf() doesn't
support x86-64.
For example, I am running qemu with
-object memory-backend-file,id=MEM,size=128m,mem-path=/tmp/MEM,share=on
-numa node,memdev=MEM
options and after I crash the kernel (/bin/bash running as "init" exits)
$ ./crash ../VMLINUX raw:/tmp/MEM@0
reports:
KERNEL: ../VMLINUX
DUMPFILES: ramdump [raw]
/tmp/MEM
CPUS: 2 [OFFLINE: 1]
DATE: Mon Apr 25 11:47:26 2016
UPTIME: 00:00:07
LOAD AVERAGE: 0.00, 0.00, 0.00
TASKS: 45
NODENAME: (none)
RELEASE: 3.10.0+
VERSION: #42 SMP Sun Apr 3 17:09:09 CEST 2016
MACHINE: x86_64 (2593 Mhz)
MEMORY: 127.5 MB
PANIC: "Kernel panic - not syncing: Attempted to kill init!
exitcode=0x00000000"
PID: 1
COMMAND: "bash"
TASK: ffff880007150000 [THREAD_INFO: ffff880007158000]
CPU: 0
STATE: TASK_RUNNING (PANIC)
crash> bt
PID: 1 TASK: ffff880007150000 CPU: 0 COMMAND: "bash"
#0 [ffff88000715bc28] __schedule at ffffffff816801c0
#1 [ffff88000715bc40] __module_text_address at ffffffff810ef782
#2 [ffff88000715bc58] is_module_text_address at ffffffff810f625e
#3 [ffff88000715bc68] __kernel_text_address at ffffffff810a85ad
#4 [ffff88000715bc80] print_context_stack at ffffffff810174e3
#5 [ffff88000715bce0] dump_trace at ffffffff81016391
#6 [ffff88000715bd60] show_trace_log_lvl at ffffffff810176ad
#7 [ffff88000715bda8] up at ffffffff810b1cf2
#8 [ffff88000715bdc8] console_unlock at ffffffff8108137e
#9 [ffff88000715be08] i8042_panic_blink at ffffffff814c362d
#10 [ffff88000715be28] panic at ffffffff81679de0
#11 [ffff88000715beb0] do_exit at ffffffff810870ff
#12 [ffff88000715bf40] do_group_exit at ffffffff81087183
#13 [ffff88000715bf70] sys_exit_group at ffffffff81087204
#14 [ffff88000715bf80] system_call_fastpath at ffffffff8168ba89
RIP: 00007f2493bdaaf8 RSP: 00007fff2ae49ee8 RFLAGS: 00010206
RAX: 00000000000000e7 RBX: ffffffff8168ba89 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
RBP: 00007f2493ec98b8 R8: 00000000000000e7 R9: ffffffffffffff98
R10: 00007f24934dcd98 R11: 0000000000000246 R12: ffffffff81087204
R13: ffff88000715bf78 R14: 0000000000000000 R15: 00007f2493ecec60
ORIG_RAX: 00000000000000e7 CS: 0033 SS: 002b
and everything else seems to work too.
Note:
- The usage of CRASHBUILTIN doesn't look nice, we need to cleanup
this logic. I hope we can do this later, and it seems to me that
the usage of MEMORY_SOURCES/DUMPFILE_TYPES needs some cleanups in
any case.
- I can't verify this because x86-64 is not supported, but afaics
the current usage of RAMDUMP/is_ramdump_image/read_ramdump is not
right. RAMDUMP should be always set if is_ramdump() returns true,
and read_ramdump() should not be used if we set KDUMP.
But I can't test this, so the patch adds another is_ramdump_image()
block before ramdump_to_elf(). Which obviously asks for cleanup too.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
filesys.c | 2 +-
main.c | 11 +++++++++++
ramdump.c | 21 ++++++++++++++++++++-
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/filesys.c b/filesys.c
index de04486..a1d1d1e 100644
--- a/filesys.c
+++ b/filesys.c
@@ -207,7 +207,7 @@ memory_source_init(void)
return;
}
- if (pc->dumpfile) {
+ if (pc->dumpfile && !(pc->flags & CRASHBUILTIN)) {
if (!file_exists(pc->dumpfile, NULL))
error(FATAL, "%s: %s\n", pc->dumpfile,
strerror(ENOENT));
diff --git a/main.c b/main.c
index 821bb4e..9b72c7e 100644
--- a/main.c
+++ b/main.c
@@ -428,6 +428,17 @@ main(int argc, char **argv)
"too many dumpfile arguments\n");
program_usage(SHORT_FORM);
}
+
+ if (is_ramdump_image()) {
+ /*
+ * FIXME: we set CRASHBUILTIN to tell memory_source_init()
+ * it should ignore pc->dumpfile and for memory_page_size().
+ */
+ pc->flags |= CRASHBUILTIN;
+ optind++;
+ continue;
+ }
+
pc->dumpfile = ramdump_to_elf();
if (is_kdump(pc->dumpfile, KDUMP_LOCAL)) {
pc->flags |= KDUMP;
diff --git a/ramdump.c b/ramdump.c
index c6a8a85..e9716ae 100644
--- a/ramdump.c
+++ b/ramdump.c
@@ -232,14 +232,22 @@ end:
return e_file;
}
+#define PREFIX(ptr, pat) \
+ (strncmp((ptr), (pat), sizeof(pat)-1) ? 0 : \
+ ((ptr) += sizeof(pat)-1, 1))
+
int is_ramdump(char *p)
{
char *x = NULL, *y = NULL, *pat;
size_t len;
char *pattern;
struct stat64 st;
+ int is_raw = 0;
int err = 0;
+ if (PREFIX(p, "raw:"))
+ is_raw = 1;
+
if (nodes || !strchr(p, '@'))
return 0;
@@ -276,6 +284,13 @@ int is_ramdump(char *p)
pat = NULL;
}
+ if (nodes && is_raw) {
+ pc->flags2 |= RAMDUMP;
+ /* also disables get_live_memory_source() logic in fd_init() */
+ pc->dumpfile = "ramdump";
+ pc->readmem = read_ramdump;
+ pc->writemem = NULL;
+ }
return nodes;
}
@@ -345,7 +360,11 @@ show_ramdump_files(void)
{
int i;
- fprintf(fp, "%s [temporary ELF header]\n", elf_default);
+ if (pc->flags & KDUMP)
+ fprintf(fp, "%s [temporary ELF header]\n", elf_default);
+ else
+ fprintf(fp, "%s [raw]\n", pc->dumpfile);
+
for (i = 0; i < nodes; i++) {
fprintf(fp, "%s %s",
i ? "\n" : "", ramdump[i].path);
--
2.5.0
9:49 a.m.
New subject: [PATCH 10/10] teach crash to accept live:MEMORY-IMAGE[@ADDRESS]
Now I can run qemu with the memory-backend-file option and use /usr/bin/crash
in "live" mode.
Signed-off-by: Oleg Nesterov <oleg(a)redhat.com>
---
ramdump.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ramdump.c b/ramdump.c
index e9716ae..5501752 100644
--- a/ramdump.c
+++ b/ramdump.c
@@ -242,11 +242,13 @@ int is_ramdump(char *p)
size_t len;
char *pattern;
struct stat64 st;
- int is_raw = 0;
+ int is_raw = 0, is_live = 0;
int err = 0;
if (PREFIX(p, "raw:"))
is_raw = 1;
+ else if (PREFIX(p, "live:"))
+ is_raw = is_live = 1;
if (nodes || !strchr(p, '@'))
return 0;
@@ -290,6 +292,10 @@ int is_ramdump(char *p)
pc->dumpfile = "ramdump";
pc->readmem = read_ramdump;
pc->writemem = NULL;
+ if (is_live) {
+ pc->flags |= LIVE_SYSTEM;
+ pc->live_memsrc = pc->dumpfile;
+ }
}
return nodes;
}
--
2.5.0
10:57 a.m.
----- Original Message -----
Hi Oleg,
This is going to take some time for me to digest. But without my studying it
at all, there are a couple things that immediately come to mind.
First, a couple of nits about the files being modified.
I see that your overloading the ramdump.c file, but the ramdump facility was
proposed and added by companies that use a firmware-based facility for dumping
raw ARM/ARM64/MIPS RAM into one or more ramdump files, typically because they
do/did not have kdump capability with their embedded hardware. I really don't
feel comfortable conflating that facility with a remote/live QEMU/KVM memory
source. Those companies support it, and so any changes that you've added to
ramdump.c should be accomplished elsewhere.
Also, we don't want to get this confused in any way with the REMOTE_xxx
stuff. The remote.c file and remote access facility was deprecated many
years ago, but recently resurrected in crash-7.0.4 by Verizon specifically
for use like so:
- Resurrection of the remote analysis capability for use with the
"xen-crashd" daemon running on a Xen Dom0 host, which communicates
with a paused or shutdown DomU guest kernel. The daemon can be
accessed like so:
$ crash localhost:5001,/dev/xenmem vmlinux
(dslutz(a)verizon.com
I don't think you are modifying their "resurrected" usage, but just an FYI
since
you refer to it in your patches.
From a crash utility perspective, this QEMU/KVM live memory source is
a completely
new live memory source, and as such should have its own source file
and/or set of
functions, as is the case with the other actual dumpfile types, or live memory
sources /proc/kcore, /dev/mem and /dev/crash.
Now, more to the point...
Anyway, this is a great idea, and one I've pondered in the past, but is there a
concrete reason that it could not be a simple matter of plugging in a new function
into pc->readmem()? Currently there are 20+ pluggable readmem functions that exist
for supporting all of the supported dumpfile types and live memory sources.
Regardless of how it's accomplished, the plug-in basically just needs to be able to
take a physical address and a count (all guaranteed to be contained within a page),
and access/return it. How it accomplishes that task is hidden from the mainstream
crash code.
Thanks,
Dave
11:19 a.m.
New subject: PATCH 00/10] teach crash to work with "live" ramdump
Hi Oleg,
Just to (maybe) clarify my first reply...
As I see it, this facility is simply another LIVE_SYSTEM memory source,
of which there currently are /dev/mem, /proc/kcore and /dev/crash. The
essential difference between them is the pc->readmem plugin:
/dev/mem -> read_dev_mem
/proc/kcore -> read_proc_kcore
/dev/crash -> read_memory_device
The CRASHBUILTIN stuff and related stuff you ran into is only there because
live system analysis typically does not require a crash command line option,
so crash has to figure what to do when a user just enters "crash".
In this QEMU/KVM live system case, it would be much easier to initialize things
based upon the crash command line option that specifies it.
Dave
----- Original Message -----
10:57 a.m.
New subject: PATCH 00/10] teach crash to work with "live" ramdump
Dave, sorry I need to run away. I'll reply tomorrow. And I need to
actually read your emails first, perhaps I misunderstood you...
Just one note,
On 04/25, Dave Anderson wrote:
Well yes, to some degree...
But let me repeat, I believe that crash needs something like 1-7 (and probably
more) anyway. Otherwise it can't work with remote LIVE_SYSTEM correctly.
And while I think that 10/10 can be useful by itself (and least for me), this
is mostly POC which which allows to test/justify these LOCAL_ACTIVE changes
in 1-7.
Oleg.
1:30 p.m.
New subject: PATCH 00/10] teach crash to work with "live" ramdump
----- Original Message -----
But the ACTIVE() macro has been the standard since forever, external extension modules
depend upon it, etc., and so I'd strongly prefer to not change it to LOCAL_ACTIVE().
I'd rather keep the handling of this new facility segregated, maybe create something
like an ACTIVE_QEMU macro/define that can be plugged in wherever you've modified the
ACTIVE() callers where ACTIVE() alone is not enough.
OK good, so you can keep your stuff completely outside of ramdump.c, and not use
is_ramdump(), etc., and then place as many of your changes as possible in a new
file, say something like qemu-live.c. There may be redundancies between the new
file and ramdump.c, but so be it.
7:22 a.m.
On 04/25, Dave Anderson wrote:
but I didn't change the ACTIVE() macro. My point is, it is not always correct to
use it in live/remote case.
Hmm. And this is what I strongly disagree with... Or I misunderstood.
OK. Suppose we add ACTIVE_QEMU() helper. IMO this is a bad idea in any case, the core
code should not even know that this kernel runs under qemu. Nevermind, suppose we have
say
#define ACTIVE_QEMU() ((pc->flags & LIVE_SYSTEM) && (pc->flags2
&& QEMU))
Now what? We need the same 1-7 patches, just LOCAL_ACTIVE() should be replaced
with "ACTIVE() && !QEMU_ACTIVE()".
Let's look at 03/10,
--- a/kernel.c
+++ b/kernel.c
@@ -2900,7 +2900,7 @@ back_trace(struct bt_info *bt)
return;
}
- if (ACTIVE() && !INSTACK(esp, bt)) {
+ if (LOCAL_ACTIVE() && !INSTACK(esp, bt)) {
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
The usage of ACTIVE() is obviously wrong if this is the live (so that ACTIVE()
is true) but remote kernel. We should not even try to look at /proc files on
the local system in this case.
Or perhaps you mean that ACTIVE_QEMU() should be defined as
#define ACTIVE_QEMU() (pc->flags2 & QEMU_LIVE)
? iow, it should not imply ACTIVE() ? This would be even worse, in this case we
would neet to replace almost every ACTIVE() with "ACTIVE() || ACTIVE_QEMU()".
And in fact most of DUMPFILE() users should be updated too. Just for example,
restore_sanity() does
/*
* Clear the structure cache references -- no-ops if DUMPFILE().
*/
clear_task_cache();
clear_machdep_cache();
clear_swap_info_cache();
clear_file_cache();
clear_dentry_cache();
clear_inode_cache();
clear_vma_cache();
clear_active_set();
yes, and every function above will need the fix.
So I strongly disagree, but perhaps I misunderstood you...
If you meant that its name is ugly, or it should not abuse MEMSRC_LOCAL - I won't
argue. But this is minor.
OK, I won't argue. And of course I won't insist if you simply don't like this
new
feauture, I agree it is not that useful.
say something like qemu-live.c.
Again, this has almost nothing to do with qemu in particular, but this doesn't
matter.
OK, lets suppose we add this feature... How do you think the command line should
look?
I mean, after this series we can do, say,
./crash vmlinux raw:DUMP_1@OFFSET_1,DUMP_2@OFFSET_2
if we have 2 ramdump's which should be used together. How do you think the new
syntax should look? I am fine either way.
Oleg.
8:38 a.m.
----- Original Message -----
Correct. ACTIVE() is used ~100 times, and in the vast majority of cases, its use
applies to a live QEMU/KVM session. In the few circumstances that it doesn't, then
ACTIVE_QEMU() should be applied so that it's obvious to the maintainer (me), what
the issue is. Who know what "live" mechanism may come about in the future that
may also have its own quirks? I don't want to hide it, but rather make it strikingly
obvious.
Correct. So restrict it meaningfully (to me anyway).
I agree that there are a handful of circumstances that you have run into where
ACTIVE() may not apply, such as the case where /proc was accessed. But I don't
understand why you say "almost every" instance?
Why? If the target is live, then all of the above should be called as-is. Each
of them returns if the target is a dumpfile.
I just suggested qemu-live.c simply because the facility is for accessing a live
QEMU/KVM instance. Name it whatever you'd like.
I guess I've got some basic misunderstandings here...
If it's a live system, why is necessary to specify RAM offsets?
And if you're just emulating the ramdump facility by first dumping the guest's
memory
into a dumpfile, why isn't it just a ramdump clone?
Dave
10:24 a.m.
On 04/26, Dave Anderson wrote:
to a live QEMU/KVM session, and/or to any other live-and-remote session, please see
below.
Ah, but this is another story.
I mean... OK, as 00/10 says, my vague/distant goal is teach /usr/bin/crash to use
gdb-remote protocol to debug the live guests. And in this case ACTIVE_QEMU() makes
a lot of sense. Say, cmd_bt() can use it to get the registers/trace even if the
process is running, pause/resume the guest, etc.
But all the LOCAL_ACTIVE changes in 1-7 do not care about the details of "live"
mechanism at all. So I still think we need a generic helper which should be true
if local-and-active. Or, vice versa, remote-and-active, this doesn't matter.
So you suggest to change this patch to do
if (ACTIVE() && !ACTIVE_QEMU() && !INSTACK(...))
To me this simply looks worse, but I won't insist. But note that if we ever have
anothe ACTIVE_SOMETHING() source, we will need to modify this code again. While
this code do not care about qemu/something at all. So I still think we need a new
helper which doesn't depend on qemu or whatever else.
Ah, sorry for confusion. I meant, If we add ACTIVE_QEMU() it should imply
ACTIVE(), otherwise we have even more problems.
Why? If the target is live, then all of the above should be called
as-is. Each
of them returns if the target is a dumpfile.
Yes, sure, see above. If ACTIVE_QEMU() plugin sets LIVE_SYSTEM flag too, most users
of ACTIVE() are fine.
I suspect we will need offsets in more complex situations, qemu can have multiple
memory-backend-file/numa options. And perhaps even a single file may need it, not
sure.
And if you're just emulating the ramdump facility by first
dumping the guest's memory
into a dumpfile, why isn't it just a ramdump clone?
Sorry, can't understand... could you spell?
Oleg.
11:11 a.m.
----- Original Message -----
Right, but this is definitely the outlier with respect to "live" systems.
QEMU_LIVE should be in pc->flags, and appear as part of MEMORY_SOURCES. And
LIVE_SYSTEM
should also be set so that the facility falls under both ACTIVE() and ACTIVE_QEMU().
And then in the subset of cases where ACTIVE() is too broad, ACTIVE_QEMU() can be added
as a restriction.
But the above is not relevant with respect to some new extension of the ramdump.
Correct.
But with any live system, crash reads the relevant kernel data structures and sets
up its picture of the system's physical memory accordingly. There's no need to
specify
where the memory lies -- it's all available in the live kernel itself.
On the other hand, typical dumpfile headers give the crash utility instructions
on how to randomly access physical memory in the dump, i.e., like the PT_LOAD
segments in an ELF vmcore. Ramdumps don't have any header information, so the
physical memory blocks have to be specified on the crash command line -- and then
crash creates a temporary in-memory ELF header for subsequent memory reads.
(or the user can specify "-o dumpfile" to transform the ramdump into a kdump
clone.
I'm not sure because that's what I don't understand. You seem to be
describing two completely
different facilities:
(1) a live access facility like /dev/mem, et al, but to a live KVM guest
(2) some kind of ramdump facility?
And if it's a ramdump facility, couldn't you just copy it from the guest to the
host
and analyze it there?
Dave
12:08 p.m.
On 04/26, Dave Anderson wrote:
...
Can't understand...
OK. But not in the case above? This particular /proc/$tc->pid doesn't need to
know if this (remote) live system is ACTIVE_QEMU() or ACTIVE_SOMETHING_ELSE() ?
All it needs to know if we debug the (live) kernel on the same machine or not.
So why we can't have a generic macro which can be used in this code and other
paths which do not care?
But the above is not relevant with respect to some new extension of
the ramdump.
Exactly. Another reason to not add the ACTIVE_QEMU() helper right now. The fact
that this remote-and-live system runs under qemu doesn't matter at all. We use
ramdump because /tmp/MEM is actually the dump of the physical memory, and if
the guest doesn't crash it is as "live" as /dev/crash is "live".
I guess I seriously misunderstand you.
Yes, yes, and again /tmp/MEM I used in the previous discussion _is_ the live
memory of the guest. And in this particular case the mapping is trivial, so we
do not need the offsets.
There's no need to specify
where the memory lies -- it's all available in the live kernel itself.
...
Ramdumps don't have any header information, so the
physical memory blocks have to be specified on the crash command line
Yes, and that is why we may need OFFSET (and probably LENGTH too) in more complex
situations with memory-backend-file, but again I am not sure.
I tried to explain this in the previous email, not sure I was clear...
Oleg.
12:57 p.m.
----- Original Message -----
I mean that it's a special case of a "live" system. For the most part it
is, but
there are a number of gotcha's that would need to be caught and addressed.
I think we're still talking about the code segment in back_trace()? All that
is doing is recognizing that the "current" task no longer exists, and the
backtrace
attempt should fail. Then there's some other code that runs before the next
"crash> "
prompt that resets it to the live "crash" task that's running. In the live
QEMU
session, that will have to handled differently in both places. I don't know how
you're going to easily determine whether task still exists.
Dave
1:18 p.m.
n 04/26, Dave Anderson wrote:
Yes, but (in this particular case) it is only "special" because it is remote.
Nothing
else.
And again, again. My initial plan was
- set
pc->flags |= (LIVE_SYSTEM | REM_LIVE_SYSTEM);
in 10/10
- use !REMOTE_ACTIVE() in the code above and other patches in 1-7.
But then I noticed that all this REM_ code is mostly broken. So I added the
new LOCAL_ACTIVE() helper, so these changes do not depend on REMOTE logic.
Oh, I see. I didn't try to say this change is very important. Still this looks
just wrong.
I don't know how
you're going to easily determine whether task still exists.
I am not going to do this. Heh, I just noticed the patch is wrong! :/ Probably
this confused you, sorry.
if (LOCAL_ACTIVE() && !INSTACK(esp, bt)) {
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
else
error(INFO,
"invalid/stale stack pointer for this task: %lx\n",
esp);
return;
}
of course, this is not what I actually tried to do. I meant something like
if (ACTIVE() && !INSTACK(esp, bt)) {
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!LOCAL_ACTIVE() || !file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
else
error(INFO,
"invalid/stale stack pointer for this task: %lx\n",
esp);
return;
}
Damn ;)
Oleg.
7:22 a.m.
On 04/25, Dave Anderson wrote:
As I see it, this facility is simply another LIVE_SYSTEM memory source,
of which there currently are /dev/mem, /proc/kcore and /dev/crash.
Yes, if we are talking about 09/10 and 10/10.
The
essential difference between them is the pc->readmem plugin:
The essential difference is that this source is remote. See another email.
Ah. Yes, the usage of CRASHBUILTIN is ugly, and I tried to document this.
We need the new (say) RAW_MEM_DUMP flag. We can't use RAMDUMP because it can
be set along with KDUMP.
And there is no room in pc->flags, so I decided to abuse CRASHBUILTIN for now.
Oleg.
8:14 a.m.
----- Original Message -----
I understand.
It's exceeding rare that a new dumpfile type, or a new live system accessor in
this case, gets introduced. When that happens, we can move deprecated leftovers
from pc->flags to pc->flags2. So for example, all of these can be moved to
pc->flags2 in one fell swoop:
REM_NETDUMP
REM_MCLXCD
REM_LKCD
REM_S390D
Checking everywhere they get used, the (useless) settings in remote.c can be
changed, and the very few macros that reference them can be changed to get
them from pc->flags2.
Dave
9:26 a.m.
On 04/26, Dave Anderson wrote:
Yes, yes, sure. But while this is trivial, this needs some tiresome changes.
and that is what this part of changelog
The usage of CRASHBUILTIN doesn't look nice, we need to cleanup
this logic. I hope we can do this later, and it seems to me that
the usage of MEMORY_SOURCES/DUMPFILE_TYPES needs some cleanups in
any case.
in 9/10 tried to say ;)
Say, memory_page_size(). It does "switch (pc->flags & MEMORY_SOURCES)"
and it
needs the update if we move (say) NETDUMP in pc->flags2. Trivial, but needs
another patch/discussion/etc.
Another problem is that there is no simple/clear way to tell fd_init() paths that
we do not need get_live_memory_source/memory_source_init. Unless we set
REMOTE() == T but we should not do this.
So, if REMOTE() is false, fd_init() calls get_live_memory_source() if
pc->dumpfile is NULL. This is not what RAMDUMP need, so 09/10 has to initialize
pc->dumpfile. At the same time memory_source_init() assumes that if
pc->dumpfile must at least exist if it is non-NULL. Perhaps this needs needs
some cleanups too, but this is off-topic right now.
Heh ;). and I think fd_init() is simply wrong. The problem is minor and off-topic
too, I'll report it later (probably with simple fixes tomorrow). But in short,
you can't use /dev/crash unless you are root, and if you root and /dev/crash
is modular then /dev/crash will be removed and the module will be unloaded when
the crash exits, even if it was not loaded/created by crash. Although I need
to verify this, I can be wrong.
Oleg.
9:57 a.m.
----- Original Message -----
One-time tiresome changes. I should probably do it anyway. They are a
waste of pc->flags space.
CRASHBUILTIN is used to indicate the the Red Hat /dev/crash driver exists and
the kernel module was built into the kernel -- as opposed to having to load the
crash.ko driver. I'm not sure how that is associated with this facility.
Who said to move NETDUMP? I only suggested the REM_NETDUMP (and the other REM_xxx)
dumpfiles.
Right, presumably there would need to be a separate "if (QEMU_ACTIVE())" section
in memory_source_init().
Correct. When it's modular (and it isn't any more), the module is unloaded and
/dev/crash is purposely removed. It works as intended.
Dave
10:42 a.m.
On 04/26, Dave Anderson wrote:
Yes, yes, I see how it is used now. Damn, but now I also see that my changelog
looks very confusing!
"The usage of CRASHBUILTIN doesn't look nice" above means "The usage of
CRASHBUILTIN
IN THIS PATCH doesn't look nice", and I even added the FIXME comment. Sorry for
confusion Dave.
Ah, yes.
Well, I still disagree, see the previous email... I still think we need some
generic macro.
Even if this module was loaded and /dev/crash existed before I start /bin/crash?
# ll /dev/crash
cr--r--r--. 1 root root 10, 57 Apr 26 12:33 /dev/crash
# crash ../VMLINUX
...
WARNING: ../VMLINUX and /proc/version do not match!
(just in case, this is correct)
# ll /dev/crash
ls: cannot access /dev/crash: No such file or directory
doesn't look friendly.
And I can't use /bin/crash without root even if I do "chmod a+r /dev/crash"
on my
machine.
Is it all intentional?
Oleg.
11:19 a.m.
----- Original Message -----
Yes. The whole /dev/crash driver bullshit was put in place because of
CONFIG_STRICT_DEVMEM.
So we pretty much had to work around it by creating the read-only /dev/crash driver.
Anyway, in the original discussions, the security folks didn't want /dev/crash
hanging
around unless it was actively being used by a root-only live crash session. Later on it
was decided that the driver should be built into the kernel.
Please, just leave it be... ;-)
Dave
12:25 p.m.
On 04/26, Dave Anderson wrote:
...
> # crash ../VMLINUX
...
I understand the problems with /dev/crash. But I don't think /bin/crash should remove
the file created by me.
Please, just leave it be... ;-)
OK.
But why /bin/crash REMOVES /dev/crash even if this file was NOT created at startup time?
Why get_live_memory_source() doesn't even check file_exists("/dev/crash")
if
/lib/modules/%s/kernel/drivers/char/crash.ko exists?
Why I can't use /bin/crash on MY machine as non-root user even if I have the
permission
to read /dev/crash?
OK, I'll probably keep the patch below for myself. It doesn't really fix the
problems,
but it allows me to use "crash /dev/crash" on my machine without root.
Oleg.
--- x/filesys.c
+++ x/filesys.c
@@ -3859,8 +3859,7 @@ create_memory_device(dev_t dev)
/*
* It already exists -- just use it.
*/
- if ((stat.st_mode == MEMORY_DRIVER_DEVICE_MODE) &&
- (stat.st_rdev == dev))
+ if (S_ISCHR(stat.st_mode) && (stat.st_rdev == dev))
return TRUE;
/*
7:22 a.m.
On 04/25, Dave Anderson wrote:
It's a pity ;) But this is minor.
But let me also say that there is nothing qemu-specific in this series. Except
it happens to work with the recent memory-backend-file qemu option.
I mean, you can just do
$ dd if=/dev/crash of=RAMDUMP count=...
and then
$ crash path-to-your-vmlinux raw:RAMDUMP@0
or even just
$ crash path-to-your-vmlinux live:/dev/crash@0
and this should work. Not that I think this makes sense, but should work.
9/10 and 10/10 just add the "raw:" and "live:" options. The 1st one
means
that crash should avoid avoid ramdump_to_elf()/read_kdump() and just use
read_ramdump() to access this RAM dump. The 2nd one naturally tells that
this memory is alive.
Those companies support it, and so any changes that you've added
to
ramdump.c should be accomplished elsewhere.
OK, we can add the new (say) --rawdump/livedump options and avoid any
changes in ramdump.c. Except probably 08/10 which can help to avoid
the code duplicating. But again, this is minor.
Also, we don't want to get this confused in any way with the
REMOTE_xxx
stuff.
Yes. And this series tries to not mess with REMOTE logic. And this is why
I didn't try to define LOCAL_ACTIVE() as (say) !REMOTE_ACTIVE().
At the same time, it tries to re-introduce the ability to debug the remote
live kernels. And the QEMU/KVM memory source is just the example.
And that is why I think we need 1-7. More on this in another email.
The remote.c file and remote access facility was deprecated many
years ago, but recently resurrected in crash-7.0.4 by Verizon specifically
for use like so:
OK, I see, thanks.
Simply because read_ramdump() is already implemented and it does exactly we need.
And, say, "pc->flags2 |= RAMDUMP" makes sense too, display_sys_stats() will
use
show_ramdump_files().
Sure, it would be simple to write another plugin. The main problem is that there
is no room in pc->flags and that is why this series abuses CRASHBUILTIN ;)
Oleg.
8:01 a.m.
----- Original Message -----
I'm getting somewhat confused now. I was under the impression that this QEMU/KVM
concept for live systems only. If you create a ramdump file that fits within the
constraints of the ramdump.c functionality -- essentially creating a "ramdump
clone",
then yes, I guess I can understand why you would want to use ramdump.c. (as long
as it doesn't break compatibility with the aims of the embedded folks who wrote it
and use it).
If we're *just* talking about supporting live system access of a QEMU/KVM guest,
then it should have nothing to do with ramdump.c. Even if there is duplication
of effort, please keep any initialization functions, its pc->readmem plugin,
and so on in its own separate file.
That's not a problem -- see subsequent email.
Dave
9 a.m.
On 04/26, Dave Anderson wrote:
No,
Yes. And to remind, MEMORY-IMAGE@ADDRESS (which is even documented in crash.8 as
"raw RAM dumpfile that has no header information") doesn't work on x86-64,
because
ramdump_to_elf() fails with "unsupported machine type".
So imo 08/10 (which is the trivial preparation) and 09/10 make sense anyway, this
doesn't need the previous LOCAL_ACTIVE patches.
Yes, let me repeat that 09/10 asks for cleanup (the change in main.c). This is
because I fail to understand this "if (is_ramdump())" block in main(), it looks
simply wrong to me, but this is off-topic right now.
And can refactor these changes so that ramdump.c won't be changed at all, we can
move these simple changes to the caller.
(as long
as it doesn't break compatibility with the aims of the embedded folks who wrote it
and use it).
It shouldn't. As long they do not try to use a RAMDUMP file which name starts with
"raw:" or "live:" prefix.
If we're *just* talking about supporting live system access of a
QEMU/KVM guest,
then it should have nothing to do with ramdump.c.
See above. But as for "live" ramdump's I do not see another use-case except
qemu
running with memory-backend-file.
But note that this feature comes in 10/10 which is essentially on-liner! We only
need to set "pc->flags |= LIVE_SYSTEM", but this change depends on the
previous
LOCAL_ACTIVE changes. Because again, currently crash can not work correctly with
remote live kernels.
Oleg.
9:32 a.m.
----- Original Message -----
Correct. It's only supported the architectures listed because they are the only
architectures used by the companies who wrote and support the facility. If somebody
comes along and makes it work with x86_64, it can easily be added. Nobody has.
Right, it is confusing. There are two ways to utilize their ramdump files.
You can take the ramdump file and either:
(1) use the -o option to create a new vmcore containing an ELF header prepended to the
ramdump file, and then access it like the kdump clone that it is. At that point,
the original ramdump file can be deleted.
(2) create a temporary in-memory ELF header for accessing the ramdump during the
crash session.
Sorry Oleg, but I'm afraid that I seem to be regressing...
I don't understand the difference between what you call a "raw" and a
"live" ramdump?
Let's get back to basics. Is is possible for the crash utility to simply issue
readmem
requests for physical memory to whatever magic you've got running on the live guest,
and have it satisfy the request? So that it would be essentially the same thing as
reading from /dev/mem, /proc/kcore or /dev/crash?
Or is there always going to have to be an actual dumpfile somewhere?
Dave
11:30 a.m.
On 04/26, Dave Anderson wrote:
I see. What I can't understand is why (2) sets KDUMP but switches to
pc->readmem = read_ramdump() instead of read_kdump(), even if it creates the same
elf vmcore.
Me too ;)
OK. So I am running the rhel7 guest on my Fedora machine and I pass the following
(additional) options to qemu:
-object memory-backend-file,id=MEM,size=128m,mem-path=/tmp/MEM,share=on \
-numa node,memdev=MEM \
so in this (trivial) case /tmp/MEM represents the physical memory as it seen by
the guest.
Now suppose that this guest crashes and qemu exits. In this case the "live"
mode
makes no sense, if nothing else it is slower.
So I can do
$ crash path-to-rhel7-vmlinux raw:/tmp/MEM@0
on the host, and it works. Sure, not that useful. I could do dump-guest-memory
from qemu before exiting and use the generated kvm dump.
"live" ramdump is a bit more interesting. I can do
$ crash path-to-rhel7-vmlinux live:/tmp/MEM@0
and debug the live guest running under qemu, when it is not going to stop/crash.
Just like I can use /bin/crash to debug the host kernel using /dev/crash, but
this guest is obviously "remote", so we need those LOCAL_ACTIVE changes in 1-7
to make it work.
Oleg.
12:38 p.m.
----- Original Message -----
I didn't write this code, but here's how I understand it.
(1) If the crash command line contains a memory@address reference, then it's
describing
a ramdump, and it calls ramdump_to_elf() to create an ELF header.
(2) If "-o dumpfile" had also been specified, then it creates a new ELF vmcore
file.
(3) If "-o" was not used, then it creates a temporary in-memory ELF header.
(4) It then calls is_kdump() to verify that the ELF header was created correctly, and
sets KDUMP because it needs to set at least one of the bits in MEMORY_SOURCES.
(5) If it's just using the temporary ELF header, it uses read_ramdump().
(6) If it created a new ELF kdump clone vmcore, it uses read_kdump().
As far as (5) and (6) go, certainly read_ramdump() is far more efficient than
using read_kdump() in the temporary ELF header case. As for using read_kdump()
with the newly-created kdump clone, perhaps they wanted to make absolutely
sure that it would work in the future if/when they re-use the vmcore as if it
were a real kdump vmcore, i.e., entering "crash vmlinux vmcore"? Or it could
be because the "-o clone" option was added after the initial ramdump supporting
only the temporary header was put in place? Honestly, I'm not sure.
Exactly what is this /tmp/MEM that you speak of?
I don't understand. Does the /tmp/MEM file still exist somewhere after the guest
crashed?
Again, I am clueless as to what /tmp/MEM consists of on the guest. How does it
get there to begin with? Is is a pseudo-file that constantly contains the
current contents of the guest's physical memory? Is it like /dev/mem?
Dave
12:59 p.m.
On 04/26, Dave Anderson wrote:
I didn't write this code, but here's how I understand it.
Thanks Dave, I'll read your explanation tomorrow, I need to run away again.
please note the "mem-path=/tmp/MEM" in the memory-backend-file arg above.
With this option qemu doesn't use the anonymous/private mapping for the guest's
physical memory it creates a file (specified by mem-path=). The host can read
(and write of course) to this file. This file _is_ the guest's physical memory.
Just in case, you pass multiple memory-backend-file/numa arguments, so you will
have the "multi-file" ramdump.
Yes,
See above,
Is is a pseudo-file
No, just a regular file, qemu creates it and does mmap(MAP_SHARED) on it.
that constantly contains the
current contents of the guest's physical memory?
Yes,
Is it like /dev/mem?
yes, but more like /dev/crash.
Oleg.
1:27 p.m.
On 04/26, Dave Anderson wrote:
I too do not know much about qemu options,
So if I were to log into the guest machine, does a /tmp/MEM file
exist?
No,
Or does it exist on the host machine?
Yes, it is just the normal file on the host which runs qemu.
Well, "normal" is not neccessarily true in that you can use, for example,
mem-path=/path/to/hugetlb-mount/...
but this doesn't matter. It is still the "regular" file mmaped by qemu, the
host can read it to acess the guest's physical memory.
Oleg.
7:49 a.m.
On 04/26, Dave Anderson wrote:
Yes, and this file is the physical memory of the guest. So it is essentially
the RAM dump which can be used by "crash PATH-TO-THIS-FILE@0" right now without
any patches. And in this particular case the offset is always zero.
But not on x86-64, is_ramdump() insists on ramdump_to_elf() even if we could
use read_ramdump(), and ramdump_to_elf() doesn't support x86-64.
And of course, you can't use this RAM dump in "live" mode (without these
changes).
So what's the remote access needed
for -- just to query for the particulars of the layout of the memory file?
Sorry, I don't understand...
Do you ask me what qemu does with this file or what?
Oleg.
8:08 a.m.
----- Original Message -----
Right, but that's a trivial fix, right? As I mentiond before, the only reason
it doesn't support is because nobody's tried/asked/needed-to. And with x86_64
support put into place, at least the "non-live" file should be a considered
a ramdump, right?
And of course, you can't use this RAM dump in "live"
mode (without these
changes).
Right -- that's the rub here. The "live version of a dumpfile" is a hybrid
that's
never been considered before.
This is where I totally got off on the wrong foot. I thought you had mentioned
something about remote gdb protocol, or something along those lines where there
was some remote communication protocal? I'm sorry about that, I really went off
into the weeds...
Dave
9:43 a.m.
On 04/27, Dave Anderson wrote:
Probably yes, I simply do not know. I know nothing about elf magic.
And with x86_64
support put into place, at least the "non-live" file should be a considered
a ramdump, right?
Sure. But again, we do not even need to update ramdump_to_elf() and create
the elf header, read_ramdump() can work just fine. This is what 09/10 does.
Yes, so 10/10 is just a one-liner. But it depends on 1-7. Say, without 02/10
memory_source_init() fails. Without 04/10 /bin/crash segfaults.
Heh ;) I tried to say that this is my very distant goal. Unlikely I will even try
to do this in the foreseeable future. I think it would be nice to implement, but
obviously this need more work than just 30 lines of code added by this series.
Sorry for confusion!
But after this series we can already debug the live guest using /bin/crash, just
you need to pass the additional arguments to qemu.
Oleg.
10:24 a.m.
----- Original Message -----
It should simply be a matter of setting e_machine to EM_X86_64 in ramdump_to_elf(),
and letting alloc_elf_header() do the rest.
Right, I understand. But it would be preferable if "-o dumpfile" could still be
used
for use with the "non-live" file.
Right, but there should be no need for the "raw" distinction given that the
"non-live" dumpfile is really just a "regular" ramdump, for lack of a
better
term (and with x86_64 support added).
And yes, the 1-7 qualifiers (and probably a few others) are always going to be
necessary for the "live-dump-hybrid".
So, getting back to our original discussions, the handling of this hybrid-live
dumpfile is the main issue. I don't like the re-use of unrelated definitions
like MEMSRC_LOCAL, which was used back in the remote-access days if the vmlinux
file was available on a remote machine but the dumpfile had been copied to the
host machine running crash. But I still don't know what to call it so that it
makes sense. And BTW, give that the live-dump-hybrid will still require a new
dumpfile-type #define that can be plugged into the pc->flags MEMORY_SOURCES bitmask,
I see now that the MEMSRC_LOCAL would be a great candidate to move from pc->flags
to pc->flags2.
Dave
8:46 a.m.
sorry for delay Dave,
On 04/27, Dave Anderson wrote:
Yes, but afaics elf header buys nothing in this case, so it is not clear why do
we need it if we can just use read_ramdump().
OK. Given that I confused you many times, can't we forget this for the moment
and try to make the necessary changes step-by-step? I mean, lets discuss the
LOCAL_ACTIVE() patches first, then return to RAM dumps.
...
OK, agreed.
And yes, the 1-7 qualifiers (and probably a few others) are always
going to be
necessary for the "live-dump-hybrid".
OK, good. So can't you apply 1-7 first? so that we can finish this part and then
add the support for live dumpfiles.
3/10 was buggy, I'll send v2 in a minute.
What else do you think I should change in this series?
Do you agree with 1/10? I mean do you agree with the name of new LOCAL_ACTIVE()
helper and its semantics?
OK, good, so lets use MEMSRC_LOCAL at least for now. It should be trivial to change
it later because only fd_init() uses this flag directly. Ignoring REMOTE() code, but
they should not conflict.
And BTW, give that the live-dump-hybrid will still require a new
dumpfile-type #define that can be plugged into the pc->flags MEMORY_SOURCES bitmask,
Yes, yes. And then we do not need to abuse CRASHBUITIN. But lets discuss this later.
In short: what do you want me to change in 1-7 to get them applied?
Oleg.
11:24 a.m.
----- Original Message -----
Well, for the non-live, crashed. version of this dumpfile, it should look exactly
as the current ramdump MEMORY-IMAGE@ADDRESS implementation, correct?
As for the "hybrid-live-dump" version, I'm not sure. So for now I guess you
can
continue using the "live:" prefix to the dumpfile name. If we come up with a
more logical naming scheme in the future, we can always change it later.
2. Should I re-use ramdump.c or should I just add the new file which
re-implements read_ramdump() ?
Given that these *are* essentially ramdump files, you've convinced me that ramdump.c
should be used.
When I was babbling on about creating a new file, I was still under the impression
that there was some kind of remote protocol going on. If I had been aware of exactly
what your "/tmp/MEM" file consisted of, and that it exists on the host machine,
I could
have avoided 80% of our back-and-forth emails. I'm really sorry for having wasted
your time.
Dave
3240
days inactive
3243
days old
devel@lists.crash-utility.osci.io
54 comments
2 participants
participants (2)
-
Dave Anderson -
Oleg Nesterov