Arch Linux

Hrm, I don't know how I feel about this. I personally want shutdown to be faster than startup, but I could see people liking this.

With that in mind, a couple of things:

a) Could you make this work in BOTH cases via a rc.conf variable? That would allow us to keep the current behavior, bt have users turn on a "fsck on shutdown" flag if they want

b) As this is a full patch, I'd recommend putting the shutdown_fsck stuff in /etc/rc.d/functions itself

c) How does this behave when using pm-suspend or pm-hibernate? It's worth a test, if you ask me

a) Yes, using a rc.conf variable makes sense.

You could add something like the following:
#
# SHUTDOWN_FSCK: Check all filesystems at shutdown (if needed),
# to make startup time more uniform.
#
SHUTDOWN_FSCK="yes"


b) It probably doesn't belong to /etc/rc.d/functions either since it's used just in one place. In rc.sysinit, the fsck code is just pasted inline so I did something similar with rc.shutdown. I'm attaching a new patch which does away with the shutdown_fsck file.


c) I believe pm-suspend and pm-hibernate don't actually go through the shutdown process. I tried pm-suspend and it works correctly.


Thanks for your comments.

Regarding (b) - I assumed the fsck code would be identical to the startup fsck code, in which case it could be factored out into /etc/rc.d/functions and just called twice.

I didn't compare though - is it the same code?

It's different enough to make refactoring not worth the hassle, IMHO (the function would be 4 lines long).
Error handling is done differently because:
I) When a reboot is needed we don't reboot, we just halt
II) When a fatal error occurs we don't want to be dumped to a shell: the shutdown might be unattended, while a bootup, usually, is not.

Ok, the rationale makes sense. Everything looks good to me.

Would you mind adding a git patch against the initscripts tree that includes an addition of the value to rc.conf with a little documentation (default it to off or commented out or something)

http://projects.archlinux.org/initscripts.git/



Thomas, do you see anything wrong with this patch?

It looks fine to me. The patch should also include a change in the default rc.conf though.

If we could somehow reduce the code duplication a bit, it would be nice.

I think the patch needs some update because there are already patches for fsck -C file descriptor support in git. http://projects.archlinux.org/initscripts.git/

Aye, that's why I suggested a git patch - to pick up unreleased changes.

I'm sorry for scrolling you away to fast, but maybe an explicit hint didn't hurt in this case. :-/

Ok, I'll update my patch and refactor a little since Thomas seems to dislike redundancy. Expect a new function in /etc/rc.d/functions.

@Kurt: thanks for the hint.

Updated git patch. Includes rc.sysinit refactoring.

fsck_all function should be corrected:
* provide variable fsckret to the hooks as before
* use a local variable (or maybe just set it and simply drop return and fsckret assignments after calling?)
* run the postfsck hook right after stat_fail/stat_done instead of outside
* use correct hook names ${0##*/rc.}_prefsck and ${0##*/rc.}_postfsck (or should we simply rename the hooks to 'prefsck' and 'postfsck'?)
On shutdown there is a 20 minutes sleep when fsck fails. Isn't that a bit long?

* /forcefsck is removed on sysinit only. So don't do forced fsck on shutdown (and again on sysinit).
* NETFS is needed for mount too.

Currently there is no support for checking loop filesystems (like pacman-db-cage)
This patch on top of the previous would add it:

Kurt, would you mind if I modified your patch to do forced fsck on shutdown and remove the /forcefsck file? Or is it better to do forced fsck on sysinit for some reason?

No problem to me, but you would need to mount / rw again to do it.

You're right. Probably too much of a hassle.
Also, 20m were meant to be 20s.
Other than this, your patches look good to me.

So this one corrects the sleep.

... and this one would add loop fsck to rc.shutdown too:

Ok, just to confirm, as you guys are iterating nicely here - all 5 patches above can/should be applied for this, correct?

Thomas, would you mind reviewing, in case I missed anything?

Give me a thumbs up and I'll merge these

To be precise: Patches 0001..0005 should be applied if we want fsck on shutdown *and* additionally fsck for loop file-systems.

The fsck reboot condition has been broken. This one corrects it.

Hm, we haven't applied this yet, I'll see when I can get a change to review it. I don't know if that will be anytime soon, so Aaron if you think they are good, please apply them.

I updated and cleaned up the patches a bit.
edit: Added a patch to avoid duplicate fsck when forced.

These patches are very interesting, and seems to be quite complete. Why haven't been applied yet? (or, are you planning to apply?)

@Kurt: thanks for your work on this, and sorry for taking forever to react.

I have now looked at your patches, and while I don't like the idea of fsck on shutdown, I _do_ like the patches ;-)

This is what I propose:

I will accept patches that move split up and move all our fsck stuff into functions (essentially your first patch, without adding new hooks, and without changing rc.shutdown).
I will also accept a patch to make it possible to not fsck on boot (preferable via a kernel command line argument, and preferably something that has been used by someone else before, so we don't introduce new stuff).
Finally, I'd be happy to add one new hook if needed to rc.shutdown.

That should allow the people who want to use this functionality to add a hook file (something for AUR?), and to boot with the correct kernel parameter.

What do you think? Is there still interest in this?

To disable fsck on boot, one would just need to override fsck_all() by a null-function. I don't know if disabling fsck via kernel command line would be a good idea.

@Kurt: good point. Could you possibly post your patches to the projects mailing list so more people can have a look?

Looks good by the way. Thanks for your work!

@Tom: done.

@Kurt: Thanks! I pushed your patches. I assume whatever hooks are needed can be made into an AUR package, posted on a forum or something similar, so I'll close this as "implemented".