----- Original Message -----
From: "Vivek Goyal" <vgoyal(a)redhat.com>
To: "Martin Perina" <mperina(a)redhat.com>
Cc: kexec(a)lists.fedoraproject.org
Sent: Thursday, March 13, 2014 11:14:35 PM
Subject: Re: Adding support for manually configured fence_kdump v2
On Thu, Mar 13, 2014 at 10:18:47AM -0400, Martin Perina wrote:
> Hi,
>
> I changed this from initial patch:
>
> 1) /etc/sysconfig/fence_kdump is optional, only following two files are
> mandatory:
>
> /usr/libexec/fence_kdump_send
> /etc/sysconfig/fence_kdump_nodes
>
> 2) Nodes read from /etc/sysconfig/fence_kdump_nodes are filtered not to
> contain hostname
>
> 3) Patch is created against master on
>
http://pkgs.fedoraproject.org/cgit/kexec-tools.git/
>
Can you please also modify kdump-in-cluster-environment.txt file and
create a small section which talks about that a different cluster
mechanism is used in OVirt. What nodes need to be modifed. How that
node list is generated and how kdump includes that list.
Sure
>
> Thanks
>
> Martin Perina
>
>
> From 48f09ed8cda2c1fe52b0a48eb501373cd40d6b8f Mon Sep 17 00:00:00 2001
> From: Martin Perina <mperina(a)redhat.com>
> Date: Thu, 13 Mar 2014 14:59:08 +0100
> Subject: [PATCH] fence_kdump manual configuration support
>
> Adds ability to configure fence_kdump without Pacemaker. If following
> files exist:
>
> /usr/libexec/fence_kdump_send
> /etc/sysconfig/fence_kdump_nodes
Rethinking about it, I think I like this fence_kdump_nodes idea. This
way we don't care who generated it and we don't have to worry about other
tools.
That was my idea.
>
> it's supposed that fence_kdump is configured manually.
>
> Signed-off-by: Martin Perina <mperina(a)redhat.com>
> ---
> dracut-module-setup.sh | 37 +++++++++++++++++++++++++------------
> kdump-lib.sh | 20 +++++++++++++++-----
> kdumpctl | 5 +++++
> 3 files changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index bdadf7c..103a84e 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -421,28 +421,41 @@ kdump_check_iscsi_targets () {
> # also preserve '[node list]' for 2nd kernel /etc/fence_kdump_nodes
^^^^^^^^
Need to change to /etc/sysconfig/fence_kdump_nodes?
IMO it's standard directory for system configuration. And when fence_kdump
config file is already there, why should fence_kdump_nodes be on different place?
But if you don't like the idea we can move both files for example to
/etc/fence_kdump directory (RPM fence-agents-kdump doensn't include any
info about configuration, it just contains only 2 binaries and man pages).
> kdump_check_fence_kdump () {
> local nodes
> + local xnodes
> is_fence_kdump || return 1
>
> - # get cluster nodes from cluster cib, get interface and ip address
> - nodelist=`pcs cluster cib | xmllint --xpath
> "/cib/status/node_state/@uname" -`
> + if is_cluster; then
> + # get cluster nodes from cluster cib, get interface and ip address
> + nodelist=`pcs cluster cib | xmllint --xpath
> "/cib/status/node_state/@uname" -`
>
> - # nodelist is formed as 'uname="node1" uname="node2"
...
> uname="nodeX"'
> - # we need to convert each to node1, node2 ... nodeX in each iteration
> - for node in ${nodelist}; do
> - # convert $node from 'uname="nodeX"' to 'nodeX'
> - eval $node
> - nodename=$uname
> + # nodelist is formed as 'uname="node1"
uname="node2" ...
> uname="nodeX"'
> + # we need to convert each to node1, node2 ... nodeX in each
> iteration
> + for node in ${nodelist}; do
> + # convert $node from 'uname="nodeX"' to
'nodeX'
> + eval $node
> + xnodes="$xnodes $uname"
> + done
> + else
> + # fence_kdump configured manually
> + read xnodes < $FENCE_KDUMP_NODES
> + fi
Can we create a separate function to get list of nodes. Something like.
nodes = `get_list_of_nodes_to_notify()`
OK, I will create it
And then this function can figure out whether to
> +
> + # install network for each node
> + for node in ${xnodes}; do
> # Skip its own node name
> if [ "$nodename" = `hostname` ]; then
> continue
> fi
> - nodes="$nodes $nodename"
> -
> - kdump_install_net $nodename
> + kdump_install_net $node
> + nodes="$nodes $node"
> done
> +
There should not be any need for both nodes and xnodes if we get list
of nodes using a function.
OK
> echo
>
> - echo "$nodes" > ${initdir}/$FENCE_KDUMP_NODES
> + mkdir -p ${initdir}$FENCE_KDUMP_NODES
> + rmdir ${initdir}$FENCE_KDUMP_NODES
> + echo "$nodes" > ${initdir}$FENCE_KDUMP_NODES
What's that mkdir/rmdir magic. Also why are you not saving nodes into
${initdir}/$FENCE_KDUMP_NODES file?
${initdir}/etc/sysconfig directory doesn't exist yet
Why don't you do.
mkdir -p ${initdir}/etc/sysconfig
echo "$nodes" > ${initdir}$FENCE_KDUMP_NODES
If I do rmdir/mkdir I don't care what's the value of variable. In your
case I need to know that FENCE_KDUMP_NODES contains /etc/sysconfig directory
and then filename with nodes
> +
> dracut_install $FENCE_KDUMP_SEND
> dracut_install -o $FENCE_KDUMP_CONFIG
> }
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 384f7b4..089d40b 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -5,7 +5,7 @@
>
> FENCE_KDUMP_CONFIG="/etc/sysconfig/fence_kdump"
> FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send"
> -FENCE_KDUMP_NODES="/etc/fence_kdump_nodes"
> +FENCE_KDUMP_NODES="/etc/sysconfig/fence_kdump_nodes"
Can we keep this change in a separate patch. This is more of a cleanup
that now fence_kdump_nodes will be under /etc/sysconfig/ instead of /etc.
OK
>
> is_ssh_dump_target()
> {
> @@ -27,15 +27,25 @@ strip_comments()
> echo $@ | sed -e 's/\(.*\)#.*/\1/'
> }
>
> +# Check if cluster environment is present
> +is_cluster()
> +{
> + # no pcs executable installed
> + type -P pcs > /dev/null || return 1
> +}
> +
> # Check if fence kdump is configured in cluster
> is_fence_kdump()
I want to break this down in two functions. See more below.
> {
> - # no pcs or fence_kdump_send executables installed?
> - type -P pcs > /dev/null || return 1
> [ -x $FENCE_KDUMP_SEND ] || return 1
>
> - # fence kdump not configured?
> - (pcs cluster cib | grep -q 'type="fence_kdump"') &>
/dev/null ||
> return 1
> + if is_cluster; then
> + # fence kdump configured in cluster?
> + (pcs cluster cib | grep -q 'type="fence_kdump"')
&> /dev/null ||
> return 1
> + else
> + # fence kdump configured manually?
> + [ -f $FENCE_KDUMP_NODES ] || return 1
> + fi
> }
>
> get_user_configured_dump_disk()
> diff --git a/kdumpctl b/kdumpctl
> index 8d5498a..c310904 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -193,6 +193,7 @@ function check_fence_kdump()
> local cib_time
>
> is_fence_kdump || return 1
> + is_cluster || return 1
This is odd. I can't wrap my head around it. is_fence_kdump() checks
for clsuter internally and then we checek for cluster again.
Previously fence_kdump meant one thing to us and hence we lumped
everything in *_fence_kdump() functions. We need to improve naming now
for better understanding.
How about we distinguis two mechanism using function is_pcs_cluster()
and is_generic_cluster().
pcs cluster requires that pcs utility be installed and fence_kdump_send
be installed.
generic cluster mechanism requires that /etc/sysconfig/fence_kdump_nodes
be present and fence_kdump_send is installed.
So we can break down is_fence_kdump() function in two functions and
replace with is_pcs_cluster() or is_generic_cluster().
Rename check_fence_kdump() to check_pcs_cluster_rebuild().
OK, I will do as you suggest
>
> cib_time=`pcs cluster cib | xmllint --xpath
> 'string(/cib/@cib-last-written)' - | \
> xargs -0 date +%s --date`
> @@ -253,6 +254,10 @@ function check_rebuild()
> files="$files $FENCE_KDUMP_CONFIG"
> fi
>
> + if [ -f $FENCE_KDUMP_NODES ]; then
> + files="$files $FENCE_KDUMP_NODES"
> + fi
And above can be equivalent of.
if (is_generic_cluster())
files = "$files $FENCE_KDUMP_NODES"
if
Thanks
Vivek