[PATCH 0/5 v3] image_types: use IMAGE_FILE_MAXSIZE variable to create fixed partition size

[PATCH 0/5 v3] image_types: use IMAGE_FILE_MAXSIZE variable to create fixed partition size

[Charles-Antoine Couret]

Details are described in the first patch.

Difference from v2:

* Added working and failing tests in meta/lib/oeqa/selftest/cases/imagefeatures.py
* Split commit to send documentation to right mailing list

Difference from v1:

* Added documentation for IMAGE_FILE_MAXSIZE variable
* Added Python function to get the value of this variable from shell functions
otherwise parsing issue can happen
* Added an additional task to check the final result which works for all filesystems
and not only those created with dd command.

Charles-Antoine Couret (6):
  image_types: add python function to get the IMAGE_FILE_MAXSIZE:fstype
    value
  image_types: use IMAGE_FILE_MAXSIZE variable for ext2/3/4 image types
  image_types: use IMAGE_FILE_MAXSIZE variable for btrfs image types
  image_types: use IMAGE_FILE_MAXSIZE variable for f2fs image types
  image: add check_image_max_size as post function to check file size
    against IMAGE_FILE_MAXSIZE
  ref-manual: document IMAGE_FILE_MAXSIZE variable

 documentation/ref-manual/variables.rst        | 14 +++++
 meta/classes-recipe/image.bbclass             | 30 +++++++++++
 meta/classes-recipe/image_types.bbclass       | 43 ++++++++++++---
 meta/lib/oeqa/selftest/cases/imagefeatures.py | 52 +++++++++++++++++++
 4 files changed, 131 insertions(+), 8 deletions(-)

-- 
2.41.0

[PATCH 1/5 v3] image_types: add python function to get the IMAGE_FILE_MAXSIZE:fstype value

[Charles-Antoine Couret]

It returns 0 if the variable is not set for this filesystem.

In case of fixed partitionning where the rootfs partition can't exceed an
amount of bytes, there is currently no automatic and no generic way to have
this requirement met in any case.

Until now, ROOTFS_SIZE value got from directory_size() does not takes into account
the size of required metadata for the filesystem itself (and does not work well
for other block size than 4k BTW).
Obviously it's a difficult task which depends on rootfs size and filesystem type.

The workaround was to set IMAGE_OVERHEAD_FACTOR and IMAGE_ROOTFS_EXTRA_SPACE
to add the required extra margins. But when the final rootfs is closed to the
maximum size, it's difficult to adjust them correctly. And if you remove
or add new recipes in your image, you've to recompute these margins to have enough
space for these metadata when the rootfs is small, and to not have too big final
image when the rootfs is big.

It's cumbersome and error prone to just have a build failure when the final output
can't be flashed into the partition.

The solution is to follow how it's implemented in buildroot by having a
specific variable, here IMAGE_FILE_MAXSIZE, to create the final sparse file
and trying to fill it with the content of rootfs. If there is enough space,
margins are well compressed and does not consume space in the filesystem.
If there is no enough space, an error is triggered to warm the developer before
trying to use it in the device.

If IMAGE_FILE_MAXSIZE is not set, the idea is to keep the previous behaviour
for compatibility reason and to met other requirements.

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@mind.be>
---
 meta/classes-recipe/image_types.bbclass | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/meta/classes-recipe/image_types.bbclass b/meta/classes-recipe/image_types.bbclass
index 4aed64e27f..be8197f1f6 100644
--- a/meta/classes-recipe/image_types.bbclass
+++ b/meta/classes-recipe/image_types.bbclass
@@ -54,6 +54,13 @@ def imagetypes_getdepends(d):
     # Sort the set so that ordering is consistant
     return " ".join(sorted(deps))
 
+def get_max_image_size(d, fs):
+    max_size = d.getVar("IMAGE_FILE_MAXSIZE:%s" % fs)
+    if max_size is not None:
+        return max_size
+
+    return 0
+
 XZ_COMPRESSION_LEVEL ?= "-9"
 XZ_INTEGRITY_CHECK ?= "crc32"
 
