[PATCH] wic partition.py: add --apparent-size to du calls

[PATCH] wic partition.py: add --apparent-size to du calls

[Mikko Rapeli]

If build happens on zfs filesystem with compression enabled,
then image size calculations in do_image_wic task can fail:

output: mke2fs 1.47.0 (5-Feb-2023)
Discarding device blocks: done
Creating filesystem with 351999 4k blocks and 176000 inodes
Filesystem UUID: 6091b3a4-ce08-3020-93a6-f755a22ef03b
Superblock backups stored on blocks:
        32768, 98304, 163840, 229376, 294912

Allocating group tables: done
Writing inode tables: done
Creating journal (8192 blocks): done
Copying files into the device: __populate_fs: Could not allocate block in ext2 filesystem while writing file "service-2.json"
mkfs.ext4: Could not allocate block in ext2 filesystem while populating file system

du --help says:

      --apparent-size   print apparent sizes, rather than disk usage; although
                          the apparent size is usually smaller, it may be
                          larger due to holes in ('sparse') files, internal
                          fragmentation, indirect blocks, and the like

du -b already includes --apparent-size.

Same issue reported also in https://lists.yoctoproject.org/g/poky/message/12389

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 scripts/lib/wic/partition.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 795707ec5d..4690ddaa4d 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -254,7 +254,7 @@ class Partition():
                 # Bitbake variable ROOTFS_SIZE is not defined so compute it
                 # from the rootfs_dir size using the same logic found in
                 # get_rootfs_size() from meta/classes/image.bbclass
-                du_cmd = "du -ks %s" % rootfs_dir
+                du_cmd = "du -ks --apparent-size %s" % rootfs_dir
                 out = exec_cmd(du_cmd)
                 self.size = int(out.split()[0])
 
@@ -273,7 +273,7 @@ class Partition():
         """
         Prepare content for an ext2/3/4 rootfs partition.
         """
-        du_cmd = "du -ks %s" % rootfs_dir
+        du_cmd = "du -ks --apparent-size %s" % rootfs_dir
         out = exec_cmd(du_cmd)
         actual_rootfs_size = int(out.split()[0])
 
@@ -349,7 +349,7 @@ class Partition():
         """
         Prepare content for a btrfs rootfs partition.
         """
-        du_cmd = "du -ks %s" % rootfs_dir
+        du_cmd = "du -ks --apparent-size %s" % rootfs_dir
         out = exec_cmd(du_cmd)
         actual_rootfs_size = int(out.split()[0])
 
-- 
2.34.1

Re: [OE-core] [PATCH] wic partition.py: add --apparent-size to du calls

[Richard Purdie]

On Tue, 2024-02-27 at 13:23 +0200, Mikko Rapeli wrote:
> If build happens on zfs filesystem with compression enabled,
> then image size calculations in do_image_wic task can fail:
> 
> output: mke2fs 1.47.0 (5-Feb-2023)
> Discarding device blocks: done
> Creating filesystem with 351999 4k blocks and 176000 inodes
> Filesystem UUID: 6091b3a4-ce08-3020-93a6-f755a22ef03b
> Superblock backups stored on blocks:
>         32768, 98304, 163840, 229376, 294912
> 
> Allocating group tables: done
> Writing inode tables: done
> Creating journal (8192 blocks): done
> Copying files into the device: __populate_fs: Could not allocate
> block in ext2 filesystem while writing file "service-2.json"
> mkfs.ext4: Could not allocate block in ext2 filesystem while
> populating file system
> 
> du --help says:
> 
>       --apparent-size   print apparent sizes, rather than disk usage;
> although
>                           the apparent size is usually smaller, it
> may be
>                           larger due to holes in ('sparse') files,
> internal
>                           fragmentation, indirect blocks, and the
> like
> 
> du -b already includes --apparent-size.
> 
> Same issue reported also in
> https://lists.yoctoproject.org/g/poky/message/12389
> 
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>


From memory we've gone around in circles on this. In some cases we do
really want the size in bytes but in others we want it rounded up to
block size for each file. I think this was why what bug was never fixed
as you can address one set of issues but create a new set.

So for that reason I'm extremely cautious about this, particularly this
late in the release cycle. I don't really know what to do with this
change.

Cheers,

Richard

Re: [OE-core] [PATCH] wic partition.py: add --apparent-size to du calls

[Mikko Rapeli]

Hi,

