Re: [Crash-utility] PATCH v2 00/10] teach crash to work with "live" ramdump

----- Original Message ----- Hi Oleg, This looks pretty good, but I do have a couple of questions/comments. In [PATCH v2 02/10]: @@ -124,6 +124,7 @@ fd_init(void) if (!pc->dumpfile) { pc->flags |= LIVE_SYSTEM; + pc->flags2 |= MEMSRC_LOCAL; get_live_memory_source(); } Now that MEMSRC_LOCAL has been effectively moved out of the way, why is it being set above in fd_init()? And in [PATCH v2 10/10]: @@ -428,6 +428,17 @@ main(int argc, char **argv) "too many dumpfile arguments\n"); program_usage(SHORT_FORM); } + + if (ACTIVE()) { + pc->flags |= LIVEDUMP; + /* disable get_live_memory_source() logic in fd_init() */ + pc->dumpfile = "livedump"; + pc->readmem = read_ramdump; + pc->writemem = NULL; + optind++; + continue; + } The pc->dumpfile should point to "/tmp/MEM" or however it was invoked on the command line, i.e., just like any other dumpfile, regardless of whether it is live or not. And if pc->dumpfile were changed to be the actual filename, then the following qualifier shouldn't be necessary: @@ -209,7 +209,8 @@ memory_source_init(void) } if (pc->dumpfile) { - if (!file_exists(pc->dumpfile, NULL)) + if (!(pc->flags & LIVEDUMP) && + !file_exists(pc->dumpfile, NULL)) error(FATAL, "%s: %s\n", pc->dumpfile, strerror(ENOENT)); And this change in [PATCH vs 9/10]: @@ -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; causes this compiler warning if "make warn" is used: $ make warn ... [ cut ] ... cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 ramdump.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security ramdump.c: In function 'ramdump_to_elf': ramdump.c:230:1: warning: label ‘end’ defined but not used [-Wunused-label] end: ^ ... We'll also need to vet every instance of ACTIVE() to make sure there are no other dependencies on the local system. For example, this one comes to mind, where x86_64_calc_phys_base() looks at /proc/iomem: if (ACTIVE()) { if ((iomem = fopen("/proc/iomem", "r")) == NULL) return; ... And if you bring up a cscope session, and search for the "/proc/" strings, there may be other local /proc filesystem references that aren't obviously inside of "if (ACTIVE())". I did check some of them, and most would only be relevant/called if it's a local active instance, but some may not. But anyway, this looks good for starters. Talk to you next week, Dave