-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enhance bmcsetup logic for openbmc #2990
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,6 +112,13 @@ fi | |
# Add ipmi_devintf module to allow the ipmitool operation in-band | ||
modprobe ipmi_devintf | ||
|
||
for parm in `cat /proc/cmdline`; do | ||
key=`echo $parm|awk -F= '{print $1}'` | ||
if [ "$key" = "xcatd" ]; then | ||
XCATMASTER=`echo $parm|awk -F= '{print $2}'|awk -F: '{print $1}'` | ||
fi | ||
done | ||
|
||
allowcred.awk & | ||
CREDPID=$! | ||
sleep 5 | ||
|
@@ -161,6 +168,7 @@ logger -s -t $log_label -p local4.info "IPMIVER=$IPMIVER, IPMIMFG=$IPMIMFG, XPRO | |
# | ||
# IPMIMFG=2 = IBM | ||
# IPMIMFG=0 = OpenPower | ||
# IPMIMFG=42817 and XPROD=16975 = OpenBMC | ||
# | ||
if [ "$IPMIMFG" == 2 ]; then #IBM | ||
if [ "$XPROD" == "220" ]; then | ||
|
@@ -258,13 +266,20 @@ elif [ "$IPMIMFG" == "674" ]; then # DELL | |
ipmitool delloem lan set shared with lom$BMCPORT &>/dev/null | ||
ipmitool delloem lan set shared with failover all loms &>dev/null | ||
fi | ||
elif [ "$IPMIMFG" = "42817" -a "$XPROD" = "16975" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment here, |
||
ISOPENBMC=1 | ||
fi | ||
|
||
if [ -z "$ISOPENBMC" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest that we test for OpenBMC and override the default, and be consistent with this. So default to non-openbmc and then override for OpenBMC always... Example:
However, it looks like nothing else is defined at this time to put into this statement, but perhaps it will set the stage for future additions. |
||
LAN_MED_TYPE="802.3" | ||
else | ||
LAN_MED_TYPE="Other LAN" | ||
fi | ||
while [ -z "$LANCHAN" ]; do | ||
logger -s -t $log_label -p local4.info "Auto detecting LAN channel..." | ||
for TLANCHAN in {1..16}; do | ||
# Try to get the channel information; then get the MAC which is used for the channel | ||
if ipmitool channel info $TLANCHAN 2> /dev/null | grep 802.3 > /dev/null 2>&1 && ipmitool raw 0xc 2 $TLANCHAN 5 0 0 > /dev/null 2>&1; then | ||
if ipmitool channel info $TLANCHAN 2> /dev/null | grep "$LAN_MED_TYPE" > /dev/null 2>&1 && ipmitool raw 0xc 2 $TLANCHAN 5 0 0 > /dev/null 2>&1; then | ||
LANCHAN=$TLANCHAN | ||
break; | ||
fi; | ||
|
@@ -279,6 +294,9 @@ logger -s -t $log_label -p local4.info "Detected LAN channel $LANCHAN" | |
|
||
let idev=NUMBMCS | ||
if [ $IPCFGMETHOD="static" ]; then | ||
if [ ! -z "$ISOPENBMC" ]; then | ||
let idev=0 | ||
fi | ||
while [ $idev -gt 0 ]; do | ||
let idev=idev-1 | ||
TRIES=0 | ||
|
@@ -334,7 +352,11 @@ if [ $IPCFGMETHOD="static" ]; then | |
done | ||
fi | ||
else | ||
let idev=NUMBMCS | ||
if [ -z "$ISOPENBMC" ];then | ||
let idev=NUMBMCS | ||
else | ||
let idev=0 | ||
fi | ||
while [ $idev -gt 0 ]; do | ||
let idev=idev-1 | ||
TRIES=0 | ||
|
@@ -363,6 +385,38 @@ for b in $BMCVLAN; do | |
let idev=idev+1 | ||
done | ||
|
||
|
||
|
||
# update the node status to 'bmcready' for openbmc, no more configuration is needed. | ||
if [ ! -z "$ISOPENBMC" ]; then | ||
# To enable network configuration for openbmc | ||
let idev=0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to set this |
||
TRIES=0 | ||
bmc_config_rc=0 | ||
# Set Channel Access to apply network setting | ||
#while ! ipmitool -d $idev lan set $LANCHAN access on; do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line should be deleted. |
||
while ! ipmitool -d $idev raw 0x06 0x40 $LANCHAN 0x42 0x44 > /dev/null; do | ||
snooze | ||
let TRIES=TRIES+1 | ||
if [ $TRIES -gt $TIMEOUT ]; then | ||
bmc_config_rc=1 | ||
break; | ||
fi | ||
done | ||
|
||
# update the node status to 'bmcready' | ||
if [ ! -z "$XCATMASTER" ]; then | ||
if [ "$bmc_config_rc" = "0" ]; then | ||
# Wait for some time for the new network setting is ready | ||
snooze | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we check with firmware team about this snooze, it will default to 1 second, is it even needed? |
||
updateflag.awk $XCATMASTER 3002 "installstatus bmcready" | ||
else | ||
updateflag.awk $XCATMASTER 3002 "installstatus failed" | ||
fi | ||
fi | ||
rm -f /tmp/ipmicfg.xml | ||
exit $bmc_config_rc | ||
fi | ||
# After network commands are issued, pause to allow the BMC to apply (OpenPower) | ||
snooze | ||
|
||
|
@@ -587,12 +641,6 @@ while [ $idev -gt 0 ]; do | |
cold_reset_bmc | ||
|
||
# update the node status to 'bmcready' | ||
for parm in `cat /proc/cmdline`; do | ||
key=`echo $parm|awk -F= '{print $1}'` | ||
if [ "$key" = "xcatd" ]; then | ||
XCATMASTER=`echo $parm|awk -F= '{print $2}'|awk -F: '{print $1}'` | ||
fi | ||
done | ||
if [ ! -z "$XCATMASTER" ]; then | ||
updateflag.awk $XCATMASTER 3002 "installstatus bmcready" | ||
fi | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,10 +29,17 @@ for LANCHAN in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16; do | |
done | ||
BMCMAC=`ipmitool lan print $LANCHAN|grep ^MAC|awk '{print $4}'` #bmcconfig may opt to use DHCP, if so we need to feed up the mac address | ||
#TODO: need a way to get the DUID the service processor may use, perhaps reserve that for 'ibmsetup' since spec doesn't touch ipv6? | ||
|
||
IPMIMFG=`ipmitool mc info |grep "^Manufacturer ID"|awk '{print $4}'` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we consider having a common file to move some of the common functions into another place?
If we source it in another file, we will get the variables set....
Result:
One downside is that we can't just use relative path |
||
XPROD=`ipmitool mc info | grep "^Product ID"|awk '{print $4}'` | ||
if [ "$IPMIMFG" == "42817" -a "$XPROD" == "16975" ]; then | ||
ISOPENBMC=1 | ||
else | ||
ISOPENBMC=0 | ||
fi | ||
echo "<xcatrequest> | ||
<command>getbmcconfig</command> | ||
<callback_port>300</callback_port> | ||
<isopenbmc>$ISOPENBMC</isopenbmc> | ||
<bmcmac>$BMCMAC</bmcmac> | ||
</xcatrequest>" > /tmp/bmcreq.xml | ||
rm -f /tmp/ipmicfg.xml | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,20 +96,24 @@ sub process_request { | |
my $request = shift; | ||
my $callback = shift; | ||
my $node = $request->{'_xcat_clienthost'}->[0]; | ||
my $open_table = "ipmi"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This variable name doesn't really make sense.. "open table"... Suggest |
||
if ($request->{isopenbmc}->[0]) { | ||
$open_table = "openbmc"; | ||
} | ||
unless (ok_with_node($node, 300)) { | ||
$callback->({ error => ["Unable to prove root on your IP approves of this request"], errorcode => [1] }); | ||
return; | ||
} | ||
|
||
#my $sitetable = xCAT::Table->new('site'); | ||
my $ipmitable = xCAT::Table->new('ipmi'); | ||
my $ipmitable = xCAT::Table->new("$open_table"); | ||
my $tmphash; | ||
my $username; | ||
my $gennedpassword = 0; | ||
my $bmc; | ||
my $password; | ||
$tmphash = $ipmitable->getNodesAttribs([$node], [ 'bmc', 'username', 'bmcport', 'password', 'taggedvlan' ]); | ||
my $authmap = xCAT::PasswordUtils::getIPMIAuth(noderange => [$node], ipmihash => $tmphash); | ||
my $authmap = xCAT::PasswordUtils::getIPMIAuth(noderange => [$node], ipmihash => $tmphash, keytype => $open_table); | ||
|
||
if ($::XCATSITEVALS{genpasswords} eq "1" or $::XCATSITEVALS{genpasswords} =~ /y(es)?/i) { | ||
$password = genpassword(10) . "1cA!"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is very important, but in some cases in this file we use strings for comparisons and in some cases integers. Should we make this consistent:
if [ "$xxxx" == "yyy" ]
if [ "$xxxx" == yyy ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @gurevichmark , thx very much for the comments. You are right we shall keep the consistent. I do that for the OpenBMC part.
But for others, UT is needed if there is any modifications. But I don't thnk we need to put too much effort to verify it on multiple kind of servers, right? What is your idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree. Maybe just change the code that gets executed for this PR.