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`