Skip to content

Meta: Cap DISK_SIZE_BYTES max to 2TB#26121

Closed
StarterX4 wants to merge 1 commit into
SerenityOS:masterfrom
StarterX4:fix-disk_size_bytes-too-large
Closed

Meta: Cap DISK_SIZE_BYTES max to 2TB#26121
StarterX4 wants to merge 1 commit into
SerenityOS:masterfrom
StarterX4:fix-disk_size_bytes-too-large

Conversation

@StarterX4

Copy link
Copy Markdown
Contributor

In the case if it would somehow been too large.
I've got this issue during build, either due to building on tmpfs, or due to some uutils coreutils bug, no idea.

-- Install configuration: ""
ninja: Entering directory `/tmp/serenity/Build/x86_64'
[0/1] cd /tmp/serenity/Build/x86_64 && /usr/bin/cmake -E env SER...N=GNU LLVM_VERSION=15.1.0 /tmp/serenity/Meta/build-image-qemu.sh
checking existing image
e2fsck 1.47.3 (8-Jul-2025)
ext2fs_open2: Bad magic number in super-block
/usr/bin/e2fsck: Superblock invalid, trying backup blocks...
/usr/bin/e2fsck: Bad magic number in super-block while trying to open _disk_image

The superblock could not be read or does not describe a valid ext2/ext3/ext4
filesystem.  If the device is valid and it really contains an ext2/ext3/ext4
filesystem (and not swap or ufs or something else), then the superblock
is corrupt, and you might try running e2fsck with an alternate superblock:
    e2fsck -b 8193 <device>
 or
    e2fsck -b 32768 <device>

failed, not using existing image
setting up disk image... done
creating new filesystem... mke2fs: Size of device (0x5847bf9f47 blocks) _disk_image too big to be expressed
        in 32 bits using a blocksize of 4096.
die: could not create filesystem
FAILED: CMakeFiles/qemu-image _disk_image /tmp/serenity/Build/x86_64/CMakeFiles/qemu-image /tmp/serenity/Build/x86_64/_disk_image 
cd /tmp/serenity/Build/x86_64 && /usr/bin/cmake -E env SERENITY_SOURCE_DIR=/tmp/serenity SERENITY_ARCH=x86_64 SERENITY_TOOLCHAIN=GNU LLVM_VERSION=15.1.0 /tmp/serenity/Meta/build-image-qemu.sh
ninja: build stopped: subcommand failed.

But setting a DISK_SIZE_BYTES max fixed the issue.

In the case if it would somehow been too large.
As the 32-bit filesystem is being used.
@github-actions github-actions Bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Aug 10, 2025
@spholz

spholz commented Aug 10, 2025

Copy link
Copy Markdown
Member

I think the problem is that uutils ignores the du -m flag instead of throwing an error.
-m should make du print the size in megabytes.

@spholz

spholz commented Aug 11, 2025

Copy link
Copy Markdown
Member

A more proper fix would be to make disk_usage() not use -m.

Though I also think that if uutils is so broken that du -m gets silently ignored, it isn't worth the effort for us to support it tbh.
What distro even uses uutils? Using a coreutils reimplementation that only reached 0.1 recently seems a bit silly.

@StarterX4

Copy link
Copy Markdown
Contributor Author

coreutils-uutils is available for Arch from AUR, but CachyOS also provides it in their cachyos repo.
du -m, -sm seem to work fine for me:

[starterx4@doadgrz left4dead2]$ du -s downloads/
219619  downloads/
[starterx4@doadgrz left4dead2]$ du -sm downloads/
215     downloads/
[starterx4@doadgrz left4dead2]$ du -sm /bin/
2740    /bin/

But noticed now the -b is: -b, --bytes equivalent to '--apparent-size --block-size=1'
(though for GNU it should be the same)

[starterx4@doadgrz left4dead2]$ du -b -sm /bin/
2861896559      /bin/

I think the issue might be then in the multiplication in line 39 of build-image-qemu.sh, or the INODE calculation, but need to check it later.

@spholz

spholz commented Aug 11, 2025

Copy link
Copy Markdown
Member

Oh I guess uutils doesn't behave correctly if both flags are passed.

If I put -m after -b I get the apparent size in MB for both GNU and busybox du:

$ du -bsm Base
23	Base
$ busybox du -bsm Base
23	Base
$ uu-du -bsm Base
23483797	Base

But if I put it before -b I get the size in bytes for GNU du:

$ du -mbs Base
23483797	Base
$ busybox du -mbs Base
23	Base
$ uu-du -mbs Base
23483797	Base

So for GNU du the order seems to matter and busybox du always uses megabytes if -m is passed anywhere.

I don't think I can run BSD du on my linux system, so I can't test that.

@spholz

spholz commented Aug 11, 2025

Copy link
Copy Markdown
Member

BusyBox doesn't support long arguments like --apparent-size, so we can't use that either.
Don't know about the BSDs.

BusyBox's -b flag is described like this in the help text:

-b	Apparent size (including holes)

@StarterX4

Copy link
Copy Markdown
Contributor Author

On FreeBSD 12.2 it looks like this:
image

@spholz

spholz commented Aug 12, 2025

Copy link
Copy Markdown
Member

So FreeBSD behaves like BusyBox. macOS probably as well, since it also uses BSD-like command line utils.

uutils coreutils seems to be a reimplementation of GNU coreutils specificially, so it should probably take the order of flags into account. So maybe file an issue about this in uutils/coreutils?

You could make your previous PR use --apparent-size -sm for uutils instead (and add a note that this is a workaround for a uutils bug). If this is the only necessary fix for making uutils coreutils work, I guess it's fine.

@StarterX4

Copy link
Copy Markdown
Contributor Author

This, and #26120 for uutils detection, then the compilation succeeds.

@spholz

spholz commented Aug 12, 2025

Copy link
Copy Markdown
Member

Capping the max disk size to 2 TB is not a good workaround. I would rather make your previous PR use --apparent-size -sm for uutils coreutils.

@StarterX4

Copy link
Copy Markdown
Contributor Author

I know, --apparent-size is a better solution. But just a question, force-push it to this pull request, or add it to #26120?

@spholz

spholz commented Aug 12, 2025

Copy link
Copy Markdown
Member

git commit --amend it to the commit in #26120.

StarterX4 added a commit to StarterX4/serenity that referenced this pull request Aug 12, 2025
As uutils coreutils (a Rust GNU coreutils reimplementation)
`du --help` doesn't include any information where it comes from,
but `du -V` does:
```
[starterx4@doadgrz ~]$ du -V
du (uutils coreutils) 0.1.0
```

We have to use it then to detect uutils correctly as
the BSD `-A` argument fails (uutils is not BSD-compatible):
```
error: unexpected argument '-A' found

  tip: to pass '-A' as a value, use '-- -A'

