On 01/05/15 at 10:19am, Dave Young wrote:
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.
Yes, there are several trival patches to support ipv6. It is more
confusing, if we patch it single.
>
> > > _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.
We may consider the case which the hostname/domain is only support ipv6
address. So we will parse the target value which is the domain/hostname
only supported by ipv6 address in the is_ipv6_target function.
>
> > > + 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
>
>