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

[Dave Anderson]

----- Original Message -----
> Hi Dave,
> 
> Following the mails quiet late, and it looks like lot of discussion has gone
> in between you and Oleg.
> right now ramdump.c just addresses ARM since we work on ARM arch.
> 
> 
> if we have to support x86/_64, headers need to be dealt differently.
> 
> when I had written code (before ramdump was submitted), I just wrote it with ARM in mind,
> but I think, there is a design change/tiny framework where arch specific
> implementation could hook their functions such as alloc_elf_headers.
> 
> let me know what you think.

Hi Oza,

Note that Rabin Vincent added support for 32-bit MIPS in crash-7.1.1.  And given
that the ELF format is 64-bit LSB, the only difference would be the ehdr->e_machine
field.  So I believe that setting e_machine to EM_X86_64 in ramdump_to_elf() should
just work.  If it doesn't work, the vmcore will be rejected during initialization.

Dave



> 
> Regards,
> Oza.
> 
> 
> 
> 
> On Saturday, 30 April 2016, 20:39, paawan oza <paawan1982 at yahoo.com> wrote:
> 
> 
> Hi Oleg,
> 
> Just was curious, what kind of addition your patch is bringing ?
> Broadcom brought the support for raw ramdump, well you know justpreparing ELF
> header and sparse support.
> 
> so just wanted to understand how what you patch does, it addresses live dump
> ? could you elaborate on it ?
> 
> Regards,
> Oza.
> 
> 
> 
> 
> On Saturday, 30 April 2016, 1:27, Dave Anderson <anderson at redhat.com> wrote:
> 
> 
> 
> 
> ----- Original Message -----
> > Hi Dave,
> > 
> > please consider V2, I tried to address your comments.
> > 
> > Oleg.
> 
> 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
> 
> 
> 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
> 
> 
> 
> 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility