Hi Anders Widell,
Thanks for the review , I will incorporate your comments and will
re-publish the patch V1.
Please find my answers below tagged with [AVM].
On 9/2/2016 5:10 PM, Anders Widell wrote:
> Hi!
>
> Comments / questions:
>
> 1) When I run "make shellcheck" I get the following warnings on the
> modified code. These warnings should be fixed.
>
> In configure_tipc.in line 269:
> if [ "$configured_tipc_addr" == "0.0.0" ]; then
> ^-- SC2039: In POSIX sh, == in
> place of = is undefined.
>
>
> In configure_tipc.in line 280:
> configured_net_id=`tipc-config -netid | cut -d: -f2`
> ^-- SC2006: Use $(..) instead of legacy `..`.
>
>
> In configure_tipc.in line 282:
> if [ $configured_net_id != $opensaf_net_id ]; then
> ^-- SC2086: Double quote to prevent globbing and word
> splitting.
> ^-- SC2086: Double quote to prevent
> globbing and word splitting.
>
> 2) The line " tipc_duplicate_node_detect" has a
> three-space indentation. It should be four spaces.
>
> 3) The line "configured_net_id=`tipc-config -netid | cut -d: -f2`"
> calls the tipc-config tool directly, whereas other parts of this
> script call it using the ${tipc_config} variable. You should also call
> it using the variable.
[AVM] I will incorporate the "make shellcheck" warnings .
>
> 4) In the case where TIPC is build into the kernel, you are calling
> the functions tipc_duplicate_node_detect and tipc_configure. Both of
> these functions try to insert and/or remove the TIPC kernel module. I
> suppose this will fail, right? What is the exit code from modprobe and
> insmod when the kernel has TIPC built-in (and maybe the kernel doesn't
> even support loadable modules)? tipc_configure will exit the script if
> the exit code from modprobe/insmod is non-zero. I suppose duplicate
> address detection shouldn't be done at all when TIPC is built-in.
> Maybe we need to extract the TIPC module insertion from the
> tipc_configure function, so that it is not done when TIPC is built-in?
[AVM] In case of TIPC node address not yet configured (tipc_addr =
"0.0.0" ) is equal to where TIPC support loadable modules
so we required to check duplicate address detection
even in case of in case of kernel has TIPC built-in .
I missed to test
`DUPLICATE_NODE_DETECT=${TIPC_DUPLICATE_NODE_DETECT:-"YES"}` case , i
will re-test and
will publish the patch again.
-AVM
>
> / Anders Widell
>
> On 09/02/2016 08:36 AM, [email protected] wrote:
>> osaf/services/infrastructure/nid/scripts/configure_tipc.in | 38
>> ++++++++-----
>> 1 files changed, 23 insertions(+), 15 deletions(-)
>>
>>
>> Issue:
>> In some OS like Montavista, TIPC is built in kernel module,
>> the default node address will be <0.0.0>.
>>
>> Fix :
>> This patch determines such OS which has TIPC is built in kernel module
>> and configures the TIPC node address to OpenSAF requirements.
>>
>> Note: In OS like Montavista where TIPC is built in kernel module,
>> once TIPC has joined a network with a network id & address
>> it cannot change node address once assigned, until node reboots,
>> in other words opensafd stop will not Reset tipc link/link Lost.
>> the only way to get Reset tipc link/link Lost is rebooting node.
>>
>> diff --git
>> a/osaf/services/infrastructure/nid/scripts/configure_tipc.in
>> b/osaf/services/infrastructure/nid/scripts/configure_tipc.in
>> --- a/osaf/services/infrastructure/nid/scripts/configure_tipc.in
>> +++ b/osaf/services/infrastructure/nid/scripts/configure_tipc.in
>> @@ -266,22 +266,30 @@ else
>> configured_tipc_addr=`tipc-config -addr | tr -s '<>' % | cut
>> -d% -f2`
>> opensaf_tipc_addr=1.1.$TIPC_NODEID
>> if [ $configured_tipc_addr != $opensaf_tipc_addr ]; then
>> - logger -t opensaf -s "TIPC node address not configured to
>> OpenSAF requirements, exiting..."
>> - exit 1
>> - fi
>> + if [ "$configured_tipc_addr" == "0.0.0" ]; then
>> + logger -t opensaf -s "TIPC node address not yet
>> configured , Configuring..."
>> + if [ "$DUPLICATE_NODE_DETECT" = "YES" ]; then
>> + tipc_duplicate_node_detect
>> + fi
>> + tipc_configure
>> + else
>> + logger -t opensaf -s "TIPC node address not configured
>> to OpenSAF requirements, Exiting..."
>> + exit 1
>> + fi
>> + else
>> + configured_net_id=`tipc-config -netid | cut -d: -f2`
>> + opensaf_net_id=$TIPC_NETID
>> + if [ $configured_net_id != $opensaf_net_id ]; then
>> + logger -t opensaf -s "TIPC network ID not configured to
>> OpenSAF requirements, exiting..."
>> + exit 1
>> + fi
>> - configured_net_id=`tipc-config -netid | cut -d: -f2`
>> - opensaf_net_id=$TIPC_NETID
>> - if [ $configured_net_id != $opensaf_net_id ]; then
>> - logger -t opensaf -s "TIPC network ID not configured to
>> OpenSAF requirements, exiting..."
>> - exit 1
>> - fi
>> -
>> - configured_bearers=$(${tipc_config} -b | grep -v Bearer | cut
>> -d: -f2 | sort)
>> - opensaf_bearers=$(echo "$ETH_NAME" | tr "," "\n" | sort)
>> - if [ "$configured_bearers" != "$opensaf_bearers" ]; then
>> - logger -t opensaf -s "TIPC bearer not configured to OpenSAF
>> requirements, exiting..."
>> - exit 1
>> + configured_bearers=$(${tipc_config} -b | grep -v Bearer |
>> cut -d: -f2 | sort)
>> + opensaf_bearers=$(echo "$ETH_NAME" | tr "," "\n" | sort)
>> + if [ "$configured_bearers" != "$opensaf_bearers" ]; then
>> + logger -t opensaf -s "TIPC bearer not configured to
>> OpenSAF requirements, exiting..."
>> + exit 1
>> + fi
>> fi
>> fi
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