On Tue, Feb 27, 2024 at 11:48:27AM +0000, Richard Purdie wrote:
> On Tue, 2024-02-27 at 13:23 +0200, Mikko Rapeli wrote:
> > If build happens on zfs filesystem with compression enabled,
> > then image size calculations in do_image_wic task can fail:
> > 
> > output: mke2fs 1.47.0 (5-Feb-2023)
> > Discarding device blocks: done
> > Creating filesystem with 351999 4k blocks and 176000 inodes
> > Filesystem UUID: 6091b3a4-ce08-3020-93a6-f755a22ef03b
> > Superblock backups stored on blocks:
> > ������� 32768, 98304, 163840, 229376, 294912
> > 
> > Allocating group tables: done
> > Writing inode tables: done
> > Creating journal (8192 blocks): done
> > Copying files into the device: __populate_fs: Could not allocate
> > block in ext2 filesystem while writing file "service-2.json"
> > mkfs.ext4: Could not allocate block in ext2 filesystem while
> > populating file system
> > 
> > du --help says:
> > 
> > ����� --apparent-size�� print apparent sizes, rather than disk usage;
> > although
> > ������������������������� the apparent size is usually smaller, it
> > may be
> > ������������������������� larger due to holes in ('sparse') files,
> > internal
> > ������������������������� fragmentation, indirect blocks, and the
> > like
> > 
> > du -b already includes --apparent-size.
> > 
> > Same issue reported also in
> > https://lists.yoctoproject.org/g/poky/message/12389
> > 
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> 
> From memory we've gone around in circles on this. In some cases we do
> really want the size in bytes but in others we want it rounded up to
> block size for each file. I think this was why what bug was never fixed
> as you can address one set of issues but create a new set.
> 
> So for that reason I'm extremely cautious about this, particularly this
> late in the release cycle. I don't really know what to do with this
> change.

I have a build machine with zfs and this issue pops up ever now and then.
Right now I can't build there without this change. I don't see a way to
fix this issue in my build/host specific configuration.

Cheers,

-Mikko

Re: [OE-core] [PATCH] wic partition.py: add --apparent-size to du calls

[Richard Purdie]

On Tue, 2024-02-27 at 13:55 +0200, Mikko Rapeli wrote:
> On Tue, Feb 27, 2024 at 11:48:27AM +0000, Richard Purdie wrote:
> > On Tue, 2024-02-27 at 13:23 +0200, Mikko Rapeli wrote:
> > > If build happens on zfs filesystem with compression enabled,
> > > then image size calculations in do_image_wic task can fail:
> > > 
> > > output: mke2fs 1.47.0 (5-Feb-2023)
> > > Discarding device blocks: done
> > > Creating filesystem with 351999 4k blocks and 176000 inodes
> > > Filesystem UUID: 6091b3a4-ce08-3020-93a6-f755a22ef03b
> > > Superblock backups stored on blocks:
> > >         32768, 98304, 163840, 229376, 294912
> > > 
> > > Allocating group tables: done
> > > Writing inode tables: done
> > > Creating journal (8192 blocks): done
> > > Copying files into the device: __populate_fs: Could not allocate
> > > block in ext2 filesystem while writing file "service-2.json"
> > > mkfs.ext4: Could not allocate block in ext2 filesystem while
> > > populating file system
> > > 
> > > du --help says:
> > > 
> > >       --apparent-size   print apparent sizes, rather than disk usage;
> > > although
> > >                           the apparent size is usually smaller, it
> > > may be
> > >                           larger due to holes in ('sparse') files,
> > > internal
> > >                           fragmentation, indirect blocks, and the
> > > like
> > > 
> > > du -b already includes --apparent-size.
> > > 
> > > Same issue reported also in
> > > https://lists.yoctoproject.org/g/poky/message/12389
> > > 
> > > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > 
> > 
> > From memory we've gone around in circles on this. In some cases we do
> > really want the size in bytes but in others we want it rounded up to
> > block size for each file. I think this was why what bug was never fixed
> > as you can address one set of issues but create a new set.
> > 
> > So for that reason I'm extremely cautious about this, particularly this
> > late in the release cycle. I don't really know what to do with this
> > change.
> 
> I have a build machine with zfs and this issue pops up ever now and then.
> Right now I can't build there without this change. I don't see a way to
> fix this issue in my build/host specific configuration.

Should we break it for others so it works for you?

