diff mbox series

classes-recipe/image: Add variable to create rootfs with a exact size

Message ID 20250602213004.2123954-1-fbberton@gmail.com
State New
Headers show
Series classes-recipe/image: Add variable to create rootfs with a exact size | expand

Commit Message

Fabio Berton June 2, 2025, 9:30 p.m. UTC
From: Fabio Berton <fabio.berton@criticaltechworks.com>

Add the IMAGE_ROOTFS_EXACT_SIZE variable to allow the creation of a root
filesystem with an exact size as specified by the user. This bypasses
the default size calculation done by the get_rootfs_size function.

This feature is useful for creating a small ext4 filesystem with a block
size of 1k. The get_rootfs_size function uses a block size of 4k to
determine the size of IMAGE_ROOTFS, which may be larger than the value
set in IMAGE_ROOTFS_SIZE. If an ext4 rootfs with a block size of 1K is
nearly full, the .ext4 file generated by Yocto will be larger than
expected due to the way get_rootfs_size calculates the size.

By using IMAGE_ROOTFS_EXACT_SIZE, users can specify the exact size
required. Users should ensure that the size is set correctly according
to the filesystem requirements. It is not easy to predict the block size
that is used by a filesystem because it can be set by a parameter or it
can be added automatically by the tool that creates the file system.

Signed-off-by: Fabio Berton <fabio.berton@criticaltechworks.com>
---
 meta/classes-recipe/image.bbclass | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Richard Purdie June 2, 2025, 9:37 p.m. UTC | #1
