[warm reboot] Warm Reboot Support for EVPN VXLAN #350
Conversation
1. Added new enums to WarmStartState 2. Added a new function to get the warm start state. Refer to HLD: sonic-net/SONiC#437
|
Request review from @dzhangalibaba , @zhenggen-xu |
common/warm_restart.cpp
Outdated
| @@ -181,6 +181,41 @@ bool WarmStart::isSystemWarmRebootEnabled(void) | |||
| return warmStart.m_systemWarmRebootEnabled; | |||
| } | |||
|
|
|||
| void WarmStart::getWarmStartState(const std::string &app_name, WarmStartState &state) | |||
| { | |||
| //WarmStartStateNameMap::iterator it; | |||
There was a problem hiding this comment.
//WarmStartStateNameMap::iterator it; [](start = 4, length = 37)
Remove commented code #Closed
| @@ -181,6 +181,41 @@ bool WarmStart::isSystemWarmRebootEnabled(void) | |||
| return warmStart.m_systemWarmRebootEnabled; | |||
| } | |||
|
|
|||
| void WarmStart::getWarmStartState(const std::string &app_name, WarmStartState &state) | |||
There was a problem hiding this comment.
WarmStartState [](start = 63, length = 14)
Why not just return this state? #WontFix
There was a problem hiding this comment.
Don't see any issue with pass by reference. Will consider that while updating other files where this api is used.
common/warm_restart.cpp
Outdated
| */ | ||
| state = WSUNKNOWN; | ||
|
|
||
| for(std::map<WarmStart::WarmStartState, std::string>::const_iterator it = warmStartStateNameMap.begin(); it != warmStartStateNameMap.end(); it++) |
There was a problem hiding this comment.
std::map<WarmStart::WarmStartState, std::string>::const_iterator [](start = 8, length = 64)
auto? #Closed
common/warm_restart.cpp
Outdated
| { | ||
| state = it->first; | ||
| break; | ||
| } |
There was a problem hiding this comment.
[](start = 4, length = 2)
Use 4x spaces as indentation #Closed
| /* If warm-start is enabled, state cannot be assumed as Reconciled | ||
| * It should be set to unknown | ||
| */ | ||
| state = WSUNKNOWN; |
There was a problem hiding this comment.
state = WSUNKNOWN; [](start = 4, length = 18)
Is it expected state in runtime? If not, just throw an exception. #Closed
There was a problem hiding this comment.
Yes, this is expected. When Module A check for the status of Module B, module B may not have reached/initialised to any state yet. Hence module A needs to know that Module B is not ready yet an act accordingly.
Addressed review comments
common/warm_restart.cpp
Outdated
|
|
||
| state = RECONCILED; | ||
|
|
||
| if(!isWarmStart()) |
There was a problem hiding this comment.
Please have a space after if, also with open/close braces. Please do it for the rest of the if and for below
Addressed review comments
Refer to HLD: sonic-net/SONiC#437