CISCO: Provide support for platform hooks in syncd.sh #7331
Conversation
| @@ -28,6 +28,10 @@ function startplatform() { | |||
| debug "Firmware update procedure ended" | |||
| fi | |||
|
|
|||
| PLATFORM_DIR=`sonic-cfggen -H -v DEVICE_METADATA.localhost.platform` | |||
There was a problem hiding this comment.
why not use $sonic_asic_platform?
There was a problem hiding this comment.
if we want to do this one, we should inject the hook for all functions startplatform, waitplatform, stopplatform1, and stopplatform2.
There was a problem hiding this comment.
and we should also refactor the existing code to use this platform hook.
There was a problem hiding this comment.
why not use $sonic_asic_platform?
$sonic_asic_platform does not provide platform agnostic approach to otherwise a Platform Independent SONiC code under files/scripts path. Given that this is a run time check, using DEVICE_METADATA seems more as a generic approach
There was a problem hiding this comment.
and we should also refactor the existing code to use this platform hook.
I agree with refactoring the existing code. The whole code has vendor hooks and is not maintainable. I can look into that approach.
Can that be considered as a different exercise given we have dependencies with other vendors using this code.
There was a problem hiding this comment.
startplatform, waitplatform, stopplatform1, and stopplatform2.
Presently, we need platform hooks for startplatform() API only. This is an easily extendable hook for any other function. We see an opportunity to restructure files/scripts/syncd.sh to pursue the following instead of making vendor specific calls from each function:
function startplatform () {
F="/usr/share/sonic/device/$PLATFORM_DIR/scripts/startplatform_routines.sh"
[ -x $F ] && $F start $DEV
}
function stopplatform() {
F="/usr/share/sonic/device/$PLATFORM_DIR/scripts/stopplatform_routines.sh"
[ -x $F ] && $F start $DEV
}
...
PLATFORM_DIR=sonic-cfggen -H -v DEVICE_METADATA.localhost.platform
case "$1" in
start|wait|stop)
$1
;;
*)
echo "Usage: $0 {start|wait|stop}"
exit 1
;;
esac
you mean 'add comments'. Done. |
|
Hi Guhon, please let me know if there is anything to be addressed further. |
|
PR Scrum meeting 5/4: |
|
Hi @lguohan - please review and approve, thanks. |
|
Closing ; Tracked via #7635 |
Why I did it
This is a platform agnostic & vendor agnostic change to files/scripts/syncd.sh to check if 'platform' entry exists in the DEVICE_METADATA table. If it exits it checks if there is a file to source and hence does the needful.
How to verify it
This is a PR followup from the code-review discussion we had with Guhon/Rita and team. This also addresses one of the comment that was raised to convert to sourcing a file instead of executing the same., so the logic within is run in the same shell.
How I did it#### Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)