On Mon, 2025-06-02 at 22:30 +0100, Fabio Berton via lists.openembedded.org wrote:
> From: Fabio Berton <fabio.berton@criticaltechworks.com>
> 
> Add the IMAGE_ROOTFS_EXACT_SIZE variable to allow the creation of a root
> filesystem with an exact size as specified by the user. This bypasses
> the default size calculation done by the get_rootfs_size function.
> 
> This feature is useful for creating a small ext4 filesystem with a block
> size of 1k. The get_rootfs_size function uses a block size of 4k to
> determine the size of IMAGE_ROOTFS, which may be larger than the value
> set in IMAGE_ROOTFS_SIZE. If an ext4 rootfs with a block size of 1K is
> nearly full, the .ext4 file generated by Yocto will be larger than
> expected due to the way get_rootfs_size calculates the size.
> 
> By using IMAGE_ROOTFS_EXACT_SIZE, users can specify the exact size
> required. Users should ensure that the size is set correctly according
> to the filesystem requirements. It is not easy to predict the block size
> that is used by a filesystem because it can be set by a parameter or it
> can be added automatically by the tool that creates the file system.
> 
> Signed-off-by: Fabio Berton <fabio.berton@criticaltechworks.com>
> ---
>  meta/classes-recipe/image.bbclass | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/classes-recipe/image.bbclass b/meta/classes-recipe/image.bbclass
> index 24a19fce1a..2df0a5c30d 100644
> --- a/meta/classes-recipe/image.bbclass
> +++ b/meta/classes-recipe/image.bbclass
> @@ -132,7 +132,7 @@ python () {
>  
>  def rootfs_variables(d):
>      from oe.rootfs import variable_depends
> -    variables = ['IMAGE_DEVICE_TABLE','IMAGE_DEVICE_TABLES','BUILD_IMAGES_FROM_FEEDS','IMAGE_TYPES_MASKED','IMAGE_ROOTFS_ALIGNMENT','IMAGE_OVERHEAD_FACTOR','IMAGE_ROOTFS_SIZE','IMAGE_ROOTFS_EXTRA_SPACE',
> +    variables = ['IMAGE_DEVICE_TABLE','IMAGE_DEVICE_TABLES','BUILD_IMAGES_FROM_FEEDS','IMAGE_TYPES_MASKED','IMAGE_ROOTFS_ALIGNMENT','IMAGE_OVERHEAD_FACTOR','IMAGE_ROOTFS_SIZE', 'IMAGE_ROOTFS_EXACT_SIZE', 'IMAGE_ROOTFS_EXTRA_SPACE',
>                   'IMAGE_ROOTFS_MAXSIZE','IMAGE_NAME','IMAGE_LINK_NAME','IMAGE_MANIFEST','DEPLOY_DIR_IMAGE','IMAGE_FSTYPES','IMAGE_INSTALL_COMPLEMENTARY','IMAGE_LINGUAS', 'IMAGE_LINGUAS_COMPLEMENTARY', 'IMAGE_LOCALES_ARCHIVE',
>                   'MULTILIBRE_ALLOW_REP','MULTILIB_TEMP_ROOTFS','MULTILIB_VARIANTS','MULTILIBS','ALL_MULTILIB_PACKAGE_ARCHS','MULTILIB_GLOBAL_VARIANTS','BAD_RECOMMENDATIONS','NO_RECOMMENDATIONS',
>                   'PACKAGE_ARCHS','PACKAGE_CLASSES','TARGET_VENDOR','TARGET_ARCH','TARGET_OS','OVERRIDES','BBEXTENDVARIANT','FEED_DEPLOYDIR_BASE_URI','INTERCEPT_DIR','USE_DEVFS',
> @@ -594,9 +594,14 @@ def get_rootfs_size(d):
>      return base_size
>  
>  python set_image_size () {
> -        rootfs_size = get_rootfs_size(d)
> -        d.setVar('ROOTFS_SIZE', str(rootfs_size))
> -        d.setVarFlag('ROOTFS_SIZE', 'export', '1')
> +    rootfs_size = d.getVar('IMAGE_ROOTFS_EXACT_SIZE')
> +    # If IMAGE_ROOTFS_EXACT_SIZE variable is not set, use the result of
> +    # get_rootfs_size as ROOTFS_SIZE.
> +    if not rootfs_size:
> +        rootfs_size = str(get_rootfs_size(d))
> +
> +    d.setVar('ROOTFS_SIZE', rootfs_size)
> +    d.setVarFlag('ROOTFS_SIZE', 'export', '1')
>  }
> 

I am loathed to add yet another rootfs size variable. This is like
having variables like:

IMAGE_ROOTFS_SIZE
IMAGE_ROOTFS_REALLY_THIS_SIZE
IMAGE_ROOTFS_REALLY_REALLY_THIS_SIZE
IMAGE_ROOTFS_REALLY_THIS_EXACT_SIZE

and then you don't know if "really really" wins compared to "really
this exact" or not without looking at the docs. It isn't a good
interface.

If we were to add one, shouldn't it be in the get_rootfs_size()
function along with the rest of the logic? Would it be better to set
the block size somewhere instead?

I'd also point out there are no tests for it.

Cheers,

Richard
Quentin Schulz June 3, 2025, 9:15 a.m. UTC | #2
Hi all,

On 6/2/25 11:37 PM, Richard Purdie via lists.openembedded.org wrote:
> On Mon, 2025-06-02 at 22:30 +0100, Fabio Berton via lists.openembedded.org wrote:
>> From: Fabio Berton <fabio.berton@criticaltechworks.com>
>>
>> Add the IMAGE_ROOTFS_EXACT_SIZE variable to allow the creation of a root
>> filesystem with an exact size as specified by the user. This bypasses
>> the default size calculation done by the get_rootfs_size function.
>>
>> This feature is useful for creating a small ext4 filesystem with a block
>> size of 1k. The get_rootfs_size function uses a block size of 4k to

That's just the default, the function directory_size takes a blocksize 
argument.

I'm wondering now if it really makes sense to have 
IMAGE_ROOTFS_ALIGNMENT not be a multiple of the blocksize passed to 
directory_size (well I guess the issue is that we then apply an overhead 
percentage on it, which may misalign it...).

>> determine the size of IMAGE_ROOTFS, which may be larger than the value
>> set in IMAGE_ROOTFS_SIZE. If an ext4 rootfs with a block size of 1K is
>> nearly full, the .ext4 file generated by Yocto will be larger than
>> expected due to the way get_rootfs_size calculates the size.
>>

 From my understanding, this means we'd be exceeding 
IMAGE_ROOTFS_MAXSIZE? Does Yocto fail the build because of that? If not, 
then I'd say we should fix it somehow indeed. If yes, this feels just 
like misconfiguration to me?

>> By using IMAGE_ROOTFS_EXACT_SIZE, users can specify the exact size
>> required. Users should ensure that the size is set correctly according

To have the exact size, isn't it just IMAGE_OVERHEAD_FACTOR = 1 and 
IMAGE_ROOTFS_EXTRA_SPACE = 0? Though I assume you need some extra space 
for the ext4 journaling for example?

>> to the filesystem requirements. It is not easy to predict the block size
>> that is used by a filesystem because it can be set by a parameter or it
>> can be added automatically by the tool that creates the file system.
>>

Should we have a get_rootfs_size that is specific to different images? 
e.g. if you generate an ext4 with a given block size, and then another 
file system with another block size, get_rootfs_size would need to 
report something different for both?

But I agree with Richard that we already have too many variables there 
and this feels a bit like a hammer solution for now?

FWIW, we recently had contributions to the documentation for this piece 
of code and this was one of the contributions that took the longest 
because the code is already not that straightforward to read and 
understand so adding yet another exception/variable will not help with 
that. Maybe that's the only option but maybe not, maybe we need a bigger 
rework to simplify all this.

Cheers,
Quentin
diff mbox series

Patch

diff --git a/meta/classes-recipe/image.bbclass b/meta/classes-recipe/image.bbclass
index 24a19fce1a..2df0a5c30d 100644
--- a/meta/classes-recipe/image.bbclass
+++ b/meta/classes-recipe/image.bbclass
@@ -132,7 +132,7 @@  python () {
 
 def rootfs_variables(d):
     from oe.rootfs import variable_depends
-    variables = ['IMAGE_DEVICE_TABLE','IMAGE_DEVICE_TABLES','BUILD_IMAGES_FROM_FEEDS','IMAGE_TYPES_MASKED','IMAGE_ROOTFS_ALIGNMENT','IMAGE_OVERHEAD_FACTOR','IMAGE_ROOTFS_SIZE','IMAGE_ROOTFS_EXTRA_SPACE',
+    variables = ['IMAGE_DEVICE_TABLE','IMAGE_DEVICE_TABLES','BUILD_IMAGES_FROM_FEEDS','IMAGE_TYPES_MASKED','IMAGE_ROOTFS_ALIGNMENT','IMAGE_OVERHEAD_FACTOR','IMAGE_ROOTFS_SIZE', 'IMAGE_ROOTFS_EXACT_SIZE', 'IMAGE_ROOTFS_EXTRA_SPACE',
                  'IMAGE_ROOTFS_MAXSIZE','IMAGE_NAME','IMAGE_LINK_NAME','IMAGE_MANIFEST','DEPLOY_DIR_IMAGE','IMAGE_FSTYPES','IMAGE_INSTALL_COMPLEMENTARY','IMAGE_LINGUAS', 'IMAGE_LINGUAS_COMPLEMENTARY', 'IMAGE_LOCALES_ARCHIVE',
                  'MULTILIBRE_ALLOW_REP','MULTILIB_TEMP_ROOTFS','MULTILIB_VARIANTS','MULTILIBS','ALL_MULTILIB_PACKAGE_ARCHS','MULTILIB_GLOBAL_VARIANTS','BAD_RECOMMENDATIONS','NO_RECOMMENDATIONS',
                  'PACKAGE_ARCHS','PACKAGE_CLASSES','TARGET_VENDOR','TARGET_ARCH','TARGET_OS','OVERRIDES','BBEXTENDVARIANT','FEED_DEPLOYDIR_BASE_URI','INTERCEPT_DIR','USE_DEVFS',
@@ -594,9 +594,14 @@  def get_rootfs_size(d):
     return base_size
 
 python set_image_size () {
-        rootfs_size = get_rootfs_size(d)
-        d.setVar('ROOTFS_SIZE', str(rootfs_size))
-        d.setVarFlag('ROOTFS_SIZE', 'export', '1')
+    rootfs_size = d.getVar('IMAGE_ROOTFS_EXACT_SIZE')
+    # If IMAGE_ROOTFS_EXACT_SIZE variable is not set, use the result of
+    # get_rootfs_size as ROOTFS_SIZE.
+    if not rootfs_size:
+        rootfs_size = str(get_rootfs_size(d))
+
+    d.setVar('ROOTFS_SIZE', rootfs_size)
+    d.setVarFlag('ROOTFS_SIZE', 'export', '1')
 }
 
 #