Summary: | makefs(8) can underestimate image size | ||
---|---|---|---|
Product: | Base System | Reporter: | Alan Somers <asomers> |
Component: | bin | Assignee: | Kirk McKusick <mckusick> |
Status: | Closed FIXED | ||
Severity: | Affects Many People | CC: | kornel555 |
Priority: | --- | Flags: | asomers:
mfc-stable13+
asomers: mfc-stable12- |
Version: | CURRENT | ||
Hardware: | Any | ||
OS: | Any |
Description
Alan Somers
2018-07-21 17:20:58 UTC
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(-) |