8:33 p.m.
The RHEL 5 release of mkdumprd allowed for comments in the kdump config file
as shown below:
net 192.168.1.1 # this is the comment part
The Fedora 18/19/Rawhide release would fail when inline comments were used
at runtime and when building the initrd.
This patch introduces a function which is repeated a few times, but
could be broken out at a later date into its an existing or new shell
library which could be source.
This patch does not introduce any new functionality.
This patch includes the suggestions by David Young to fix my whitespace
errors.
I have not tested this patch as I do not yet have build permissions on koji.
8:33 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
From: Wade Mealing <wmealing(a)redhat.com>
The RHEL 5 release of mkdumprd allowed for comments in the kdump config
file as shown below:
net 192.168.1.1 # this is the comment part
This patch strips them out during processing, but leaves the configuration
file in original condition.
---
dracut-kdump.sh | 8 ++++++++
dracut-module-setup.sh | 6 ++++++
kdumpctl | 9 +++++++++
mkdumprd | 6 ++++++
4 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/dracut-kdump.sh b/dracut-kdump.sh
index 7baa673..fab176a 100755
--- a/dracut-kdump.sh
+++ b/dracut-kdump.sh
@@ -230,6 +230,10 @@ get_host_ip()
return 0
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
read_kdump_conf()
{
if [ ! -f "$conf_file" ]; then
@@ -240,6 +244,8 @@ read_kdump_conf()
# first get the necessary variables
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
path)
KDUMP_PATH="$config_val"
@@ -283,6 +289,8 @@ read_kdump_conf()
# rescan for add code for dump target
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
ext[234]|xfs|btrfs|minix|nfs)
add_dump_code "dump_fs $config_val"
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
index 099c03a..cbf2912 100755
--- a/dracut-module-setup.sh
+++ b/dracut-module-setup.sh
@@ -62,6 +62,10 @@ kdump_is_vlan() {
[ -f /proc/net/vlan/"$1" ]
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
# $1: netdev name
kdump_setup_dns() {
_dnsfile=${initdir}/etc/cmdline.d/42dns.conf
@@ -252,6 +256,8 @@ kdump_install_conf() {
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
ext[234]|xfs|btrfs|minix|raw)
sed -i -e "s#$config_val#$(kdump_to_udev_name $config_val)#"
/tmp/$$-kdump.conf
diff --git a/kdumpctl b/kdumpctl
index aba1e3c..e2c4960 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -78,6 +78,10 @@ function check_exist()
done
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
#$1: the files to be checked with IFS=' '
function check_executable()
{
@@ -100,6 +104,8 @@ function check_config()
}
while read config_opt config_val; do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
\#* | "")
;;
@@ -257,6 +263,9 @@ function load_kdump()
function check_ssh_config()
{
while read config_opt config_val; do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
+
case "$config_opt" in
sshkey)
if [ -f "$config_val" ]; then
diff --git a/mkdumprd b/mkdumprd
index 26fe5af..57217d1 100644
--- a/mkdumprd
+++ b/mkdumprd
@@ -518,8 +518,14 @@ if [ "$(uname -m)" = "s390x" ]; then
add_dracut_module "znet"
fi
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
extra_modules)
extra_modules="$extra_modules $config_val"
--
1.7.1
12:42 a.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On 08/20/2013 09:33 AM, wmealing(a)redhat.com wrote:
From: Wade Mealing <wmealing(a)redhat.com>
The RHEL 5 release of mkdumprd allowed for comments in the kdump config
file as shown below:
net 192.168.1.1 # this is the comment part
This patch strips them out during processing, but leaves the configuration
file in original condition.
Missing sob line (Signed-off-by) though this is not strictly necessary
in kdump. Also there's still 2 trailing white space. It's more of style
issue.
For checking patch coding style you can use the kernel scripts of below:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scri...
Download the script and run it as below:
#checkpatch.pl --notree xxx.patch
Otherwise the patch itself is ok to me, assume you have tested it.
Care to resend with simple fix for the style issue?
---
dracut-kdump.sh | 8 ++++++++
dracut-module-setup.sh | 6 ++++++
kdumpctl | 9 +++++++++
mkdumprd | 6 ++++++
4 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/dracut-kdump.sh b/dracut-kdump.sh
index 7baa673..fab176a 100755
--- a/dracut-kdump.sh
+++ b/dracut-kdump.sh
@@ -230,6 +230,10 @@ get_host_ip()
return 0
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
read_kdump_conf()
{
if [ ! -f "$conf_file" ]; then
@@ -240,6 +244,8 @@ read_kdump_conf()
# first get the necessary variables
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
path)
KDUMP_PATH="$config_val"
@@ -283,6 +289,8 @@ read_kdump_conf()
# rescan for add code for dump target
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
ext[234]|xfs|btrfs|minix|nfs)
add_dump_code "dump_fs $config_val"
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
index 099c03a..cbf2912 100755
--- a/dracut-module-setup.sh
+++ b/dracut-module-setup.sh
@@ -62,6 +62,10 @@ kdump_is_vlan() {
[ -f /proc/net/vlan/"$1" ]
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
# $1: netdev name
kdump_setup_dns() {
_dnsfile=${initdir}/etc/cmdline.d/42dns.conf
@@ -252,6 +256,8 @@ kdump_install_conf() {
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
ext[234]|xfs|btrfs|minix|raw)
sed -i -e "s#$config_val#$(kdump_to_udev_name $config_val)#"
/tmp/$$-kdump.conf
diff --git a/kdumpctl b/kdumpctl
index aba1e3c..e2c4960 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -78,6 +78,10 @@ function check_exist()
done
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
#$1: the files to be checked with IFS=' '
function check_executable()
{
@@ -100,6 +104,8 @@ function check_config()
}
while read config_opt config_val; do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
\#* | "")
;;
@@ -257,6 +263,9 @@ function load_kdump()
function check_ssh_config()
{
while read config_opt config_val; do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
+
case "$config_opt" in
sshkey)
if [ -f "$config_val" ]; then
diff --git a/mkdumprd b/mkdumprd
index 26fe5af..57217d1 100644
--- a/mkdumprd
+++ b/mkdumprd
@@ -518,8 +518,14 @@ if [ "$(uname -m)" = "s390x" ]; then
add_dracut_module "znet"
fi
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
extra_modules)
extra_modules="$extra_modules $config_val"
--
Thanks
Dave
12:45 a.m.
For update version of patch, usually we will add the version number
in subject like:
[patch v2] Allow inline comments in config file
If there's just one patch then you need not send a cover letter. I do
not use git-send-email, but I believe there should be an option to send
patch only without cover letter.
On 08/20/2013 09:33 AM, wmealing(a)redhat.com wrote:
The RHEL 5 release of mkdumprd allowed for comments in the kdump
config file
as shown below:
net 192.168.1.1 # this is the comment part
The Fedora 18/19/Rawhide release would fail when inline comments were used
at runtime and when building the initrd.
This patch introduces a function which is repeated a few times, but
could be broken out at a later date into its an existing or new shell
library which could be source.
This patch does not introduce any new functionality.
This patch includes the suggestions by David Young to fix my whitespace
errors.
I have not tested this patch as I do not yet have build permissions on koji.
_______________________________________________
kexec mailing list
kexec(a)lists.fedoraproject.org
https://lists.fedoraproject.org/mailman/listinfo/kexec
--
Thanks
Dave
6:39 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
G'day,
I'm going to have to say that this has fallen so far off my priority list, that I
don't have time for it anymore. The patch can be taken as is or someone else
can work on it.
I think that there should be a white space removal tool when applying patches to a
repository
to prevent this kind of re-work.
Wade Mealing.
----- Original Message -----
From: "Dave Young" <dyoung(a)redhat.com>
To: wmealing(a)redhat.com
Cc: kexec(a)lists.fedoraproject.org
Sent: Wednesday, August 21, 2013 3:42:41 PM
Subject: Re: [PATCH] Strip inline comments from the kdump config file before use as values
in shell.
On 08/20/2013 09:33 AM, wmealing(a)redhat.com wrote:
From: Wade Mealing <wmealing(a)redhat.com>
The RHEL 5 release of mkdumprd allowed for comments in the kdump config
file as shown below:
net 192.168.1.1 # this is the comment part
This patch strips them out during processing, but leaves the configuration
file in original condition.
Missing sob line (Signed-off-by) though this is not strictly necessary
in kdump. Also there's still 2 trailing white space. It's more of style
issue.
For checking patch coding style you can use the kernel scripts of below:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scri...
Download the script and run it as below:
#checkpatch.pl --notree xxx.patch
Otherwise the patch itself is ok to me, assume you have tested it.
Care to resend with simple fix for the style issue?
---
dracut-kdump.sh | 8 ++++++++
dracut-module-setup.sh | 6 ++++++
kdumpctl | 9 +++++++++
mkdumprd | 6 ++++++
4 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/dracut-kdump.sh b/dracut-kdump.sh
index 7baa673..fab176a 100755
--- a/dracut-kdump.sh
+++ b/dracut-kdump.sh
@@ -230,6 +230,10 @@ get_host_ip()
return 0
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
read_kdump_conf()
{
if [ ! -f "$conf_file" ]; then
@@ -240,6 +244,8 @@ read_kdump_conf()
# first get the necessary variables
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
path)
KDUMP_PATH="$config_val"
@@ -283,6 +289,8 @@ read_kdump_conf()
# rescan for add code for dump target
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
ext[234]|xfs|btrfs|minix|nfs)
add_dump_code "dump_fs $config_val"
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
index 099c03a..cbf2912 100755
--- a/dracut-module-setup.sh
+++ b/dracut-module-setup.sh
@@ -62,6 +62,10 @@ kdump_is_vlan() {
[ -f /proc/net/vlan/"$1" ]
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
# $1: netdev name
kdump_setup_dns() {
_dnsfile=${initdir}/etc/cmdline.d/42dns.conf
@@ -252,6 +256,8 @@ kdump_install_conf() {
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
ext[234]|xfs|btrfs|minix|raw)
sed -i -e "s#$config_val#$(kdump_to_udev_name $config_val)#"
/tmp/$$-kdump.conf
diff --git a/kdumpctl b/kdumpctl
index aba1e3c..e2c4960 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -78,6 +78,10 @@ function check_exist()
done
}
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
#$1: the files to be checked with IFS=' '
function check_executable()
{
@@ -100,6 +104,8 @@ function check_config()
}
while read config_opt config_val; do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
\#* | "")
;;
@@ -257,6 +263,9 @@ function load_kdump()
function check_ssh_config()
{
while read config_opt config_val; do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
+
case "$config_opt" in
sshkey)
if [ -f "$config_val" ]; then
diff --git a/mkdumprd b/mkdumprd
index 26fe5af..57217d1 100644
--- a/mkdumprd
+++ b/mkdumprd
@@ -518,8 +518,14 @@ if [ "$(uname -m)" = "s390x" ]; then
add_dracut_module "znet"
fi
+strip_comments() {
+ echo $1 | sed -e 's/\(.*\)#.*/\1/'
+}
+
while read config_opt config_val;
do
+ # remove inline comments after the end of a directive.
+ config_val=$(strip_comments $config_val)
case "$config_opt" in
extra_modules)
extra_modules="$extra_modules $config_val"
--
Thanks
Dave
8:06 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
Hi Wade,
Thanks for your patch. I know high priority tasks need be handled
firstly.
Don't be discouraged. I will merge it with tiny adjusting.
Baoquan
Thanks a lot
On 08/21/13 at 07:39pm, Wade Mealing wrote:
G'day,
I'm going to have to say that this has fallen so far off my priority list, that I
don't have time for it anymore. The patch can be taken as is or someone else
can work on it.
I think that there should be a white space removal tool when applying patches to a
repository
to prevent this kind of re-work.
Wade Mealing.
12:20 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On Thu, Aug 22, 2013 at 09:06:32AM +0800, Baoquan He wrote:
Hi Wade,
Thanks for your patch. I know high priority tasks need be handled
firstly.
Don't be discouraged. I will merge it with tiny adjusting.
Bao,
This path define strip_xyz() 2 times in two separate files. I think it
is high time we start thinking of intorducing a shared functions file
for kdump which various kdump scripts can share.
Adding duplicate code is bad.
Chao, you were planning to look into this. Did you make any progress?
Thanks
Vivek
2:44 a.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On 08/30/13 at 01:20pm, Vivek Goyal wrote:
On Thu, Aug 22, 2013 at 09:06:32AM +0800, Baoquan He wrote:
> Hi Wade,
>
> Thanks for your patch. I know high priority tasks need be handled
> firstly.
>
> Don't be discouraged. I will merge it with tiny adjusting.
>
Bao,
This path define strip_xyz() 2 times in two separate files. I think it
is high time we start thinking of intorducing a shared functions file
for kdump which various kdump scripts can share.
Adding duplicate code is bad.
Chao, you were planning to look into this. Did you make any progress?
Last time I checked, I could only extract about 6 or 7 common functions
without doing major change crossing the kdump source code.
It looks like we do have common functions used by multiple file. But
that's quite a small group. I don't know if it's worth it to have
minority in a shared lib file and leave the majority still. What do you
think?
Thanks
WANG Chao
10:13 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On 08/30/13 at 01:20pm, Vivek Goyal wrote:
On Thu, Aug 22, 2013 at 09:06:32AM +0800, Baoquan He wrote:
> Hi Wade,
>
> Thanks for your patch. I know high priority tasks need be handled
> firstly.
>
> Don't be discouraged. I will merge it with tiny adjusting.
>
Bao,
This path define strip_xyz() 2 times in two separate files. I think it
is high time we start thinking of intorducing a shared functions file
for kdump which various kdump scripts can share.
Adding duplicate code is bad.
Chao, you were planning to look into this. Did you make any progress?
Hi,
Since Chao has taken the task to introducing a shared functions file for
kdump file, I would like to merge this patch directly. Or if you think
it's not good, I can wait until Chao's patch for shared functions file
is OK.
Baoquan
Thanks
_______________________________________________
kexec mailing list
kexec(a)lists.fedoraproject.org
https://lists.fedoraproject.org/mailman/listinfo/kexec
1:59 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On Mon, Sep 02, 2013 at 03:44:15PM +0800, WANG Chao wrote:
On 08/30/13 at 01:20pm, Vivek Goyal wrote:
> On Thu, Aug 22, 2013 at 09:06:32AM +0800, Baoquan He wrote:
> > Hi Wade,
> >
> > Thanks for your patch. I know high priority tasks need be handled
> > firstly.
> >
> > Don't be discouraged. I will merge it with tiny adjusting.
> >
>
> Bao,
>
> This path define strip_xyz() 2 times in two separate files. I think it
> is high time we start thinking of intorducing a shared functions file
> for kdump which various kdump scripts can share.
>
> Adding duplicate code is bad.
>
> Chao, you were planning to look into this. Did you make any progress?
Last time I checked, I could only extract about 6 or 7 common functions
without doing major change crossing the kdump source code.
I think that's a good start to create a file for shared functions.
It looks like we do have common functions used by multiple file.
Where is that. We don't seem to have any file with common functions
which is included by others?
But
that's quite a small group. I don't know if it's worth it to have
minority in a shared lib file and leave the majority still. What do you
think?
I still think that we need to introduce a common function file. We might
start small but once we have a place holder, future common functions will
go there.
Only thing we need to make sure is that same functions should be usable
in both dracut context as well as mkdumprd context.
Thanks
Vivek
2:18 p.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On Tue, Sep 10, 2013 at 11:13:00AM +0800, Baoquan He wrote:
On 08/30/13 at 01:20pm, Vivek Goyal wrote:
> On Thu, Aug 22, 2013 at 09:06:32AM +0800, Baoquan He wrote:
> > Hi Wade,
> >
> > Thanks for your patch. I know high priority tasks need be handled
> > firstly.
> >
> > Don't be discouraged. I will merge it with tiny adjusting.
> >
>
> Bao,
>
> This path define strip_xyz() 2 times in two separate files. I think it
> is high time we start thinking of intorducing a shared functions file
> for kdump which various kdump scripts can share.
>
> Adding duplicate code is bad.
>
> Chao, you were planning to look into this. Did you make any progress?
>
Hi,
Since Chao has taken the task to introducing a shared functions file for
kdump file, I would like to merge this patch directly. Or if you think
it's not good, I can wait until Chao's patch for shared functions file
is OK.
We have let it go multiple times. I don't like the idea of duplicating
functions any more.
So somebody please create a common functions file and put strip_xyz()
in that. Let us not create common instance of that function twice.
Thanks
Vivek
12:13 a.m.
New subject: [PATCH] Strip inline comments from the kdump config file before use as values in shell.
On 09/23/13 at 02:59pm, Vivek Goyal wrote:
On Mon, Sep 02, 2013 at 03:44:15PM +0800, WANG Chao wrote:
> On 08/30/13 at 01:20pm, Vivek Goyal wrote:
> > On Thu, Aug 22, 2013 at 09:06:32AM +0800, Baoquan He wrote:
> > > Hi Wade,
> > >
> > > Thanks for your patch. I know high priority tasks need be handled
> > > firstly.
> > >
> > > Don't be discouraged. I will merge it with tiny adjusting.
> > >
> >
> > Bao,
> >
> > This path define strip_xyz() 2 times in two separate files. I think it
> > is high time we start thinking of intorducing a shared functions file
> > for kdump which various kdump scripts can share.
> >
> > Adding duplicate code is bad.
> >
> > Chao, you were planning to look into this. Did you make any progress?
>
> Last time I checked, I could only extract about 6 or 7 common functions
> without doing major change crossing the kdump source code.
I think that's a good start to create a file for shared functions.
Yeah, sure.
>
> It looks like we do have common functions used by multiple file.
Where is that. We don't seem to have any file with common functions
which is included by others?
We don't have any file to share common functions.
> But
> that's quite a small group. I don't know if it's worth it to have
> minority in a shared lib file and leave the majority still. What do you
> think?
I still think that we need to introduce a common function file. We might
start small but once we have a place holder, future common functions will
go there.
Only thing we need to make sure is that same functions should be usable
in both dracut context as well as mkdumprd context.
We might also need it in 2nd kernel.
I'll start working on this.
Thanks
WANG Chao
3865
days inactive
3900
days old
11 comments
5 participants
participants (5)
-
Baoquan He -
Dave Young -
Vivek Goyal -
Wade Mealing -
WANG Chao