On 08/15/2013 01:42 PM, Wade Mealing wrote:
Fedora bug lodged:
https://bugzilla.redhat.com/show_bug.cgi?id=997245
The RHEL 5 release of mkdumprd allowed for comments in the kdump file like this
net 192.168.1.1 # this is the comment part
The Fedora release of mkdumprd fails in many places when those directives use inline
comments.
I've attached a patch that seems to work for me, with basic testing. It should
introduce no additional
functionality to the system.
Thanks,
Wade Mealing
N�n�r����)em�h�yhiם�w^��
Hi, Wade
Thanks for the patch. There's a few comments from my side:
Firstly, it's encouraged to send the patch content inline instead of
attachment so it's easier to review. Also the patch usually contains
several part: Subject, Description, sob line, diffstat, diff content.
Basiclly what need to know about patch is similar to below documentation
from kernel:
https://www.kernel.org/doc/Documentation/SubmittingPatches
https://www.kernel.org/doc/Documentation/email-clients.txt
For the first time it need spend some time, but it will be natural
later, it will be more clear and saving everyone's time.
For the patch itself:
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
[dave@darkstar kexec-tools]$ grep "while read" * 2>/dev/null
dracut-kdump.sh: while read config_opt config_val;
dracut-kdump.sh: while read config_opt config_val;
dracut-module-setup.sh: while read config_opt config_val;
dracut-module-setup.sh: while read _initiator; do
kdumpctl: while read config_opt config_val; do
kdumpctl: while read config_opt config_val; do
mkdumprd:while read config_opt config_val;
mkdumprd.orig:while read config_opt config_val;
kexec-tools/dracut-kdump.sh is for 2nd kernel use,
dracut-module-setup.sh: kdump_install_conf() copy the kdump.conf to initrd.
In this place you can remove the inline comment before the copying.
For 1st kernel scripts, kdumpctl, mkdumprd, dracut-module-setup.sh
there's several places where we read config_val as you see. so they all
need fix
OTOH, need to make sure there's no other place such as setting
config_val via `grep`
--
Thanks
Dave