-
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
Support for updating tmpfs size according to Image size #7484
Conversation
installer/sharch_body.sh
Outdated
# Untar and launch install script in a tmpfs | ||
cur_wd=$(pwd) | ||
export cur_wd | ||
archive_path=$(realpath "$0") | ||
tmp_dir=$(mktemp -d) | ||
if [ "$(id -u)" = "0" ] ; then | ||
mount -t tmpfs tmpfs-installer $tmp_dir || exit 1 | ||
mount_size=$(df $tmp_dir | tail -1 | tr -s ' ' | cut -d' ' -f4) | ||
if [ "$mount_size" -lt "$((image_size*3))" ]; 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.
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.
that must be generated from the build process.
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.
use this to get the original size?
lgh@p330:~$ gzip -l docker-sonic-vs.gz
compressed uncompressed ratio uncompressed_name
242618472 684168704 64.5% docker-sonic-vs
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.
This script untars sonic-platform.bin, which is a ONIE image. We cannot use gzip command.
Will update the logic by getting image size from the "tar" command, excluding the ONIE installer script
I hit same issue on Celestica platform. |
Updated logic to get image size from "tar" command. |
installer/sharch_body.sh
Outdated
@@ -25,13 +25,19 @@ fi | |||
|
|||
echo " OK." | |||
|
|||
image_size=$(( $(sed -e '1,/^exit_marker$/d' "$0" | tar --to-stdout -xf - | wc -c) / 1024)) |
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.
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.
okay.will update with ceiling.
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.
Sorry, how do you get the ceiling? Also there are 2 more division below.
Instead of x / 1024
,
(x + 1023) / 1024
will be much safer.
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.
we can avoid ceiling here as we already adding ceiling to image_size by 1G in this command " mount_size=$(((image_size/1024/1024)+1)) " .Here if we add it will be ceiled by 1KB only ,so it is not necessary.
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.
Okay, you are using padding instead of ceiling. Do you think 1G padding is too large?
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.
"mount_size=$(((image_size/1024/1024)+1))" Here we are adding ceiling for image_size and extra free space for the mount , so extra free space will be less than 1GB.
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.
My only concern is the extreme padding size 1G seem too large. Others look good to me.
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.
@qiluo-msft,
is it fine to update 32MB extra space? Find below changes. If this is fine, we will push the changes.
mount_size=$((((image_size+1023)/1024) + 32))
mount -o remount,size="${mount_size}M" -t tmpfs tmpfs-installer $tmp_dir || exit 1
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.
32M padding is good. However you divided 3 times, so I think you formula will not work for corner cases.
This is my math
bytes = $(sed -e '1,/^exit_marker$/d' "$0" | tar --to-stdout -xf - | wc -c)
image_size=$((bytes + 1023) / 1024)
mount_size=$((image_size + 31) / 32) * 32
mount -o remount,size="${mount_size}M"
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.
32M padding is good. However you divided 3 times, so I think you formula will not work for corner cases.
This is my mathbytes = $(sed -e '1,/^exit_marker$/d' "$0" | tar --to-stdout -xf - | wc -c) image_size=$((bytes + 1023) / 1024) mount_size=$((image_size + 31) / 32) * 32 mount -o remount,size="${mount_size}M"
Updated PR with your comments. Please review.
installer/sharch_body.sh
Outdated
# Untar and launch install script in a tmpfs | ||
cur_wd=$(pwd) | ||
export cur_wd | ||
archive_path=$(realpath "$0") | ||
tmp_dir=$(mktemp -d) | ||
if [ "$(id -u)" = "0" ] ; then | ||
mount -t tmpfs tmpfs-installer $tmp_dir || exit 1 | ||
mount_size=$(df $tmp_dir | tail -1 | tr -s ' ' | cut -d' ' -f4) | ||
if [ "$mount_size" -lt "$((image_size*3))" ]; 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.
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 have addressed the comment. Please review
retest please |
@lguohan, @qiluo-msft, we have answered the queries on this PR. Kindly let us know if you have any further queries. Can this PR be merged? |
Signed-off-by: lkunjumon <lkunjumon@marvell.com>
|
@lguohan, can this PR be merged as it is approved by Qi Luo |
#### Why I did it while sonic upgrade, Image will be extracted to tmpfs for installation so tmpfs size should be larger than image size. Image installation will fail if image size is larger than tmpfs size. we are facing below error while installing debug image with size greater than tmpfs which is 1.5g in marvell armhf platform. sonic-installer install <url> New image will be installed, continue? [y/N]: y Downloading image... ...99%, 1744 MB, 708 KB/s, 0 seconds left... Installing image SONiC-OS-202012.0-dirty-20210311.224845 and setting it as default... Command: bash /tmp/sonic_image tar: installer/fs.zip: Wrote only 7680 of 10240 bytes tar: installer/onie-image-arm64.conf: Cannot write: No space left on device tar: Exiting with failure status due to previous errors Verifying image checksum ... OK. Preparing image archive ... #### How I did it compare downloaded image size with tmpfs size, if size less than image size update the tmpfs size according to image size. #### How to verify it Install an Image with size larger than tmpfs. we verified by installing debug image with size 1.9gb which is larger than tmpfs size 1.5gb.
Please cherry-pick this PR to the 202012 and 202106 releases. |
#### Why I did it while sonic upgrade, Image will be extracted to tmpfs for installation so tmpfs size should be larger than image size. Image installation will fail if image size is larger than tmpfs size. we are facing below error while installing debug image with size greater than tmpfs which is 1.5g in marvell armhf platform. sonic-installer install <url> New image will be installed, continue? [y/N]: y Downloading image... ...99%, 1744 MB, 708 KB/s, 0 seconds left... Installing image SONiC-OS-202012.0-dirty-20210311.224845 and setting it as default... Command: bash /tmp/sonic_image tar: installer/fs.zip: Wrote only 7680 of 10240 bytes tar: installer/onie-image-arm64.conf: Cannot write: No space left on device tar: Exiting with failure status due to previous errors Verifying image checksum ... OK. Preparing image archive ... #### How I did it compare downloaded image size with tmpfs size, if size less than image size update the tmpfs size according to image size. #### How to verify it Install an Image with size larger than tmpfs. we verified by installing debug image with size 1.9gb which is larger than tmpfs size 1.5gb.
Why I did it
while sonic upgrade, Image will be extracted to tmpfs for installation so tmpfs size should be larger than image size. Image installation will fail if image size is larger than tmpfs size.
we are facing below error while installing debug image with size greater than tmpfs which is 1.5g in marvell armhf platform.
sonic-installer install
New image will be installed, continue? [y/N]: y
Downloading image...
...99%, 1744 MB, 708 KB/s, 0 seconds left...
Installing image SONiC-OS-202012.0-dirty-20210311.224845 and setting it as default...
Command: bash /tmp/sonic_image
tar: installer/fs.zip: Wrote only 7680 of 10240 bytes
tar: installer/onie-image-arm64.conf: Cannot write: No space left on device
tar: Exiting with failure status due to previous errors
Verifying image checksum ... OK.
Preparing image archive ...
How I did it
compare downloaded image size with tmpfs size, if size less than image size update the tmpfs size according to image size.
How to verify it
Install an Image with size larger than tmpfs. we verified by installing debug image with size 1.9gb which is larger than tmpfs size 1.5gb.
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)