Re: [PATCH v6] Enhance kdump to support ipv6

[WANG Chao]

On 06/30/14 at 05:45pm, Zhi Zou wrote:
> Resolves: bz821620
>
> Description:
> Currently kdump doesn't support ipv6 nfs/ssh dump. Ipv6 is the latest version
> of the Internet Protocal. so it is a significant feture for kdump to enhance
> to support ipv6.
>
> Solutions:
> Since dracut has supported ipv6 now, it is easy to change the kdump code to
> support ipv6. Just need pass the right _ip_opts to the second kernle. What is
> the main difference in userspace bettwen ipv4 and ipv6 is the ip address format.
> For ipv6 nfs dump:
>     if ipv6 address type is link scope, /etc/kdump.conf should be edited like
> "nfs [fe80::5054:ff:fe48:ca80%eth0]:/mnt"
>     else /etc/kdump.conf should be edited like "nfs [2001:db8:0:f101::2]:/mnt"
> For ipv6 ssh dump
>     if ipv6 address type is link scope, /etc/kdump.conf should be edited like
> "ssh root@fe80::5054:ff:fe48:ca80%eth0"
>     else /etc/kdump.conf should be edited like "ssh root@2001:db8:0:f101::2"
>
> What this patch do is:
>     a): Add a function is_ipv6_target to tell what version of Internet Protocal
>         (ipv4/ipv6) kdump will use to dump to remote target.
>     b): Modify kdump_install_net to handle ipv6 configuration in /etc/kdump.conf
>         correctly. Get the ipv6 address from /etc/kdump.conf is more complicated
>         than ipv4 because the difference configuration format mentioned above.
>     c): Based on the ip address type, using corresponding ip address as HOST_IP
>         in second kernel.
>
> It was tested for IPV6 and IPV4 address in beaker machine and passed.
>
> Note:
>     1): Currntly only f19 support remount a nfs target in ipv6. detail in https:
>         //bugzilla.redhat.com/show_bug.cgi?id=1099761. If using in f20, you can
>         comment out "mount -o remount,rw $_mp || return 1" in kdump.sh line 105.
>     2): If Using static ipv6 address and configuring nfs/ssh with hostname of
>         remote target, MUST add a ipv6 DNS in ifcfg-devname.
>
> How to create a ipv6 enviromnet.
>     1): Reserving two beaker machine with family fedora19.
>     2): Choosing a beaker machine as a nfs/ssh server and delete it's ipv4 address
>         by "ip address del ipv4-address dev nicname"
>     3): Configuring the /etc/kdump.conf like mentioned above.
> Signed-off-by: Arthur Zou
>
> dracut-kdump.sh        |  9 ++++--
>  dracut-module-setup.sh | 74 ++++++++++++++++++++++++++++++++++++++------------
>  kdump-lib.sh           | 25 +++++++++++++++++
>  3 files changed, 89 insertions(+), 19 deletions(-)
>
> diff --git a/dracut-kdump.sh b/dracut-kdump.sh
> index cb13d92..5dcd8a4 100755
> --- a/dracut-kdump.sh
> +++ b/dracut-kdump.sh
> @@ -206,9 +206,14 @@ get_host_ip()
>      then
>          kdumpnic=$(getarg kdumpnic=)
>          [ -z "$kdumpnic" ] && echo "kdump: failed to get kdumpnic!" && return 1
>    _host=`ip addr show dev $kdumpnic|grep 'inet '`
>    if is_ipv6_target
>    then
>        _host=`ip addr show dev $kdumpnic|grep 'inet6'`
>    else
>        _host=`ip addr show dev $kdumpnic|grep 'inet '`
>    fi

You know what, I think I'd like the following if-else coding style:

if is_ipv6_target; then
else
fi

I think It's more compact and will save you lots of newline in you whole
patch.
>      [ $? -ne 0 ] && echo "kdump: wrong kdumpnic: $kdumpnic" && return 1
>    _host="${_host##*inet }"
>    _host=`echo $_host | cut -d' ' -f2`
>    _host="${_host%%/*}"
>    [ -z "$_host" ] && echo "kdump: wrong kdumpnic: $kdumpnic" && return 1
>    HOST_IP=$_host
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index 87ad072..93ff259 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -70,14 +70,25 @@ kdump_setup_dns() {
>  #$2: srcaddr
>  #if it use static ip echo it, or echo null
>  kdump_static_ip() {
> local _netmask _gateway
> local _netmask _gateway _ipaddr
> local _netdev="$1" _srcaddr="$2"
> local _ipaddr=$(ip addr show dev $_netdev permanent | \
> if is_ipv6_target
> then
>    _ipaddr=$(ip addr show dev $_netdev permanent | \
>                awk "/ $_srcaddr\/.* /{print \$2}")
>    if [ -n "$_ipaddr" ]; then
>       _gateway=$(ip -6 route list dev $_netdev | awk '/^default /{print $3}')
>       echo -n "[${_srcaddr}]::[${_gateway}]:64::"
>    fi
> else
>    _ipaddr=$(ip addr show dev $_netdev permanent | \
>                awk "/ $_srcaddr\/.* $_netdev\$/{print \$2}")
> if [ -n "$_ipaddr" ]; then
>   _netmask=$(ipcalc -m $_ipaddr | cut -d'=' -f2)
>   _gateway=$(ip route list dev $_netdev | awk '/^default /{print $3}')
>   echo -n "${_srcaddr}::${_gateway}:${_netmask}::"
>    if [ -n "$_ipaddr" ]; then
>       _netmask=$(ipcalc -m $_ipaddr | cut -d'=' -f2)
>       _gateway=$(ip route list dev $_netdev | awk '/^default /{print $3}')
>       echo -n "${_srcaddr}::${_gateway}:${_netmask}::"
>    fi
> fi
>
> }
>
> @@ -231,24 +242,51 @@ kdump_install_net() {
>      local _server _netdev _srcaddr
>      local config_val="$1"
> _server=`echo $config_val | sed 's/.*@//' | cut -d':' -f1`
> # ipv6 config_val is like mnt [xxxx:xxxx::xxxx%eth0]:/mnt/nfs or
> # ssh username@xxxx:xxxx::xxxx%eth0, what we need is xxxx:xxxx::xxxx
> _server=`echo $config_val | sed 's/.*@//'`
> _server=${_server%:/*}
> _server=${_server#[}
> _server=${_server%]}
> _server=${_server%%*}              #strip the zone_ID if exist, like %eth0 above

newline the comment.

Above code is used twice in your patch. Try to sum up a common function.

[..]

Thanks
WANG Chao