Re: [PATCH / RFC] makedumpfile header to show LZO/snappy/zlib

[Dave Anderson <anderson@redhat.com>]

----- Original Message -----
> On Fri, 20 Sep 2013 09:04:40 -0400 (EDT)
> Dave Anderson <anderson@redhat.com> wrote:
> 
> > 
> > 
> > ----- Original Message -----
> > > Hello Dave,
> > > 
> > > (2013/09/16 0:56), Dave Anderson wrote:
> > > >
> > > > With the advent of LZO and snappy compression support, it would be
> > > > helpful if it could be readily determined which type of compression
> > > > was used to create the dumpfile.
> > > >
> > > > The zlib/LZO/snappy indicator is only found down in the bowels of each
> > > > page_desc structure:
> > > >
> > > >    /* page flags */
> > > >    #define DUMP_DH_COMPRESSED_ZLIB 0x1     /* page is compressed with zlib
> > > >    */
> > > >    #define DUMP_DH_COMPRESSED_LZO  0x2     /* paged is compressed with lzo
> > > >    */
> > > >    #define DUMP_DH_COMPRESSED_SNAPPY       0x4
> > > >                                            /* paged is compressed with snappy */
> > > >
> > > >    /* descriptor of each page for vmcore */
> > > >    typedef struct page_desc {
> > > >            off_t                   offset;         /* the offset of the page data */
> > > >            unsigned int            size;           /* the size of this dump page */
> > > >            unsigned int            flags;          /* flags */
> > > >            unsigned long long      page_flags;     /* page flags */
> > > >    } page_desc_t;
> > > >
> > > > The compressed kdump's header does contain a "status" flag:
> > > >
> > > >    struct disk_dump_header {
> > > >            char                    signature[SIG_LEN];     /* = "KDUMP   " */
> > > >            int                     header_version; /* Dump header version  */
> > > >            struct new_utsname      utsname;        /* copy of system_utsname */
> > > >            struct timeval          timestamp;      /* Time stamp */
> > > >            unsigned int            status;         /* Above flags */
> > > >    ...
> > > >
> > > > which has these 3 flags:
> > > >
> > > >    #define DUMP_HEADER_COMPLETED   0
> > > >    #define DUMP_HEADER_INCOMPLETED 1
> > > >    #define DUMP_HEADER_COMPRESSED  8
> > > >
> > > > But they are all remnants of the old "diskdump" facility, and none of them
> > > > are ever used by makedumpfile.
> > > >
> > > > Any chance that the LZO/snappy/zlib method could also be put up in
> > > > the disk_dump_header?
> > > >
> > > > Dave
> > > >
> > > > --- makedumpfile-1.5.4/makedumpfile.c.orig
> > > > +++ makedumpfile-1.5.4/makedumpfile.c
> > > > @@ -5162,6 +5162,16 @@ write_kdump_header(void)
> > > >      dh->bitmap_blocks  = divideup(info->len_bitmap, dh->block_size);
> > > >      memcpy(&dh->timestamp, &info->timestamp, sizeof(dh->timestamp));
> > > >      memcpy(&dh->utsname, &info->system_utsname, sizeof(dh->utsname));
> > > > +    if (info->flag_compress & DUMP_DH_COMPRESSED_ZLIB)
> > > > +        dh->status |= DUMP_DH_COMPRESSED_ZLIB;
> > > > +#ifdef USELZO
> > > > +    else if (info->flag_compress & DUMP_DH_COMPRESSED_LZO)
> > > > +        dh->status |= DUMP_DH_COMPRESSED_LZO)
> > > > +#endif
> > > > +#ifdef USESNAPPY
> > > > +    else if (info->flag_compress & DUMP_DH_COMPRESSED_SNAPPY)
> > > > +        dh->status |= DUMP_DH_COMPRESSED_SNAPPY)
> > > > +#endif
> > > 
> > > dh->status will not guarantee that *all pages* are compressed
> > > by the check below:
> > > 
> > > >  #ifdef USELZO
> > > >                  } else if (info->flag_lzo_support
> > > >                             && (info->flag_compress &
> > > >                             DUMP_DH_COMPRESSED_LZO)
> > > >                             && ((size_out = info->page_size),
> > > >                                 lzo1x_1_compress(buf, info->page_size,
> > > >                                 buf_out,
> > > >                                                  &size_out, wrkmem) ==
> > > >                                                  LZO_E_OK)
> > > >                             && (size_out < info->page_size)) {
> > >                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > When there is no effect of compression, the page will not be compressed.
> > > 
> > > >                          pd.flags = DUMP_DH_COMPRESSED_LZO;
> > > >                          pd.size  = size_out;
> > > >                          memcpy(buf, buf_out, pd.size);
> > > >  #endif
> > > 
> > > If we are on the same page, I'll accept your patch.
> > 
> > Right, I understand.
> 
> I think the primary intention is to let crash determine early if it is
> able to open a given dump or not. And that's a good idea!

The primary intention is informational only, either for "help -[nD]" or
for the "crash -d# ..." display which dumps the header contents immediately
after reading it.   As it is now, if crash is unable to support the compression
type because the relevant library was not built in, it will accept the vmcore, 
but will fail upon the first read attempt with a message "uncompress failed: no lzo
compression support" (or snappy).
 
> From a semantic point of view, the status field in the header should be
> a logical OR of all compression methods used in the coredump. AFAICS,
> Dave's patch would actually work for the current implementation.
> 
> Did I miss something?
> 
> Petr T
 
I don't think so -- Atsushi's point was simply that there is a good
probability that there will be a mixture of compressed and uncompressed
pages.

Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec