Adding support for manually configured fence_kdump v2

[Vivek Goyal]

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.


> 
> Thanks
> 
> Martin Perina
> 
> 
> From 48f09ed8cda2c1fe52b0a48eb501373cd40d6b8f Mon Sep 17 00:00:00 2001
> From: Martin Perina <mperina at 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.

> 
> it's supposed that fence_kdump is configured manually.
> 
> Signed-off-by: Martin Perina <mperina at 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?

>  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()`

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.

>      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?

Why don't you do.

mkdir -p ${initdir}/etc/sysconfig
echo "$nodes" > ${initdir}$FENCE_KDUMP_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.

>  
>  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().

>  
>  	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