Skip to content
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.

Sign up for GitHub

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

Jump to bottom

SONiC Image Installation Verification #933

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

SONiC Image Installation Verification #933

wants to merge 7 commits into from

Conversation

wen587
Copy link
Contributor

@wen587 wen587 commented Jan 25, 2022

No description provided.

@wen587 wen587 requested a review from qiluo-msft January 25, 2022 07:44
@wen587 wen587 marked this pull request as ready for review January 27, 2022 05:59
qiluo-msft
qiluo-msft reviewed Feb 21, 2022
View reviewed changes
doc/sonic-installer/SONiC_Image_Installation_Verification.md Outdated
```
Need to mention that, it may prompt false negative message. Assume we have an broadcom switch which has already installed mellanox built image as no ASIC check before. Its `asic_type` will be translated to mellanox. Then it will wrongly report asic doesn't match if we try to install the correct broadcom built image.

In ONiE, that info is not accessible. So we cannot do the ASIC check in ONiE.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ONiE

The capitalization is not as official. Please use ONIE or onie. ref: https://www.opencompute.org/wiki/Networking/ONIE

qiluo-msft
qiluo-msft reviewed Feb 21, 2022
View reviewed changes
doc/sonic-installer/SONiC_Image_Installation_Verification.md
- The mapping fits both ONiE and SONiC installer. During installation, we can derive current platform's expected ASIC type based on the platform string, then compare that with the to-be-installed image's ASIC.

Cons:
- It is a static mapping, which means that the mapping need to be kept in all repos and all ASIC build.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all repos

What do you mean by "all repos"? Normally we mean git repo, and the feature is only relevant on sonic-buildimage repo and sonic-utilities repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean we have public repo and internal repo. The platforms we support are somehow different.

@yxieca yxieca force-pushed the master branch 2 times, most recently from 8498931 to 8837dc2 Compare April 15, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wen587 @qiluo-msft