On 07/22/14 at 11:14am, Vivek Goyal wrote:
On Tue, Jul 22, 2014 at 05:10:14PM +0800, WANG Chao wrote:
[..]
> kdump_setup_bridge() {
> local _netdev=$1
> - local _brif _dev
> + local _brif _dev _mac _kdumpdev
> for _dev in `ls /sys/class/net/$_netdev/brif/`; do
> if kdump_is_bond "$_dev"; then
> kdump_setup_bond "$_dev"
> @@ -108,20 +124,30 @@ kdump_setup_bridge() {
> elif kdump_is_vlan "$_dev"; then
> kdump_setup_vlan "$_dev"
> else
> - echo -n " ifname=$_dev:$(kdump_get_mac_addr $_dev)" >>
${initdir}/etc/cmdline.d/41bridge.conf
> + _mac=$(kdump_get_mac_addr $_dev)
> + _kdumpdev=$(kdump_setup_ifname $_dev)
> + echo -n " ifname=$_kdumpdev:$_mac" >>
${initdir}/etc/cmdline.d/41bridge.conf
> + fi
> + if [ -n "$_kdumpdev" ]; then
> + _brif+="$_kdumpdev,"
> + else
> + _brif+="$_dev,"
> fi
> - _brif+="$_dev,"
> + _kdumpdev=""
When will _kdumpdev be zero? IOW, as per your logic kdumpdev will either
be same as _dev or kdump-$dev. So why are you putting this extra if loop?
IOW, why not simply do.
_brif+="$_kdumpdev"
Because it is if-else statement to handle different types of devices.
For bond/team/vlan, we don't modify their names, so we use $_dev. For
ethernet interfaces, we modify their names, so we use $_kdumpdev.
And we need to unset $_kdumpdev in the end of each loop, otherwise it
retains the value from last loop.
> done
> echo " bridge=$_netdev:$(echo $_brif | sed -e 's/,$//')"
>> ${initdir}/etc/cmdline.d/41bridge.conf
> }
>
[..]
> kdump_setup_bond() {
> local _netdev=$1
> - local _dev
> + local _dev _mac _slaves _kdumpdev
> for _dev in `cat /sys/class/net/$_netdev/bonding/slaves`; do
> - echo -n " ifname=$_dev:$(kdump_get_perm_addr $_dev)" >>
${initdir}/etc/cmdline.d/42bond.conf
> + _mac=$(kdump_get_perm_addr $_dev)
> + _kdumpdev=$(kdump_setup_ifname $_dev)
> + echo -n " ifname=$_kdumpdev:$_mac" >>
${initdir}/etc/cmdline.d/42bond.conf
> + _slaves+="$_kdumpdev,"
> done
> - echo -n " bond=$_netdev:$(sed -e 's/ /,/g'
/sys/class/net/$_netdev/bonding/slaves)" >>
${initdir}/etc/cmdline.d/42bond.conf
> + echo -n " bond=$_netdev:$(echo $_slaves | sed 's/,$//')"
>> ${initdir}/etc/cmdline.d/42bond.conf
So we are stroing slave names in _slaves instead of reading them from sys
file because we might have renamed them. Ok, makes sense. And you are
getting rid of last ",".
Yep. Totally right.
Thanks
WANG Chao