Bug 171137

I am awaiting a set of proposed changes to the crash utility from NEC
to properly support their new dumpfile format.  

The initial changes proposed by NEC were unnacceptable because it depended
upon untested, constantly-changing, and unmaintained (by me) LKCD code in
the crash utility.  A meeting was held with the in-house Fujitsu team to
discuss a better, safer, and acceptable manner of creating and accessing the
dumpfile, and my proposal was passed back to the NEC developers.

I am putting this bugzilla into NEEDINFO_REPORTER until a reply from NEC
is received.

NEC and Fujitsu are now trying very hard to improve the previous patch. The
current patchset is almost free for LKCD origin code and clean. I'm now
reviewing the experimental patch for crash and found it is relatively
small(600step) and clean as experimental. I'll upload the crash patch. Please
see and evaluate it.

Created attachment 120330 [details]
crash.patch

This is the patch for crash to support compressible dump. This is the first and
experimental patch and will be changed later.

> This is the patch for crash to support compressible dump. This is the first and
> experimental patch and will be changed later.

Outstanding!  This is exactly what I wanted to see -- nice work!

Looks like the NEEDINFO_REPORTER has been satisfied. Setting to ASSIGNED.

> Looks like the NEEDINFO_REPORTER has been satisfied. Setting to ASSIGNED.

Not really.  Comment #4 indicates:

> This is the first and experimental patch and will be changed later.

Just to be clear, I, the "assignee", am still waiting for the "reporter"
to provide their final, non-experimental, patch.

How can this be NAK'd if the related diskdump bugzillas are still 
in play?  Diskdump bugzillas 168641 and 171141 are both RHEL4U3CanFix:

Bugzilla Bug 168641 â FEAT RHEL4 U3 [diskdump]: Support compressing dump data
Bugzilla Bug 171141 â FEAT RHEL4 U3 [diskdump]: kernel - support compressing dump

It makes no sense to put this diskdump feature in place if crash can't
read the dumpfile!  It's a three-part feature:

 168641 - diskdumputils user package changes for creating new dumpfile format
 171141 - diskdump kernel module changes for creating new dumpfile format
 171137 - crash utility changes to read new dumpfile format

If you NAK one, you've got to NAK them all.

Peter and/or Linda -- please raise this issue at the next RHEL meeting.

 

Created attachment 120618 [details]
crash-compress-3.patch

This is the updated patch for compressed dump support for crash. It includes
performance improvement by caching index values of page_desc_t and clean up.
The size of patch became a bit smaller than the previous one. If no problem is
found, this will be the final proposed patch. Please review this.

Flipping to ASSIGNED as Tachino-san has provided what they believe is their
final pass (what Engineering was waiting for). 

Has this patch been submitted upstream? Accepted? 

