----- Original Message -----
From: "Baoquan He" <bhe(a)redhat.com>
To: "Qiao Zhao" <qzhao(a)redhat.com>
Cc: kexec(a)lists.fedoraproject.org
Sent: Monday, April 20, 2015 3:54:15 PM
Subject: Re: [PATCH] remove "noauto" options in 2nd kernel fstab
On 04/20/15 at 03:25pm, Qiao Zhao wrote:
> "noauto" will fail nfs dump, remove "noauto" in 2nd kernel
fstab.
Very good, Qiao.
I have several concerns about this patch.
About patch log, we need tell what/why/how
what is the problem. Describe the prblem your patch fix.
why it happened. Describe the root cause you found after analysis.
how you fix it. Just describe what we have talked last time with Minfei,
you and me.
Patch log doesn't appear in code, but help reviews to get the point
quickly. And most importantly it can remind us of the detail long time
later.
Here is a patch log I wrote, just for your reference. You don't have to
do like it completely, just do well on what/why/how.
Thanks for your suggestion, I will make a new patch.
commit 1a8a39aa9c9c0e7618a9b89fc340eab1b2334a33
Author: Baoquan He <bhe(a)redhat.com>
Date: Wed Dec 31 10:48:59 2014 +0800
adding the parsed path to etc/kdump.conf of kdump initrd
Steve found a bug. When mount a disk in /var and not specify path
in /etc/kdump.conf, the vmcore will be dumped into /var/crash of
that disk, but not /crash on that disk.
This is because when write the parsed path into /tmp/$$-kdump.conf
in default_dump_target_install_conf() of mkdumprd, it uses below
sed command. So if no path specified at all, this sed command won't
add it to /tmp/$$-kdump.conf. Then in 2nd kernel it will take
default
path, namely "/var/crash" as path if no path in /etc/kdump.conf in
2nd kernel.
sed -i -e "s#$_save_path#$_path#" /tmp/$$-kdump.conf
According to Dave Young's suggestion, erase the old path line and
then
insert the parsed path. This can fix it.
v2->v3:
erase the old path line and then insert the parsed path.
sed -i -e "s#^path[[:space:]]\+$_save_path##" /tmp/$$-kdump.conf
echo "path $_path" >> /tmp/$$-kdump.conf
v3->v4:
Change the sed pattern, erase lines starting with "path" and
then
insert the parsed path.
sed -i -e "s#^path.*##" /tmp/$$-kdump.conf
echo "path $_path" >> /tmp/$$-kdump.conf
v4->v5:
Chaowang suggested using sed command d to remove the whole line
like below:
sed -i "/^path/d" /tmp/$$-kdump.conf
echo "path $_path" >> /tmp/$$-kdump.conf
>
> ---
> mkdumprd | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/mkdumprd b/mkdumprd
> index 4d251ba..82e06b0 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -114,6 +114,7 @@ to_mount() {
> _fstype=$(findmnt -k -f -n -r -o FSTYPE $_dev)
> _options=$(findmnt --fstab -f -n -r -o OPTIONS $_dev)
> [ -z "$_options" ] && _options=$(findmnt -k -f -n -r -o
OPTIONS $_dev)
> + [[ $_options =~ "noauto" ]] && _options=$(echo $_options |
sed
> 's/noauto//') #noauto will fail dump nfs dump, remove "noauto" in
2nd
> kernel fstab.
I think it's unnecessary to check whether noauto exist. Just remove it.
I think
this check is necessary, because it will do less step when there
is no this options.
I'm not sure if this case is correct or not. :)
If this is the wrong idea, i will delete this check.
Anyway, it's closer to a good post.
> _options=${_options/#ro/rw} #mount fs target as rw in 2nd kernel
> # "x-initrd.mount" mount failure will trigger isolate emergency
> service
> # W/o this, systemd won't isolate, thus we won't get to emergency.
> --
> 1.9.3
>
> _______________________________________________
> kexec mailing list
> kexec(a)lists.fedoraproject.org
>
https://lists.fedoraproject.org/mailman/listinfo/kexec