Re: handling missing kdump pages in diskdump format

Hi Dave,

2007/04/03 09:40:56 -0400, Dave Anderson <andersonredhat.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

--
Crash-utility mailing list
Crash-utilityredhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Ken'ichi Ohmichi wrote:

> Hi Dave,
>
> 2007/04/03 09:40:56 -0400, Dave Anderson
<andersonredhat.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




--
Crash-utility mailing list
Crash-utilityredhat.com
https://www.redhat.com/mailman/listinfo/crash-utility