[Crash-utility] handling missing kdump pages in diskdump format

[Dave Anderson]

Ken'ichi Ohmichi wrote:

> Hi Dave,
>
> 2007/04/03 09:40:56 -0400, Dave Anderson <anderson at redhat.com> wrote:
> >> It is a good idea that "help -n" shows dump_level.
> >>
> >> I checked the code of diskdumputils-1.3.25, and I found the dumpfilter
> >> command refers to diskdump->total_ram_blocks. In struct disk_dump_header,
> >> there is no member not referred to by neither the crash utility or diskdumputils.
> >> My proposal (changing total_ram_blocks for dump_level) was not good.
> >>
> >> I think it is enough that the member for dump_level is added into the
> >> sub header of kdump (struct kdump_sub_header) only for kdump.
> >> Is the change for diskdump necessary, too ?
> >>
> >>
> >
> >If this were to be restricted to kdump only, then it would be
> >safe to add a field to the kdump_sub_header since it contains
> >only one field.
> >
> >Whether it is necessary for diskdump is up to the diskdump
> >maintainers to decide.  But if they want to do it, they would
> >have to come up with different manner of adding it to the
> >disk_dump_sub_header structure, because of the way
> >that it's declared, i.e., using "long elf_regs" as a placeholder
> >to describe the start of the processor-dependent register
> >set.  They'd need to define a new disk_dump_sub_header
> >version that alternatively would be used depending upon
> >the header->version number.
>
> We asked the diskdump developers (Indoh-san, Akiyama-san) how
> to display diskdump's dump_level, and they said that changing
> of file format is not necessary for displaying the dump_level.
> And they gave us a good advice that the crash utility can get
> diskdump's dump_level by using a symbol "dump_level" like the
> following:
>
> int get_dump_level(void)
> {
>         if (symbol_exists("dump_level")) {
>                 int dump_level;
>                 readmem(symbol_value("dump_level"), KVADDR, &dump_level,
>                         sizeof(dump_level), "dump_level",
>                         FAULT_ON_ERROR);
>                 return dump_level;
>
>         }
>         else
>         ...
>
> I think it is a good way because the crash utility can get diskdump's
> dump_level of the existing dumpfile. I propose the following way for it.
> Please let me know your opinion.
>
> - If a symbol "dump_level" is present (diskdump), the dump_level is
>   taken from a symbol "dump_level".
> - Else if a dumpfile is kdump's and header_version is 1 or more, the
>   dump_level taken from a new member "kdump_sub_header->dump_level".
>
> Thanks
> Ken'ichi Ohmichi

Both proposals sound good to me.

Just so that I can have it ready-to-go for you in the next crash release,
do you want to make the additional "dump_level" member an int or
a long?

Dave