Re: [Crash-utility] [PATCH 1/2] diskdump: Fail readmem() early if dump is incomplete
-----Original Message-----
On Wed, Jun 09, 2021 at 12:02:16AM +0000, HAGIO KAZUHITO(萩尾 一仁)
wrote:
> -----Original Message-----
> > On Tue, Jun 08, 2021 at 02:40:39AM +0000, HAGIO KAZUHITO(萩尾 一仁) wrote:
> > > > Does it still make sense to check the incomplete flag for the
> > > > zero_excluded case? Otherwise, the above code comment will be
> > > > outdated.
> > > > + } else if (0 == pd.offset) {
> > > > ...
> > > > - if (*diskdump_flags & ZERO_EXCLUDED) {
> > > > + if (is_incomplete_dump() && (*diskdump_flags
& ZERO_EXCLUDED)) {
> > >
> > > Given the fact that makedumpfile cannot mark a dump as incomplete every
time
> > > it fails, I think it might be good to be able to use zero_excluded option
> > > also without the incomplete flag.
> > >
> >
> > Hi Kazu,
> >
> > Yes, that was my thought too.
> >
> > > So I'm ok with the original change, but additionally would like to
> > > - fix the comment Lianbo pointed out and help texts of zero_excluded
> > > - change netdump.c as well
> > >
> >
> > I'll correct the comment. What would you want to change in netdump.c?
>
> I thought that if we change diskdump.c to be able to use zero_excluded
> without the incomplete flag then it would be better to change netdump.c
> the same, i.e. remove this is_incomplete_dump() and fix the comment in
> read_netdump() too:
> /*
> * If the incomplete flag has been set in the header,
> * first check whether zero_excluded has been set.
> */
> if (is_incomplete_dump() && (read_ret >= 0)
&&
> (*diskdump_flags & ZERO_EXCLUDED)) {
>
Thanks for pointing it out, Kazu.
I never worked with netdump and initially thought whether it somehow
related to processing of dumps over network but after inspecting the
code it seems like it processes regular files. Are the files written by
netdump-server?
I've never used the netdump-server actually, but my understanding is that
read_netdump() is also used for ELF kdump format, e.g. /proc/vmcore and
a dumpfile captured by makedumpfile -E option, via read_kdump().
I'm not sure if I have enough context and fully understand interaction
between netdump.c and netdump-server. I'll be confident to do the change
in v2 if you are sure it would work exactly as in the case of
diskdump.c/makedumpfile.
Only makedumpfile sets DUMP_ELF_INCOMPLETE flag to ELF dumpfile, but
there might be cases that it cannot set the flag like compressed dumpfile,
so removing the is_incomplete_dump() might be helpful.
Also, there is no behavior change without zero_excluded, and who uses
zero_excluded should know what they are doing. I think it's no problem.
Thanks,
Kazu
I tested my changes on a real incomplete core produced by makedumpfile
and I don't have an incomplete netdump core :)
Regards,
Roman