Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover automatic block number calculation #5

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

xOneca
Copy link
Contributor

@xOneca xOneca commented May 13, 2019

Total block count was removed on commit 457078c without explanation. Bring it back with a change: instead of increasing the number of blocks specified by the user (like it used to do), we keep the current behavior of crying to the user.

Xabier Oneca added 4 commits May 13, 2019 13:41
As it was defined could not work in other contexts (the current lone use works
by chance, because multiplication is done first), where the division may yield
an unexpected (but valid) zero result.
The total block count was once done automatically but it was lost on commit
457078c.

The calculation is recovered, but made optional: it is only done if total blocks
is not passed by cmdline. If passed, that value is used and not touched.

Also, reserved_frac is taken into account on the automatic calculation.
Must take into account the reserved blocks.
@xOneca
Copy link
Contributor Author

xOneca commented May 14, 2019

Although tests pass flawlessly, it seems that there are some blocks that are not being counted in statistics, and thus there aren't enough blocks for some not-so-large folders.

@bestouff
Copy link
Owner

Could you please add a test demonstrating the problem ?

@xOneca
Copy link
Contributor Author

xOneca commented May 14, 2019

I just make && ./genext2fs -v -m 0 -d . ../aa the project and it gives:

genext2fs 1.4.2
./genext2fs: couldn't allocate a block (no free space)

I don't say that the flaw is in stats gathering, but I think there must be some blocks that must be taken into account, that aren't. If I remove the -m 0 it creates the fs without throubles, but it should not be using those reserved blocks, should it?

Add indirect blocks to file block count stats.
genext2fs.c Outdated
@@ -166,8 +166,8 @@ static int blocksize = 1024;
#define BLOCKS_PER_GROUP 8192
#define INODES_PER_GROUP 8192
/* Percentage of blocks that are reserved.*/
#define RESERVED_BLOCKS 5/100
#define MAX_RESERVED_BLOCKS 25/100
#define RESERVED_BLOCKS (5/100.)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not fan of introducing floats just for this.
Could you please instead use something like

#define TO_RESERVED_BLOCKS(n) ((n) * 5 / 100)

?

Leave the two constants as were defined originally. This reverts the changes
made by commit bfc28c7.
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