Usage: du [OPTION]... [FILE]...
       du [OPTION]... --files0-from=F
error: unexpected argument '-A' found

  tip: to pass '-A' as a value, use '-- -A'

Usage: du [OPTION]... [FILE]...
       du [OPTION]... --files0-from=F
```

Also we have to use `--apparent-size`
due to a bug with `-b` (SerenityOS#26121)
@StarterX4

Copy link
Copy Markdown
Contributor Author

Done.

@StarterX4 StarterX4 closed this Aug 12, 2025
@github-actions github-actions Bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Aug 12, 2025
spholz pushed a commit that referenced this pull request Aug 13, 2025
As uutils coreutils (a Rust GNU coreutils reimplementation)
`du --help` doesn't include any information where it comes from,
but `du -V` does:
```
[starterx4@doadgrz ~]$ du -V
du (uutils coreutils) 0.1.0
```

We have to use it then to detect uutils correctly as
the BSD `-A` argument fails (uutils is not BSD-compatible):
```
error: unexpected argument '-A' found

  tip: to pass '-A' as a value, use '-- -A'

Usage: du [OPTION]... [FILE]...
       du [OPTION]... --files0-from=F
error: unexpected argument '-A' found

  tip: to pass '-A' as a value, use '-- -A'

Usage: du [OPTION]... [FILE]...
       du [OPTION]... --files0-from=F
```

Also we have to use `--apparent-size`
due to a bug with `-b` (#26121)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants