----- 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@xxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
>
>
>
>
>
> --
> Crash-utility mailing list
> Crash-utility@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