(In reply to comment #14)
> Has this patch been submitted upstream? Accepted? 

Hi,

Dave is the upstream maintainer of crash and he decides the patch is acceptable
or not.

Thanks, Nobuhiro.  I'll review the patch, and if acceptable, I'll fold
it into a new upstream version for Fujitsu and NEC to sanity-check and
verify.  Then that version will subsequently be used as a base to create
a beehive-built version for the RHEL4-U3 errata procedure.

The patch looks good.  Thanks to both NEC and Fujitsu for their time and effort.
I will update the upstream package shortly.

Whenever work of this magnitude is added to the crash utility, I have
always wanted to give credit where credit is due.  During crash initialization,
a list of the companies who have made major contributions have their copyrights
shown:

# crash

crash 4.0-2.7
Copyright (C) 2002, 2003, 2004, 2005  Red Hat, Inc.
Copyright (C) 2004, 2005  IBM Corporation
Copyright (C) 1999-2005  Hewlett-Packard Co
Copyright (C) 1999, 2002  Silicon Graphics, Inc.
Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
This program is free software, covered by the GNU General Public License,
and you are welcome to change it and/or distribute copies of it under
certain conditions.  Enter "help copying" to see the conditions.
This program has absolutely no warranty.  Enter "help warranty" for details.
...

I would like to add Fujitsu and NEC to the list.  In the past, each company
has preferred that their company "name string" be presented in a specific
manner, i.e., IBM wanted it to be "IBM Corporation", and HP wanted theirs
to be "Hewlett-Packard Co".

If Fujitsu and NEC does not want to be listed, that's fine too.
  
But if they do, how should each company name string be listed?

  

> I would like to add Fujitsu and NEC to the list.  In the past, each company
> has preferred that their company "name string" be presented in a specific
> manner, i.e., IBM wanted it to be "IBM Corporation", and HP wanted theirs
> to be "Hewlett-Packard Co".
That sounds great to us! Thanks!

> If Fujitsu and NEC does not want to be listed, that's fine too.
We want to, but we don't know what NEC guys think. So we need to ask them
about that.

> But if they do, how should each company name string be listed?
I'll ask Fujitsu Japan about that. Then, I'll let you know how. As for
NEC guys, if they do, I'll also ask them.

Does that make sense?

> I'll ask Fujitsu Japan about that. Then, I'll let you know how. As for
> NEC guys, if they do, I'll also ask them.
>
> Does that make sense?

Sure, that's fine -- I'll wait until you indicate final approval from
both NEC and Fujitsu.

Other than that, I've made only minor modifications to your last patch,
mainly changes to error messages.  For example, we don't want to
print error messages in read_netump_header() until *after* the
header's signature has been read and confirmed -- at least unless
CRASHDEBUG(1) is set.  Since is_diskdump() is called in main()
*before* is_lkcd_compressed_dump(), is_system_map() and is_s390_dump(),
you would see unnecessary "dump does not have panic dump header"
messages for those types of files.

s/read_netump_header/read_dump_header/

I've update the upstream version of crash to 4.0-2.9:

  http://people.redhat.com/anderson

As I mentioned above, I made a few minor changes that deal with
error messages, as well as a few unused variable removals that
allow crash to be built via "make Warn".

Can you please sanity check this version?  If it's OK, I will take
a snapshot of this release for use in the RHEL4-U3 crash-utility errata.

Note that I did not add NEC and Fujitus to the "credits", but will
do so if you tell me it's OK, and what name string to use.

Thanks,
  Dave

The patch looks fine. I tested 4.0-2.9 on ia64 machine in the lab and it worked
with no problem. Thanks!

OK, great, thanks!

I received the reply from NEC and Fujitsu Japan regarding to their opyrights.

Fujitsu:
Copyright (C) 2005  FUJITSU LIMITED

NEC:
Copyright (C) 2005  NEC Corporation

Thank you.

I don't mean to be a bother here, but, is there any legal reason that
the Fujitsu name cannot be expressed in lower-case?  In other words,
can I make it:

  Copyright (C) 2005  Fujitsu Limited

instead of: 

  Copyright (C) 2005  FUJITSU LIMITED

I ask because it looks a little strange compared to the others:

  $ crash -v

  crash 4.0-2.9a
  Copyright (C) 2002, 2003, 2004, 2005  Red Hat, Inc.
  Copyright (C) 2004, 2005  IBM Corporation
  Copyright (C) 1999-2005  Hewlett-Packard Co
  Copyright (C) 2005  FUJITSU LIMITED
  Copyright (C) 2005  NEC Corporation
  Copyright (C) 1999, 2002  Silicon Graphics, Inc.
  Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
  This program is free software, covered by the GNU General Public License,
  and you are welcome to change it and/or distribute copies of it under
  certain conditions.  Enter "help copying" to see the conditions.
  This program has absolutely no warranty.  Enter "help warranty" for details.

  $

It shows up more than others, but that's not our intention ;-) The uppercase
copyright is just our standard format. But I believe "Fujitsu Limited" is fine too.
Please change it.

Please QA ack this bugzilla. It has PM and DEVEL acks, and I need
to start the errata process ASAP.

> Please QA ack this bugzilla. It has PM and DEVEL acks, and I need
> to start the errata process ASAP.

Note also that the blocked diskdumputils bugzilla:

  Bugzilla Bug 168641 â FEAT RHEL4 U3 [diskdump]: Support compressing dump data

does have all 3 ACKS, but you can't have one without the other.

Please give this BZ a QA_ACK!

We, Fujitsu, are to do testing against crash package because this feature
was submitted by us. We should be responsible for this BZ. So please give
this BZ a QA_ACK.

Best regards,
Akira

Fujitsu testing of the compressed diskdump format uncovered a
bug with the "bt" command for ppc64 dumpfile only, which will
require a package update from 4.0-2.14 to 4.0-2.15.

The testing of 4.0-2.15 has been completed and no problems were found.

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHEA-2006-0075.html