-- 
2.41.0

[PATCH 2/5 v3] image_types: use IMAGE_FILE_MAXSIZE variable for ext2/3/4 image types

[Charles-Antoine Couret]

If defined, this variable value overrides the size of ext* partition file created by mkfs.
Otherwise previous logic based on ROOTFS_SIZE variable is used.

It should be set when the final file size would not be above a specific value due to fixed
partitionning for example.

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@mind.be>
---
 meta/classes-recipe/image_types.bbclass | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/meta/classes-recipe/image_types.bbclass b/meta/classes-recipe/image_types.bbclass
index be8197f1f6..fb1e33cf3e 100644
--- a/meta/classes-recipe/image_types.bbclass
+++ b/meta/classes-recipe/image_types.bbclass
@@ -79,24 +79,32 @@ IMAGE_CMD:cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${IMGDEPLOYDIR}/${IMAGE_NAME}.cr
 
 oe_mkext234fs () {
 	fstype=$1
+	image_file_maxsize=$2
 	extra_imagecmd=""
+	rootfs_file_size=$ROOTFS_SIZE
 
-	if [ $# -gt 1 ]; then
-		shift
+	if [ $# -gt 2 ]; then
+		shift 2
 		extra_imagecmd=$@
 	fi
 
+
+	if [[ "${image_file_maxsize}" -ne 0 ]]; then
+		rootfs_file_size=${image_file_maxsize}
+	fi
+
 	# If generating an empty image the size of the sparse block should be large
 	# enough to allocate an ext4 filesystem using 4096 bytes per inode, this is
 	# about 60K, so dd needs a minimum count of 60, with bs=1024 (bytes per IO)
 	eval local COUNT=\"0\"
 	eval local MIN_COUNT=\"60\"
-	if [ $ROOTFS_SIZE -lt $MIN_COUNT ]; then
+	if [ $rootfs_file_size -lt $MIN_COUNT ]; then
 		eval COUNT=\"$MIN_COUNT\"
 	fi
+
 	# Create a sparse image block
-	bbdebug 1 Executing "dd if=/dev/zero of=${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype seek=$ROOTFS_SIZE count=$COUNT bs=1024"
-	dd if=/dev/zero of=${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype seek=$ROOTFS_SIZE count=$COUNT bs=1024
+	bbdebug 1 Executing "dd if=/dev/zero of=${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype seek=$rootfs_file_size count=$COUNT bs=1024"
+	dd if=/dev/zero of=${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype seek=$rootfs_file_size count=$COUNT bs=1024
 	bbdebug 1 "Actual Rootfs size:  `du -s ${IMAGE_ROOTFS}`"
 	bbdebug 1 "Actual Partition size: `stat -c '%s' ${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype`"
 	bbdebug 1 Executing "mkfs.$fstype -F $extra_imagecmd ${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype -d ${IMAGE_ROOTFS}"
@@ -105,9 +113,9 @@ oe_mkext234fs () {
 	fsck.$fstype -pvfD ${IMGDEPLOYDIR}/${IMAGE_NAME}.$fstype || [ $? -le 3 ]
 }
 
-IMAGE_CMD:ext2 = "oe_mkext234fs ext2 ${EXTRA_IMAGECMD}"
-IMAGE_CMD:ext3 = "oe_mkext234fs ext3 ${EXTRA_IMAGECMD}"
-IMAGE_CMD:ext4 = "oe_mkext234fs ext4 ${EXTRA_IMAGECMD}"
+IMAGE_CMD:ext2 = "oe_mkext234fs ext2 ${@get_max_image_size(d, 'ext2')} ${EXTRA_IMAGECMD}"
+IMAGE_CMD:ext3 = "oe_mkext234fs ext3 ${@get_max_image_size(d, 'ext3')} ${EXTRA_IMAGECMD}"
+IMAGE_CMD:ext4 = "oe_mkext234fs ext4 ${@get_max_image_size(d, 'ext4')} ${EXTRA_IMAGECMD}"
 
 MIN_BTRFS_SIZE ?= "16384"
 IMAGE_CMD:btrfs () {
-- 
2.41.0

[PATCH 3/5 v3] image_types: use IMAGE_FILE_MAXSIZE variable for btrfs image types

[Charles-Antoine Couret]

If defined, this variable value overrides the size of btrfs partition file created by mkfs.
Otherwise previous logic based on ROOTFS_SIZE variable is used.

It should be set when the final file size would not be above a specific value due to fixed
partitionning for example.

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@mind.be>
---
 meta/classes-recipe/image_types.bbclass | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meta/classes-recipe/image_types.bbclass b/meta/classes-recipe/image_types.bbclass
index fb1e33cf3e..1356552445 100644
--- a/meta/classes-recipe/image_types.bbclass
+++ b/meta/classes-recipe/image_types.bbclass
@@ -120,6 +120,12 @@ IMAGE_CMD:ext4 = "oe_mkext234fs ext4 ${@get_max_image_size(d, 'ext4')} ${EXTRA_I
 MIN_BTRFS_SIZE ?= "16384"
 IMAGE_CMD:btrfs () {
 	size=${ROOTFS_SIZE}
+	image_file_maxsize=${@get_max_image_size(d, "btrfs")}
+
+	if [[ ${image_file_maxsize} -ne 0 ]]; then
+		size=${image_file_maxsize}
+	fi
+
 	if [ ${size} -lt ${MIN_BTRFS_SIZE} ] ; then
 		size=${MIN_BTRFS_SIZE}
 		bbwarn "Rootfs size is too small for BTRFS. Filesystem will be extended to ${size}K"
-- 
2.41.0

[PATCH 4/5 v3] image_types: use IMAGE_FILE_MAXSIZE variable for f2fs image types

[Charles-Antoine Couret]

If defined, this variable value overrides the size of f2fs partition file created by mkfs.
Otherwise previous logic based on ROOTFS_SIZE variable is used.

It should be set when the final file size would not be above a specific value due to fixed
partitionning for example.

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@mind.be>
---
 meta/classes-recipe/image_types.bbclass | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meta/classes-recipe/image_types.bbclass b/meta/classes-recipe/image_types.bbclass
index 1356552445..6e3190fa3f 100644
--- a/meta/classes-recipe/image_types.bbclass
+++ b/meta/classes-recipe/image_types.bbclass
@@ -272,6 +272,12 @@ IMAGE_CMD:f2fs () {
         # 500M the standard IMAGE_OVERHEAD_FACTOR does not work, so add additional
         # space here when under 500M
 	size=${ROOTFS_SIZE}
+	image_file_maxsize=${@get_max_image_size(d, "f2fs")}
+
+	if [[ "${image_file_maxsize}" -ne 0 ]]; then
+		size=${image_file_maxsize}
+	fi
+
 	if [ ${size} -lt ${MIN_F2FS_SIZE} ] ; then
 		size=${MIN_F2FS_SIZE}
 		bbwarn "Rootfs size is too small for F2FS. Filesystem will be extended to ${size}K"
-- 
2.41.0

[PATCH 5/5 v3] image: add check_image_max_size as post function to check file size against IMAGE_FILE_MAXSIZE

[Charles-Antoine Couret]

Trigger an error if the final size is above IMAGE_FILE_MAXSIZE value. Which is relevant if the
partition size is fixed and the user wants to be sure that the image can be entirely installed
into its partition.

If the variable is not set, no error is trigger. It works for all filesystems.

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@mind.be>
---
 meta/classes-recipe/image.bbclass             | 30 +++++++++++
 meta/lib/oeqa/selftest/cases/imagefeatures.py | 52 +++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/meta/classes-recipe/image.bbclass b/meta/classes-recipe/image.bbclass
index 7231fad940..d2fcf078c9 100644
--- a/meta/classes-recipe/image.bbclass
+++ b/meta/classes-recipe/image.bbclass
@@ -509,6 +509,7 @@ python () {
 
         d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' set_image_size')
         d.prependVarFlag(task, 'postfuncs', 'create_symlinks ')
+        d.prependVarFlag(task, 'postfuncs', 'check_image_max_size ')
         d.appendVarFlag(task, 'subimages', ' ' + ' '.join(subimages))
         d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps))
         d.appendVarFlag(task, 'vardepsexclude', ' DATETIME DATE ' + ' '.join(vardepsexclude))
@@ -609,6 +610,35 @@ python create_symlinks() {
             bb.note("Skipping symlink, source does not exist: %s -> %s" % (dst, src))
 }
 
+#
+# Check if image size is lighter than maximum size
+#
+python check_image_max_size() {
+    def get_max_image_size(d, fs):
+        max_size = d.getVar("IMAGE_FILE_MAXSIZE:%s" % fs)
+        if max_size is not None:
+            return int(max_size)
+
+        return None
+
+    deploy_dir = d.getVar('IMGDEPLOYDIR')
+    img_name = d.getVar('IMAGE_NAME')
+    taskname = d.getVar("BB_CURRENTTASK")
+    subimages = (d.getVarFlag("do_" + taskname, 'subimages', False) or "").split()
+    imgsuffix = d.getVarFlag("do_" + taskname, 'imgsuffix') or d.expand("${IMAGE_NAME_SUFFIX}.")
+
+    for type in subimages:
+        file_name = os.path.join(deploy_dir, img_name + imgsuffix + type)
+
+        if os.path.exists(file_name):
+            file_size = os.stat(file_name).st_size / 1024
+            max_size = get_max_image_size(d, type)
+            if max_size is not None:
+                if file_size > max_size:
+                    bb.error("The image %s size %d(K) exceeds IMAGE_FILE_MAXSIZE: %d(K)" % \
+                    (file_name, file_size, max_size))
+}
+
 MULTILIBRE_ALLOW_REP =. "${base_bindir}|${base_sbindir}|${bindir}|${sbindir}|${libexecdir}|${sysconfdir}|${nonarch_base_libdir}/udev|/lib/modules/[^/]*/modules.*|"
 MULTILIB_CHECK_FILE = "${WORKDIR}/multilib_check.py"
 MULTILIB_TEMP_ROOTFS = "${WORKDIR}/multilib"
diff --git a/meta/lib/oeqa/selftest/cases/imagefeatures.py b/meta/lib/oeqa/selftest/cases/imagefeatures.py
index dc88c222bd..afdc7a72fa 100644
--- a/meta/lib/oeqa/selftest/cases/imagefeatures.py
+++ b/meta/lib/oeqa/selftest/cases/imagefeatures.py
@@ -234,6 +234,58 @@ UBINIZE_ARGS_mtd_4_256 ?= "-m 4096 -p 256KiB"
                 self.assertTrue(os.path.exists(image_path),
                                 "%s image %s doesn't exist" % (itype, image_path))
 
+    def test_image_maxsize_success(self):
+        """
+        Summary:     Check if the image built is below maximum size if set
+        Expected:    image is built, size below the limit and no error are reported
+        Product:     oe-core
+        Author:      Charles-Antoine Couret <charles-antoine.couret@mind.be>
+        """
+        image = 'core-image-minimal'
+
+        config = """
+IMAGE_FSTYPES += "ext4"
+IMAGE_FILE_MAXSIZE:ext4 = "300000"
+"""
+        self.write_config(config)
+
+        res = bitbake(image)
+        bb_vars = get_bb_vars(['DEPLOY_DIR_IMAGE', 'IMAGE_LINK_NAME'], image)
+
+        image_path = os.path.join(bb_vars['DEPLOY_DIR_IMAGE'], "%s.%s" % (bb_vars['IMAGE_LINK_NAME'], "ext4"))
+        # check if result image is in deploy directory
+        self.assertTrue(os.path.exists(image_path),
+                        "%s image %s doesn't exist" % ("ext4", image_path))
+        # check if result image size is below than maximum value
+        self.assertTrue(os.stat(image_path).st_size / 1024 <= 300000)
+        # No error during execution
+        self.assertEqual(res.status, 0)
+
+    def test_image_maxsize_failure(self):
+        """
+        Summary:     Check if the image built with size above limit is triggering error
+        Expected:    the image size is above limit and triggers error
+        Product:     oe-core
+        Author:      Charles-Antoine Couret <charles-antoine.couret@mind.be>
+        """
+        image = 'core-image-minimal'
+
+        config = """
+IMAGE_FSTYPES += "ext4"
+IMAGE_FILE_MAXSIZE:ext4 = "1000"
+"""
+        self.write_config(config)
+
+        res = bitbake(image, ignore_status=True)
+        bb_vars = get_bb_vars(['DEPLOY_DIR_IMAGE', 'IMAGE_LINK_NAME'], image)
+
+        image_path = os.path.join(bb_vars['DEPLOY_DIR_IMAGE'], "%s.%s" % (bb_vars['IMAGE_LINK_NAME'], "ext4"))
+        # check if result image is not generated
+        self.assertFalse(os.path.exists(image_path),
+                        "%s image %s doesn't exist" % ("ext4", image_path))
+        # Error triggered during execution
+        self.assertNotEqual(res.status, 0)
+
     def test_useradd_static(self):
         config = """
 USERADDEXTENSION = "useradd-staticids"
-- 
2.41.0

Re: [OE-core] [PATCH 0/5 v3] image_types: use IMAGE_FILE_MAXSIZE variable to create fixed partition size

[Alexandre Belloni]

Hello,

The tests fail on the ubuntu and debian workers but seemed to pass just
fine on fedora:

https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/5941/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/6008/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/127/builds/2369/steps/15/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/6010/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/5943/steps/14/logs/stdio

I'm not sure the login timeout is relevant.

Also, can you please fix your git configuration:
https://docs.yoctoproject.org/dev/contributor-guide/submit-changes.html#fixing-your-from-identity

On 02/11/2023 00:10:49+0100, Charles-Antoine Couret via lists.openembedded.org wrote:
> Details are described in the first patch.
> 
> Difference from v2:
> 
> * Added working and failing tests in meta/lib/oeqa/selftest/cases/imagefeatures.py
> * Split commit to send documentation to right mailing list
> 
> Difference from v1:
> 
> * Added documentation for IMAGE_FILE_MAXSIZE variable
> * Added Python function to get the value of this variable from shell functions
> otherwise parsing issue can happen
> * Added an additional task to check the final result which works for all filesystems
> and not only those created with dd command.
> 
> Charles-Antoine Couret (6):
>   image_types: add python function to get the IMAGE_FILE_MAXSIZE:fstype
>     value
>   image_types: use IMAGE_FILE_MAXSIZE variable for ext2/3/4 image types
>   image_types: use IMAGE_FILE_MAXSIZE variable for btrfs image types
>   image_types: use IMAGE_FILE_MAXSIZE variable for f2fs image types
>   image: add check_image_max_size as post function to check file size
>     against IMAGE_FILE_MAXSIZE
>   ref-manual: document IMAGE_FILE_MAXSIZE variable
> 
>  documentation/ref-manual/variables.rst        | 14 +++++
>  meta/classes-recipe/image.bbclass             | 30 +++++++++++
>  meta/classes-recipe/image_types.bbclass       | 43 ++++++++++++---
>  meta/lib/oeqa/selftest/cases/imagefeatures.py | 52 +++++++++++++++++++
>  4 files changed, 131 insertions(+), 8 deletions(-)
> 
> -- 
> 2.41.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#190048): https://lists.openembedded.org/g/openembedded-core/message/190048
> Mute This Topic: https://lists.openembedded.org/mt/102331599/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com