[PATCH v3] kdumpctl: for fence_kdump, the ipaddr of this node should be excluded from list
kdump should not send fence_kdump notifications to local host, because the role of the falied node (i.e local host) is to send fence_kdump notifications to other nodes to tell them I'm kdumping, tell to itself is nonsense. And we have excluded hostname of local host but when one use ip address we also need exclude it.
Signed-off-by: Pingfan Liu piliu@redhat.com --- kdump.conf | 4 ++-- kdumpctl | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/kdump.conf b/kdump.conf index 1e24e1b..cfdaec7 100644 --- a/kdump.conf +++ b/kdump.conf @@ -137,8 +137,8 @@ # # fence_kdump_nodes <node(s)> # - List of cluster node(s), separated by spaces, to send -# fence_kdump notifications to (this option is mandatory to -# enable fence_kdump). +# fence_kdump notifications to. The list should exclude this node itself! +# (this option is mandatory to enable fence_kdump). #
#raw /dev/vg/lv_kdump diff --git a/kdumpctl b/kdumpctl index 60bbd93..e440bbb 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1099,6 +1099,7 @@ check_kdump_feasibility() check_fence_kdump_config() { local hostname=`hostname` + local ipaddrs=`hostname -I` local nodes=$(get_option_value "fence_kdump_nodes")
for node in $nodes; do @@ -1106,6 +1107,12 @@ check_fence_kdump_config() echo "Option fence_kdump_nodes cannot contain $hostname" return 1 fi + # node can be ipaddr + echo $ipaddrs | grep $node > /dev/null + if [ $? -eq 0 ]; then + echo "Option fence_kdump_nodes cannot contain $node" + return 1 + fi done
return 0
On 05/17/2017 at 11:16 AM, Pingfan Liu wrote:
kdump should not send fence_kdump notifications to local host, because the role of the falied node (i.e local host) is to send fence_kdump notifications to other nodes to tell them I'm kdumping, tell to itself is nonsense. And we have excluded hostname of local host but when one use ip address we also need exclude it.
Signed-off-by: Pingfan Liu piliu@redhat.com
kdump.conf | 4 ++-- kdumpctl | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/kdump.conf b/kdump.conf index 1e24e1b..cfdaec7 100644 --- a/kdump.conf +++ b/kdump.conf @@ -137,8 +137,8 @@ # # fence_kdump_nodes <node(s)> # - List of cluster node(s), separated by spaces, to send -# fence_kdump notifications to (this option is mandatory to -# enable fence_kdump). +# fence_kdump notifications to. The list should exclude this node itself!
I personally prefer "s/this node itself/localhost/"
+# (this option is mandatory to enable fence_kdump). #
#raw /dev/vg/lv_kdump diff --git a/kdumpctl b/kdumpctl index 60bbd93..e440bbb 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1099,6 +1099,7 @@ check_kdump_feasibility() check_fence_kdump_config() { local hostname=`hostname`
- local ipaddrs=`hostname -I`
Nitpick: using a variable name with the "local" or "host" prefix looks better? Like "local_ipaddrs", etc. Anyway: Reviewed-by: Xunlei Pang xlpang@redhat.com
local nodes=$(get_option_value "fence_kdump_nodes")
for node in $nodes; do @@ -1106,6 +1107,12 @@ check_fence_kdump_config() echo "Option fence_kdump_nodes cannot contain $hostname" return 1 fi
# node can be ipaddrecho $ipaddrs | grep $node > /dev/nullif [ $? -eq 0 ]; thenecho "Option fence_kdump_nodes cannot contain $node"return 1fidone
return 0
On 05/17/17 at 12:24pm, Xunlei Pang wrote:
On 05/17/2017 at 11:16 AM, Pingfan Liu wrote:
kdump should not send fence_kdump notifications to local host, because the role of the falied node (i.e local host) is to send fence_kdump notifications to other nodes to tell them I'm kdumping, tell to itself is nonsense. And we have excluded hostname of local host but when one use ip address we also need exclude it.
Signed-off-by: Pingfan Liu piliu@redhat.com
kdump.conf | 4 ++-- kdumpctl | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/kdump.conf b/kdump.conf index 1e24e1b..cfdaec7 100644 --- a/kdump.conf +++ b/kdump.conf @@ -137,8 +137,8 @@ # # fence_kdump_nodes <node(s)> # - List of cluster node(s), separated by spaces, to send -# fence_kdump notifications to (this option is mandatory to -# enable fence_kdump). +# fence_kdump notifications to. The list should exclude this node itself!
I personally prefer "s/this node itself/localhost/"
I can change it manually when applying..
Acked-by: Dave Young dyoung@redhat.com
+# (this option is mandatory to enable fence_kdump). #
#raw /dev/vg/lv_kdump diff --git a/kdumpctl b/kdumpctl index 60bbd93..e440bbb 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1099,6 +1099,7 @@ check_kdump_feasibility() check_fence_kdump_config() { local hostname=`hostname`
- local ipaddrs=`hostname -I`
Nitpick: using a variable name with the "local" or "host" prefix looks better? Like "local_ipaddrs", etc. Anyway: Reviewed-by: Xunlei Pang xlpang@redhat.com
local nodes=$(get_option_value "fence_kdump_nodes")
for node in $nodes; do @@ -1106,6 +1107,12 @@ check_fence_kdump_config() echo "Option fence_kdump_nodes cannot contain $hostname" return 1 fi
# node can be ipaddrecho $ipaddrs | grep $node > /dev/nullif [ $? -eq 0 ]; thenecho "Option fence_kdump_nodes cannot contain $node"return 1fidone
return 0
Thanks Dave
----- Original Message -----
From: "Dave Young" dyoung@redhat.com To: xlpang@redhat.com Cc: "Pingfan Liu" piliu@redhat.com, kexec@lists.fedoraproject.org, "Baoquan He" bhe@redhat.com Sent: Thursday, May 18, 2017 1:53:26 PM Subject: Re: [PATCH v3] kdumpctl: for fence_kdump, the ipaddr of this node should be excluded from list
On 05/17/17 at 12:24pm, Xunlei Pang wrote:
On 05/17/2017 at 11:16 AM, Pingfan Liu wrote:
kdump should not send fence_kdump notifications to local host, because the role of the falied node (i.e local host) is to send fence_kdump notifications to other nodes to tell them I'm kdumping, tell to itself is nonsense. And we have excluded hostname of local host but when one use ip address we also need exclude it.
Signed-off-by: Pingfan Liu piliu@redhat.com
kdump.conf | 4 ++-- kdumpctl | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/kdump.conf b/kdump.conf index 1e24e1b..cfdaec7 100644 --- a/kdump.conf +++ b/kdump.conf @@ -137,8 +137,8 @@ # # fence_kdump_nodes <node(s)> # - List of cluster node(s), separated by spaces, to send -# fence_kdump notifications to (this option is mandatory to -# enable fence_kdump). +# fence_kdump notifications to. The list should exclude this node itself!
I personally prefer "s/this node itself/localhost/"
I can change it manually when applying..
thanks!
Regards, Pingfan
Acked-by: Dave Young dyoung@redhat.com
+# (this option is mandatory to enable fence_kdump). #
#raw /dev/vg/lv_kdump diff --git a/kdumpctl b/kdumpctl index 60bbd93..e440bbb 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1099,6 +1099,7 @@ check_kdump_feasibility() check_fence_kdump_config() { local hostname=`hostname`
- local ipaddrs=`hostname -I`
Nitpick: using a variable name with the "local" or "host" prefix looks better? Like "local_ipaddrs", etc. Anyway: Reviewed-by: Xunlei Pang xlpang@redhat.com
local nodes=$(get_option_value "fence_kdump_nodes")
for node in $nodes; do @@ -1106,6 +1107,12 @@ check_fence_kdump_config() echo "Option fence_kdump_nodes cannot contain $hostname" return 1 fi
# node can be ipaddrecho $ipaddrs | grep $node > /dev/nullif [ $? -eq 0 ]; thenecho "Option fence_kdump_nodes cannot contain $node"return 1fidone
return 0
Thanks Dave
-
Dave Young -
Pingfan Liu -
Xunlei Pang