Page MenuHomeFreeBSD

rc: fix dependencies for growfs
ClosedPublic

Authored by trasz on Apr 13 2021, 3:45 PM.

Details

Summary

Previously it depended on sysctl, which itself has no dependencies, so rcorder(8)
had a bit too much flexibility when choosing when to run it. Make sure it runs just
between 'fsck' and 'root'.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

trasz requested review of this revision.Apr 13 2021, 3:45 PM

Looking at the comment in the original commit:

# TODO: Figure out where this should really be ordered.
# I suspect it should go just after fsck but before mountcritlocal
# but it's hard to tell for sure because of the bug described
# below.

How did you determine that it wasn't necessary? What testing have you done to verify this? There WAS most definitely an issue when I committed it that prevented it from working at mountcritlocal. Things may have changed, but also, there are lots of different configs that this is now used for from arm SD card images to VMs to cloud providers that should be tested before committing.

Do you remember any details about the bug, or the testing setup?

The problem with current ordering is that it's not really defined when the sysctl script runs - it has no dependencies, so it's up to rcorder(8), and from what I've seen it's not very stable about sorting.

Do you remember any details about the bug, or the testing setup?

The problem with current ordering is that it's not really defined when the sysctl script runs - it has no dependencies, so it's up to rcorder(8), and from what I've seen it's not very stable about sorting.

It is likely that I was testing this on system that was before growfs (committed in 2014, and likely testing on stable, not -current) grew the ability to grow on a read-write file system (2012), so at the time it had to be before mountcritlocal. like the comment says, it should likely go after fsck so that the FS is clean, but I think now that growfs works fine on UFS2 and on read-write file systems.

it's fine to move it later, BUT it should still be before things start writing to the root fs, good example of this is hostid_save. Some images are created w/ zero free space, so even creating a small file may fail.

Maybe it should be:

  1. BEFORE: root
  2. REQUIRE: fsck

Which makes sense to me, only after fsck has verified that the fs is clean, but before root becomes read-write.

I'd say if you agree, that we can drop the TODO comment as well, as that is no longer a todo. :)

Makes perfect sense, but growfs depends on awk(1), which might be on a separate /usr, which gets mounted by mountcritlocal, which runs after root.

I wonder if it would be possible to somehow extend geom(8) to make the walk through the hierarchy easier, to not require complicated text processing?

Makes perfect sense, but growfs depends on awk(1), which might be on a separate /usr, which gets mounted by mountcritlocal, which runs after root.

I wonder if it would be possible to somehow extend geom(8) to make the walk through the hierarchy easier, to not require complicated text processing?

Yea, the first two awks are trivial and could easily be replaced. The third one, however, would be tricky since it uses arrays. Since this is a common operation, it may make sense to have geom(8) have a recursive resize like is implemented here with awk spawning the proper sequence of commands.

Also, growfs is generally only ever done once, on firstboot, and that's always done with a clean filesystem, so the urgency of this change is rather low, imho, since the current ordering works for SD card images in embedded situations.

Makes perfect sense, but growfs depends on awk(1), which might be on a separate /usr, which gets mounted by mountcritlocal, which runs after root.

I wonder if it would be possible to somehow extend geom(8) to make the walk through the hierarchy easier, to not require complicated text processing?

Except since this is expanding root and not /usr, it is unlikely to do anything if awk is on /usr, as /usr is often after / preventing any expansion.

I guess the correct thing would be more likely to make sure awk exists before doing any processing.

Feel free to expand geom part to handle recursive, but IMO, that is a lot of extra code that isn't needed just to make an operation that isn't likely to work, work.

looks good.

like the comment, very nice and clear.

This revision is now accepted and ready to land.Apr 21 2021, 8:51 PM

Like the comment documenting the exception to the early rc scripts not requiring /usr.

This revision was automatically updated to reflect the committed changes.