On 01/04/15 at 07:15pm, Minfei Huang wrote:
On 12/29/14 at 03:24pm, Dave Young wrote:
> Hi, Minfei
>
> A few comments from me, Bao, can you review it as well especially the network part?
>
> On 12/15/14 at 11:20am, Minfei Huang wrote:
> > The ipv6 link scope needs to append the netdevice to identify the ipv6
> > address.
> >
> > Due to add prefix "kdump-" before ethX(commit: ba7660f) in the 2nd
> > kernel, we should correct the mount parameter and /etc/kdump.conf to add
> > the prefix "kdump-" before ethX, if use the ipv6 link scope.
> >
> > Signed-off-by: Minfei Huang <mhuang(a)redhat.com>
> > ---
> > dracut-module-setup.sh | 19 +++++++++++++++++++
> > kdump-lib.sh | 2 +-
> > mkdumprd | 17 ++++++++++++++++-
> > 3 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> > index d65c8c0..c704a65 100755
> > --- a/dracut-module-setup.sh
> > +++ b/dracut-module-setup.sh
> > @@ -363,6 +363,25 @@ kdump_install_conf() {
> > default_dump_target_install_conf
> >
> > kdump_configure_fence_kdump "/tmp/$$-kdump.conf"
> > +
> > + if is_ipv6_target; then
>
> Seems only ipv6 addresses could contains '%', so how about remove the above
check?
>
You means remove the code "if is_ipv6_target"?
Yes.
> > + local _srcaddr
> > + if is_ssh_dump_target; then
> > + _srcaddr=$(get_option_value ssh)
> > + elif is_nfs_dump_target; then
> > + _srcaddr=$(get_option_value nfs)
> > + fi
>
> If call it in the config passing while loop then it will be cleaner.
>
> > +
> > + if [ "x" != "x"$_srcaddr ] && `echo
$_srcaddr | grep -q "%"`; then
> > + local _netdev=${_srcaddr#*\%}
> > + _netdev=${_netdev%]*}
> > + local _pre_netdev=$(kdump_setup_ifname $_netdev)
> > + if [ "x"$_netdev != "x"$_pre_netdev ]; then
> > + sed -i "s#$_netdev#$_pre_netdev#"
"/tmp/$$-kdump.conf"
> > + fi
> > + fi
> > + fi
> > +
>
> Above can become a function like kdump_ipv6_fixup(), call it like below:
>
> while read config_opt config_val;
> do
> [snip]
> ssh|nfs)
> kdump_install_net "$config_val"
> kdump_ipv6_fixup "$config_val"
>
>
Good idea. will use it.
>
> > inst "/tmp/$$-kdump.conf" "/etc/kdump.conf"
> > rm -f /tmp/$$-kdump.conf
> > }
> > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > index f24f08d..b886c5d 100755
> > --- a/kdump-lib.sh
> > +++ b/kdump-lib.sh
> > @@ -146,7 +146,7 @@ check_save_path_fs()
> > kdump_setup_ifname() {
> > local _ifname
> >
> > - if [[ $1 =~ eth* ]]; then
> > + if [[ "$1" =~ eth* ]]; then
>
> It is not relevant to the patch?
>
The parameter may contain the blank, if calls by other functions, like
the function add_dracut_module. So in order to make it correct, quote
the parameter.
Agree, I means it can be in another cleanup patch, but it is trival so
I will not insist.
> > _ifname="kdump-$1"
> > else
> > _ifname="$1"
> > diff --git a/mkdumprd b/mkdumprd
> > index a30d9ca..a066d7b 100644
> > --- a/mkdumprd
> > +++ b/mkdumprd
> > @@ -78,8 +78,23 @@ add_dracut_module() {
> > add_dracut_arg "--add" "$1"
> > }
> >
> > +# The ipv6 link scope which appends the netdevice to identify the ipv6
> > +# address is start with prefix "fe80", so we should correct the
mount
> > +# parameter to add the prefix "kdump-" before ethX.
> > add_dracut_mount() {
> > - add_dracut_arg "--mount" "$1"
> > + local _val="$1"
> > +
> > + if is_ipv6_target && is_nfs_dump_target; then
> > + local _srcaddr=$(get_option_value nfs)
> > + if `echo "$_val" | grep -q "^\[fe80"`; then
>
> fe80.* should be a link-local address, what about non-local addresses?
> Correct me if I'm wrong I think I know little about ipv6, basicly we should
> cover all kinds of ipv6 addresses.
>
See the above comment.
The ipv6 link scope will append the network device to identify the ipv6
address. But non-local address does not need.
We can not filte it using character "%", because the _val value contains
the character "%".
Ok, got your point, but the checking for ipv6 target is still questionable.
Like I commented in another patch, we should only cover ipv6 addresses in kdump.conf
not the hostname case.
> > + local _prefix="${_val%%\%*}"
> > + local _netdev="${_val#*\%}"
> > + _netdev=$(kdump_setup_ifname "$_netdev")
>
> Shouldn't the setup being called in module_setup.sh instead of mkdumprd,
> Is there a better way to handle this in a general function in module_setup.sh?
>
The kdump will use command findmnt to get the SOURCE/MNTPOINT in script
mkdumprd. So we can modify the ipv6 link scope only in the script
mkdumprd.
Ok, make sense then.
> > + _val="$_prefix"%"$_netdev"
> > + fi
> > + fi
> > +
> > + add_dracut_arg "--mount" "$_val"
> > }
> >
> > add_dracut_sshkey() {
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > kexec mailing list
> > kexec(a)lists.fedoraproject.org
> >
https://lists.fedoraproject.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
kexec(a)lists.fedoraproject.org
https://lists.fedoraproject.org/mailman/listinfo/kexec