With no -M, -m, or -s options, makefs should autocalculate the image size. However, sometimes it underestimates the size required, then fails when it's writing the image with an error like this: Calculated size of `/tmp/kyua.ZQYFDj/2/work/test.img': 688128 bytes, 33 inodes Extent size set to 32768 /tmp/kyua.ZQYFDj/2/work/test.img: 0.7MB (1344 sectors) block size 32768, fragment size 4096 using 1 cylinder groups of 0.66MB, 21 blks, 256 inodes. super-block backups (for fsck -b #) at: 64, Populating `/tmp/kyua.ZQYFDj/2/work/test.img' makefs: Writing inode 22 (524288.file), bytes 491520 + 32768: No space left on device
A commit references this bug: Author: asomers Date: Sat Jul 21 17:24:15 UTC 2018 New revision: 336582 URL: https://svnweb.freebsd.org/changeset/base/336582 Log: makefs(8): add test case for PR 229929 Fix two failing makefs test cases by adding "-M 1m", which was already used for every other FFS test case. Add a new test case for the underlying issue: with no -M, -m, or -s options, makefs can underestimate image size. PR: 229929 Reported by: Jenkins MFC after: 2 weeks Changes: head/usr.sbin/makefs/tests/makefs_ffs_tests.sh
Additional info: The smallest value of -M that makes this testcase pass is "-M 753665". I think the problem probably lies in ffs_size_dir in usr.sbin/makefs/ffs.c. That's the function that does most of the size estimation. However, it relies on intimate knowledge of FFS's internals. Assigning to the resident FFS expert.
A commit references this bug: Author: asomers Date: Mon Oct 1 15:40:07 UTC 2018 New revision: 339048 URL: https://svnweb.freebsd.org/changeset/base/339048 Log: MFC r336582: makefs(8): add test case for PR 229929 Fix two failing makefs test cases by adding "-M 1m", which was already used for every other FFS test case. Add a new test case for the underlying issue: with no -M, -m, or -s options, makefs can underestimate image size. PR: 229929 Reported by: Jenkins Changes: _U stable/11/ stable/11/usr.sbin/makefs/tests/makefs_ffs_tests.sh
(In reply to Alan Somers from comment #2) Does your commit resolve this bug? If so, please close it.
No, my commit doesn't fix the bug. I just added a test case.
I've published two patches on phabricator that should resolve this: 1. https://reviews.freebsd.org/D35131 2. https://reviews.freebsd.org/D35132 With both of them applied the autocalculate_image_size test works just fine.
(In reply to Kornel Dulęba from comment #6) Thanks for your suggested fixes. I have approved your reviews as they are both accurate and a vast improvement over the current code. Alan, would you like me to commit them?
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ecdc04d006de93eb343ce3b77208abd937d4f8ac commit ecdc04d006de93eb343ce3b77208abd937d4f8ac Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2022-05-16 22:32:10 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2022-05-16 22:32:10 +0000 makefs: fix calculation of file sizes When a new FS image is created we need to calculate how much space each file is going to consume. Fix two bugs in that logic: 1) Count the space needed for indirect blocks for large files. 1) Normally the trailing data of a file is written to a block of frag size, 4 kB by default. However for files that use indirect blocks a full block is allocated, 32kB by default. Take that into account. Adjust size calculations to match what is done in ffs_mkfs routine: * Depending on the UFS version the superblock is stored at a different offset. Take that into account. * Add the cylinder group block size. * All of the above has to be aligned to the block size. Finally, Remove "ncg" variable. It's always 1 and it was used to multiply stuff. PR: 229929 Reviewed by: mckusick MFC after: 2 weeks Sponsored by: Semihalf Submitted by: Kornel Dulęba <mindal@semihalf.com> Differential Revision: https://reviews.freebsd.org/D35131 Differential Revision: https://reviews.freebsd.org/D35132 usr.sbin/makefs/ffs.c | 50 ++++++++++++++++++------------- usr.sbin/makefs/tests/makefs_ffs_tests.sh | 1 - 2 files changed, 30 insertions(+), 21 deletions(-)
done.
(In reply to Alan Somers from comment #9) Great, thanks for the followup. I presume you will close this bug report once you have done the MFC.
(In reply to Kirk McKusick from comment #10) Yep, I'll take care of it.
Thanks for committing the fix. On a side note most makefs ffs tests use -M 1m to specify a minimum size of the fs image. I don't think that's needed any more, i.e. I've run kyua with that flag removed from test cases and everything worked just fine.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=1a2920e96aacc253b4f65405640074bf595c431d commit 1a2920e96aacc253b4f65405640074bf595c431d Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2022-05-16 22:32:10 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2022-06-18 14:17:23 +0000 makefs: fix calculation of file sizes When a new FS image is created we need to calculate how much space each file is going to consume. Fix two bugs in that logic: 1) Count the space needed for indirect blocks for large files. 1) Normally the trailing data of a file is written to a block of frag size, 4 kB by default. However for files that use indirect blocks a full block is allocated, 32kB by default. Take that into account. Adjust size calculations to match what is done in ffs_mkfs routine: * Depending on the UFS version the superblock is stored at a different offset. Take that into account. * Add the cylinder group block size. * All of the above has to be aligned to the block size. Finally, Remove "ncg" variable. It's always 1 and it was used to multiply stuff. PR: 229929 Reviewed by: mckusick Sponsored by: Semihalf Submitted by: Kornel Dulęba <mindal@semihalf.com> Differential Revision: https://reviews.freebsd.org/D35131 Differential Revision: https://reviews.freebsd.org/D35132 (cherry picked from commit ecdc04d006de93eb343ce3b77208abd937d4f8ac) usr.sbin/makefs/ffs.c | 50 ++++++++++++++++++------------- usr.sbin/makefs/tests/makefs_ffs_tests.sh | 1 - 2 files changed, 30 insertions(+), 21 deletions(-)