[Crash-utility] [PATCH 1/2] diskdump: Fail readmem() early if dump is incomplete

[HAGIO KAZUHITO(萩尾 一仁)]

-----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