Bug 459614

Created attachment 314642 [details]
patch to check stamps on other file

This should suck in the extra files that need checking.  Please test and confirm.  the kernel image was already included in that list however.  CAn you tell me a bit more about what file you touched, how you verified the timestamp was modified and what you observed when the kdump service was restarted.

Thanks!

testing was:
 - touch an input file
 - service kdump restart

Findings:
  - kdump_pre script is properly checked
  - kdump_post script is properly checked
  - extra_bins was not tested, but I see no reason to think the same change wouldn't work here
These changes are definitely worth taking.


  - extra_modules does not work as expected.  Listing any extra module causes a dumprd rebuild.

I think the problem is because the kernel modules are referenced by name and not by path.  There's also a subtlety here that modules.dep should be traversed to check dependencies.--otherwise updating the dependency will not trigger rebuilding the dumprd.

 - extra_modules does not work as expected.  Listing any extra module causes a
dumprd rebuild.


Not sure what you mean by that?  Did you mean to say that it does _not_ cause a rebuild?  Or does it cause a rebuild when it should not?

What I meant was that date_check for the extra_modules needs to be more sophisticated.  

Modules are listed by module name, not file path--not the least because the path is dependent on the kernel version.  For example:

    extra_modules ipmi_msghandler 

references kernel module file:
  
   /lib/modules/2.6.18-92.1.10.el5/kernel/drivers/char/ipmi/ipmi_msghandler.ko

Treating the extra_modules as files as in:

+	CHECK_FILE=`grep ^extra_modules $KDUMP_CONFIG_FILE | cut -d\  -f2-`
+	EXTRA_FILES="$EXTRA_FILES $CHECK_FILE"

causes the dumprd to be rebuilt every time the kdump service is started because the file "ipmi_msghandler" does not exist.

The trace is:
   #> service kdump restart
   Stopping kdump:                                            [  OK  ]
   Detected change(s) the following file(s):
  
      ipmi_msghandler
   Rebuilding /boot/initrd-2.6.18-92.1.10.el5kdump.img
   Starting kdump:                                            [  OK  ]

(In reply to comment #4)
> What I meant was that date_check for the extra_modules needs to be more
> sophisticated.  
> 
[...snip...]
IOW, /lib/modules/$(uname -r)/modules.dep is rquired to dereference module names and dependencies.

For exmample:

for mod in $extra_modules
do
    mod_ref=$(grep "/$mod.ko:" /lib/modules/$(uname -r)/modules.dep)
    mod_file=$(echo $(mod_ref) | cut -d':' -f1)
    #...check $mod_file
    mod_deps=$(echo $(mod_ref) | cut -d':' -f2)
    #...recursively check $mod_deps
done

Or if it's a safe assumption that modules.dep is updated everytime a module is changed, then the following might suffice at the expense of an occasion unnecessary rebuild.

if [ "$(grep '^extra_modules' KDUMP_CONFIG_FILE | cut -d\  -f2- ) -ne "" ]
then
    EXTRA_FILES="$EXTRA_FILES /lib/modules/$(uname -r)/modules.dep
fi

I think perhaps the safest thing to do is look to see if extra_modules is non-null and rebuild if it has anything listed.  Its overkill, but it saves us having to worry about wierd cases, like if someone has a 3rd parth module that doesn't update the dep tree or some such.

I've checked this and the extra_modules detection into -35.el5.

yes, already reported, slated to be fixed in 5.4

*** Bug 474365 has been marked as a duplicate of this bug. ***

*** Bug 471306 has been marked as a duplicate of this bug. ***

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 therefore 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/RHBA-2009-0105.html