-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[arm]Support compile sonic arm image on arm server #7285
[arm]Support compile sonic arm image on arm server #7285
Conversation
9dc0759
to
2690e5f
Compare
i feel we need to name those docker properly sonic-slave-buster -> amd64 for native arm64 image, I feel we should name the image as sonic-slave-buster instead of sonic-slave-buster-amd64. to align with In the docker register, sonic-slave-buster will have three images, one for each platform, amd64/armhf/arm64. when user pull the image, it will pull the correct one based on the client machine arch. |
as you know, we introduce this ENABLE_DOCKER_BASE_PULL to pull sonic-slave docker from ACR (azure container registery), and it looks like acr supports docker manifest. https://docs.microsoft.com/en-us/azure/container-registry/container-registry-image-formats which allow registries to store multi-architecture images under a single image:tag reference. therefore, we should think our naming properly so that when user try to build arm64 natively, they can pull the proper sonic-slave container from acr. here is docker manifest example:
to fulfill our goal above, we can replace coolapp to sonic-slave-buster. then, we have following native images we can rename current multiarch image to sonic-slave-buster-march-amd64-arm64. it means this is a multiarch image that emulate arm64 on amd64. since we might have a emulated image for armhf on arm64? if we do not want to rename current multiarch image, then we will rename the native images to should we rename current multiarch image, or should we add native to new native arm64 image? |
Makefile.work
Outdated
@@ -98,8 +107,14 @@ endif | |||
|
|||
ifeq ($(CONFIGURED_ARCH),amd64) | |||
SLAVE_BASE_IMAGE = $(SLAVE_DIR) | |||
EMULATED_COMPILE = n |
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.
i am not sure how well known this emulated_compile term is. can we rename to MULTIARCH_QEMU_ENVIRON = y? It is perhaps more precise and better known.
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.
Obviously, MULTIARCH_QEMU_ENVIRON is more precise and better known, I will rename it.
build_debian.sh
Outdated
@@ -581,7 +581,7 @@ sudo mksquashfs $FILESYSTEM_ROOT $FILESYSTEM_SQUASHFS -e boot -e var/lib/docker | |||
|
|||
scripts/collect_host_image_version_files.sh $TARGET_PATH $FILESYSTEM_ROOT | |||
|
|||
if [[ $CONFIGURED_ARCH == armhf || $CONFIGURED_ARCH == arm64 ]]; then | |||
if [ $EMULATED_COMPILE == y ] && [[ $CONFIGURED_ARCH == armhf || $CONFIGURED_ARCH == arm64 ]]; then |
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.
is 2nd and 3rd condition needed?
scripts/build_debian_base_system.sh
Outdated
# qemu arm bin executable for cross-building | ||
sudo mkdir -p $FILESYSTEM_ROOT/usr/bin | ||
sudo cp /usr/bin/qemu*static $FILESYSTEM_ROOT/usr/bin || true | ||
if [ $EMULATED_COMPILE == y ]; then |
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.
why line 22 is still needed?
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.
Line 28 is only for arm, then line 22 should be kept.
Based on you comments and my research, I suggest to name the slave docker as follows:
|
2690e5f
to
f387a1e
Compare
if you look at the example, the manifest name needs to be different from the image name, so if the manifest is sonic-slave-buster, then image name needs to be different. |
# qemu arm bin executable for cross-building | ||
sudo mkdir -p $FILESYSTEM_ROOT/usr/bin | ||
sudo cp /usr/bin/qemu*static $FILESYSTEM_ROOT/usr/bin || true | ||
if [ $MULTIARCH_QEMU_ENVIRON == y ]; then |
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.
can you remove line 22?
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.
Reference to https://www.debian.org/mirror/list, debian-archive.trafficmanager.net doesn't have the arm source mirror. Is the http://deb.debian.org/debian used for the source of x86 acceptable?
f387a1e
to
5e807fa
Compare
5e807fa
to
7f24dc7
Compare
Commenter does not have sufficient privileges for PR 7285 in repo Azure/sonic-buildimage |
@guxianghong , could you please try "/azpw run" to rerun checkers? |
/azpw run "Azure.sonic-buildimage (Build mellanox)" |
/AzurePipelines run "Azure.sonic-buildimage (Build mellanox)" |
No pipelines are associated with this pull request. |
/azpw run kvmtest-t0 |
/AzurePipelines run kvmtest-t0 |
No pipelines are associated with this pull request. |
/azpw run kvmtest-t0 |
/AzurePipelines run kvmtest-t0 |
No pipelines are associated with this pull request. |
/azpw run "Azure.sonic-buildimage (Test vstest)" |
/AzurePipelines run "Azure.sonic-buildimage (Test vstest)" |
No pipelines are associated with this pull request. |
/azpw run "Azure.sonic-buildimage (Test kvmtest-t0)" |
/AzurePipelines run "Azure.sonic-buildimage (Test kvmtest-t0)" |
No pipelines are associated with this pull request. |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
Any progress on backporting this to 202012? |
we are planning to do it, but there were some issue identified for some dependency pr. the pr is not clean as it did the systemd upgrade to 247, however, there is some issue #7364 associated with systemd 247 upgrade, and thus it is blocked. |
- Support compile sonic arm image on arm server. If arm image compiling is executed on arm server instead of using qemu mode on x86 server, compile time can be saved significantly. - Add kernel argument systemd.unified_cgroup_hierarchy=0 for upgrade systemd to version 247, according to #7228 - rename multiarch docker to sonic-slave-${distro}-march-${arch} Co-authored-by: Xianghong Gu <xgu@centecnetworks.com> Co-authored-by: Shi Lei <shil@centecnetworks.com>
- Support compile sonic arm image on arm server. If arm image compiling is executed on arm server instead of using qemu mode on x86 server, compile time can be saved significantly. - Add kernel argument systemd.unified_cgroup_hierarchy=0 for upgrade systemd to version 247, according to sonic-net#7228 - rename multiarch docker to sonic-slave-${distro}-march-${arch} Co-authored-by: Xianghong Gu <xgu@centecnetworks.com> Co-authored-by: Shi Lei <shil@centecnetworks.com>
- Support compile sonic arm image on arm server. If arm image compiling is executed on arm server instead of using qemu mode on x86 server, compile time can be saved significantly. - Add kernel argument systemd.unified_cgroup_hierarchy=0 for upgrade systemd to version 247, according to sonic-net#7228 - rename multiarch docker to sonic-slave-${distro}-march-${arch} Co-authored-by: Xianghong Gu <xgu@centecnetworks.com> Co-authored-by: Shi Lei <shil@centecnetworks.com>
Why I did it
How I did it
When it is detected that the arm compiling is executed on the arm server, the emulated mode of compiling is skipped. If the arm compiling isn't executed on arm server, the emulated mode should work normally.
How to verify it
Which release branch to backport (provide reason below if selected)
Description for the changelog
Above modify is for the compile error " libsystemd-dev : Depends: libsystemd0 (= 241-7
deb10u7) but 247.3-3bpo10+1 is to be installed". This error is related to PR #7228, and the d.A picture of a cute animal (not mandatory but encouraged)