If I merge this patch and then I get other reports in of issues, I'm
going to be left to somehow fix those other problems. Since your case
will work you will object if I revert this but you likely won't have
time to work on the resulting other issues, or be able to reproduce
them since you're building on zfs.

So you can see my dilemma? :/

Most people don't build on zfs which is probably what I need to balance
this on.

Cheers,

Richard

Re: [OE-core] [PATCH] wic partition.py: add --apparent-size to du calls

[Mikko Rapeli]

Hi,

On Tue, Feb 27, 2024 at 12:06:41PM +0000, Richard Purdie wrote:
> Should we break it for others so it works for you?
> 
> If I merge this patch and then I get other reports in of issues, I'm
> going to be left to somehow fix those other problems. Since your case
> will work you will object if I revert this but you likely won't have
> time to work on the resulting other issues, or be able to reproduce
> them since you're building on zfs.
> 
> So you can see my dilemma? :/
> 
> Most people don't build on zfs which is probably what I need to balance
> this on.

What if I make this optional for zfs? If build directory type is zfs, then
--apparent-size is added, else not.

Cheers,

-Mikko

Re: [OE-core] [PATCH] wic partition.py: add --apparent-size to du calls

[Richard Purdie]

On Tue, 2024-02-27 at 14:23 +0200, Mikko Rapeli wrote:
> On Tue, Feb 27, 2024 at 12:06:41PM +0000, Richard Purdie wrote:
> > Should we break it for others so it works for you?
> > 
> > If I merge this patch and then I get other reports in of issues,
> > I'm
> > going to be left to somehow fix those other problems. Since your
> > case
> > will work you will object if I revert this but you likely won't
> > have
> > time to work on the resulting other issues, or be able to reproduce
> > them since you're building on zfs.
> > 
> > So you can see my dilemma? :/
> > 
> > Most people don't build on zfs which is probably what I need to
> > balance
> > this on.
> 
> What if I make this optional for zfs? If build directory type is zfs,
> then --apparent-size is added, else not.

Wouldn't that cause some really interesting bug reports? 

Asking the user to report their host system filesystem type to debug
anything isn't going to be great.

I don't like the idea of the non-determinism that introduces but I
agree we do already have a determinism issue.

What we probably need is the absolute files size, plus a number of
files and a number of directories, then you can round up to the worst
cases for the block size.

Currently that size number is just including the block sizes.

Cheers,

Richard

Re: [OE-core] [PATCH] wic partition.py: add --apparent-size to du calls

[Mikko Rapeli]

Hi,

On Tue, Feb 27, 2024 at 12:33:15PM +0000, Richard Purdie wrote:
> On Tue, 2024-02-27 at 14:23 +0200, Mikko Rapeli wrote:
> > On Tue, Feb 27, 2024 at 12:06:41PM +0000, Richard Purdie wrote:
> > > Should we break it for others so it works for you?
> > > 
> > > If I merge this patch and then I get other reports in of issues,
> > > I'm
> > > going to be left to somehow fix those other problems. Since your
> > > case
> > > will work you will object if I revert this but you likely won't
> > > have
> > > time to work on the resulting other issues, or be able to reproduce
> > > them since you're building on zfs.
> > > 
> > > So you can see my dilemma? :/
> > > 
> > > Most people don't build on zfs which is probably what I need to
> > > balance
> > > this on.
> > 
> > What if I make this optional for zfs? If build directory type is zfs,
> > then --apparent-size is added, else not.
> 
> Wouldn't that cause some really interesting bug reports?�

Well, if anyone builds on zfs they will hit bugs like this.
zfs usage can be detected and build can fail nicely or apply
the workaround/fix in wic.

> Asking the user to report their host system filesystem type to debug
> anything isn't going to be great.
> 
> I don't like the idea of the non-determinism that introduces but I
> agree we do already have a determinism issue.
> 
> What we probably need is the absolute files size, plus a number of
> files and a number of directories, then you can round up to the worst
> cases for the block size.
> 
> Currently that size number is just including the block sizes.

FWIW, I tried to move this workaround into image recipe with

IMAGE_CMD:wic:prepend() {
    alias df="df --apparent-size"
}

but wic python code doesn't seem to take this into account. Tried also
changing the du calls to have exec_cmd(..., as_shell=True) but this didn't
help either. Something in wic causes the use of another shell or different default
environment for processes.

So I'm stuck with this change in poky to build on a machine with zfs.

Any hints how to apply this simple workaround are welcome.

Cheers,

-Mikko