On 01/04/15 at 06:59pm, Minfei Huang wrote:
On 12/29/14 at 03:57pm, Dave Young wrote:
> On 12/15/14 at 11:20am, Minfei Huang wrote:
> > We have to get the network config by using ip route when setup the net
> > or iscsi. So wrap the similar functional code to the function to make
> > the kdump code clear.
> >
> > Call the function kdump_setup_currect_net, we can setup the net or iscsi
> > whichever you want.
> >
> > Signed-off-by: Minfei Huang <mhuang(a)redhat.com>
> > ---
> > dracut-module-setup.sh | 73
+++++++++++++++++++++++++-------------------------
> > 1 file changed, 36 insertions(+), 37 deletions(-)
> >
> > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> > index c704a65..10f7c83 100755
> > --- a/dracut-module-setup.sh
> > +++ b/dracut-module-setup.sh
> > @@ -261,24 +261,15 @@ kdump_setup_netdev() {
> > kdump_setup_dns "$_netdev"
> > }
> >
> > -#Function:kdump_install_net
> > -#$1: config values of net line in kdump.conf
> > -#$2: srcaddr of network device
> > -kdump_install_net() {
> > - local _server _netdev _srcaddr
> > - local config_val="$1"
> > -
> > - _server=`echo $config_val | sed 's/.*@//' | cut -d':'
-f1`
> > -
> > - _need_dns=`echo $_server|grep "[a-zA-Z]"`
> > - [ -n "$_need_dns" ] && _server=`getent hosts
$_server|cut -d' ' -f1`
> > +kdump_setup_currect_net() {
> > + local _server=$1 _netdev _srcaddr
> > + local _type=$2
> >
> > _netdev=`/sbin/ip route get to $_server 2>&1`
> > [ $? != 0 ] && echo "Bad kdump location: $config_val"
&& exit 1
> >
> > #the field in the ip output changes if we go to another subnet
> > - if [ -n "`echo $_netdev | grep via`" ]
> > - then
> > + if [ -n "`echo $_netdev | grep via`" ]; then
> > # we are going to a different subnet
> > _srcaddr=`echo $_netdev|awk '{print $7}'|head -n 1`
> > _netdev=`echo $_netdev|awk '{print $5;}'|head -n 1`
> > @@ -290,20 +281,36 @@ kdump_install_net() {
> >
> > kdump_setup_netdev "${_netdev}" "${_srcaddr}"
"${_server}"
> >
> > - #save netdev used for kdump as cmdline
> > - # Whoever calling kdump_install_net() is setting up the default gateway,
> > - # ie. bootdev/kdumpnic. So don't override the setting if calling
> > - # kdump_install_net() for another time. For example, after setting eth0
as
> > - # the default gate way for network dump, eth1 in the fence kdump path
will
> > - # call kdump_install_net again and we don't want eth1 to be the
default
> > - # gateway.
> > - if [ ! -f ${initdir}${initdir}/etc/cmdline.d/60kdumpnic.conf ] &&
> > - [ ! -f ${initdir}/etc/cmdline.d/70bootdev.conf ]; then
> > - echo "kdumpnic=$(kdump_setup_ifname $_netdev)" >
${initdir}/etc/cmdline.d/60kdumpnic.conf
> > - echo "bootdev=$(kdump_setup_ifname $_netdev)" >
${initdir}/etc/cmdline.d/70bootdev.conf
> > + if [ $_type = "net" ]; then
>
> I do not understand the _type, what is it for? is there other type which is not
net?
>
> If this is new code, please split the patch to two patches, one is cleanup and
> add common function, the other is for adding new functionalities.
>
For some networks, we would specify the config after setuping the network
device. Here the kdump code, we can make the parameter _type "net" or
"iscsi".
> > + #save netdev used for kdump as cmdline
> > + # Whoever calling kdump_install_net() is setting up the default
gateway,
> > + # ie. bootdev/kdumpnic. So don't override the setting if calling
> > + # kdump_install_net() for another time. For example, after setting
eth0 as
> > + # the default gate way for network dump, eth1 in the fence kdump path
will
> > + # call kdump_install_net again and we don't want eth1 to be the
default
> > + # gateway.
> > + if [ ! -f ${initdir}${initdir}/etc/cmdline.d/60kdumpnic.conf ]
&&
> > + [ ! -f ${initdir}/etc/cmdline.d/70bootdev.conf ]; then
> > + echo "kdumpnic=$(kdump_setup_ifname $_netdev)" >
${initdir}/etc/cmdline.d/60kdumpnic.conf
> > + echo "bootdev=$(kdump_setup_ifname $_netdev)" >
${initdir}/etc/cmdline.d/70bootdev.conf
> > + fi
> > fi
> > }
> >
> > +#Function:kdump_install_net
> > +#$1: config values of net line in kdump.conf
> > +#$2: srcaddr of network device
>
> It is only using one argument, so since you are modifying it please
> update the comment. Also: s/net line/ssh|nfs line
>
Will comment it here.
> > +kdump_install_net() {
> > + local config_val="$1" _server _need_dns
> > +
> > + _server=`echo $config_val | sed 's/.*@//' | cut -d':'
-f1`
> > +
> > + _need_dns=`echo $_server|grep "[a-zA-Z]"`
>
> In case ipv6 addresses which contains [a-zA-Z] it does not need dns convertion...
>
It does not matter if we filte ipv6 address again. Otherwise we should
s/filte/file?
varify the $_server value, domain/hostname or not.
But as for this function, it is not good, a variable _need_dns depends on
code in other functions will twist the code. Make the variable defination clear
is better to avoid bugs.
> > + [ -n "$_need_dns" ] && _server=`getent hosts
$_server|cut -d' ' -f1`
>
> I remember getent hosts was replaced by getent ahosts, right?
Yes. will modify it.
>
> > +
> > + kdump_setup_currect_net $_server "net"
>
> kdump-setup_currect_net looks not good, I assume you means 'correct'?
> How about _kdump_setup_net or just kdump_setup_net()?
>
> I'm confused with the second argument, see previous comment.
>
Will name the function properly.
> > +}
> > +
> > default_dump_target_install_conf()
> > {
> > local _target _fstype
> > @@ -434,13 +441,10 @@ kdump_get_iscsi_initiator() {
> > # No ibft handling yet.
> > kdump_setup_iscsi_device() {
> > local path=$1
> > - local tgt_name; local tgt_ipaddr;
> > - local username; local password; local userpwd_str;
> > - local username_in; local password_in; local userpwd_in_str;
> > - local netdev
> > - local srcaddr
> > - local idev
> > - local netroot_str ; local initiator_str;
> > + local tgt_name tgt_ipaddr
> > + local username password userpwd_str
> > + local username_in password_in userpwd_in_str
> > + local netroot_str initiator_str
> > local netroot_conf="${initdir}/etc/cmdline.d/50iscsi.conf"
> > local initiator_conf="/etc/iscsi/initiatorname.iscsi"
> >
> > @@ -472,12 +476,7 @@ kdump_setup_iscsi_device() {
> >
> > [ -n "$username_in" ] &&
userpwd_in_str=":$username_in:$password_in"
> >
> > - netdev=$(/sbin/ip route get to ${tgt_ipaddr} | \
> > - sed 's|.*dev \(.*\).*|\1|g')
> > - srcaddr=$(echo $netdev | awk '{ print $3; exit }')
> > - netdev=$(echo $netdev | awk '{ print $1; exit }')
> > -
> > - kdump_setup_netdev $netdev $srcaddr $tgt_ipaddr
> > + kdump_setup_currect_net $tgt_ipaddr "iscsi"
>
> Hmm, looks like the type becomes iscsi here, if this is the case then how about
> doing something like below:
>
> iscsi path:
> -> kdump_setup_net() $tgt_ipaddr
>
> net path:
> -> kdump_setup_net() $_server
> -> kdump_setup_kdumpnic_bootdev() _server
>
In net path, we need the $_netdev value which gets in the
kdump_setup_currect_net. So it is better wrap in one function.
I understand your purpose to add a common function, but it introduced more confuses
the two types makes the logic more complicated.
> >
> > # prepare netroot= command line
> > # FIXME: IPV6 addresses require explicit [] around $tgt_ipaddr
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > kexec mailing list
> > kexec(a)lists.fedoraproject.org
> >
https://lists.fedoraproject.org/mailman/listinfo/kexec