| 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 |
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
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
Hi Richard, I agree that it's not easy to understand what the variable does without reading the documentation. The idea is to set the image size explicitly and ignore values set by other variables, such as IMAGE_OVERHEAD_FACTOR and IMAGE_ROOTFS_EXTRA_SPACE, creating an image with the exact size specified by this variable. It wasn’t straightforward to understand why setting IMAGE_ROOTFS_SIZE to a specific value still resulted in a final image that was larger than expected. Maybe I'm missing something, but it took me a while to realize the issue is from the directory_size() function [1], which uses a hardcoded 4K block size. In my case, the size of the IMAGE_ROOTFS directory does not match the final image size on the target. With this new variable the image size will be what was defined. I can move the logic into get_rootfs_size(), but first I want to check whether this variable will be accepted or if there's another way to handle this. To set the block size, we could modify directory_size() [1] to use the `-b` option from EXTRA_IMAGECMD to specify the block size. When `-b` is not used, the block size change depending on the filesystem size, maybe the 4k can be default if the `-b` option it's not explicitly add. Also, I'm not sure if there are other options that could affect the final filesystem size. 1 - https://git.openembedded.org/openembedded-core/tree/meta/lib/oe/utils.py#n491 On 6/2/25 22:37, Richard Purdie 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 >> 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
Hi Quentin, On 6/3/25 10:15, Quentin Schulz wrote: > 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? The problem is that the image is always larger than what is defined in IMAGE_ROOTFS_SIZE, because directory_size always uses a 4k block size. So IMAGE_ROOTFS_MAXSIZE doesn't help here. > >>> 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? Yes, both variables need to be changed. But if I know the partition layout I can set the size in IMAGE_ROOTFS_EXACT_SIZE, at least that's what I'm assuming, correct me if I'm wrong. What I did was to set only IMAGE_ROOTFS_EXACT_SIZE instead of changing IMAGE_OVERHEAD_FACTOR and IMAGE_ROOTFS_EXTRA_SPACE. > >>> 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? What we could do is get the blocksize from EXTRA_IMAGECMD. > > 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. Ok, I understand your points. Thanks for the comments! > > Cheers, > Quentin
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') } #