Re: Some concern about the journal support

[Pavel Roskin]

On Sat, 2008-06-14 at 01:05 +0800, Bean wrote:

> Please see if my last patch works. Basically, it use read_hook as an
> indicator. If read_hook is not null, which means we need to get a
> block location, it use the fs address. If read_hook is null, which
> means we don't care about the location, it use the journal address.
> This should fit the problem.

My testing shows that we still have some problems with journal support.
I could not read /home in grub-fstest, and it didn't work when I
reverted your patch.  It worked with the version immediately before you
added journal support on May 20, but didn't work with the version where
the journal support was added.  After I returned to the current code, I
could read /home again - apparently something changed on the filesystem
in the meantime.

Your patch should fix the issue with hardcoding block locations, if I
understand correctly.

I was asking where journal support would be beneficial in userspace at
all.  And it has to be asked if journal support is useful at the boot
time.

We have some code that is hard to get right and hard to test.  Yet it
will be run every time an unclean ext3 filesystem is found at the boot
time.  What are we gaining?  What is the situation where using the
journal is beneficial?  How likely is it to happen?  Is it possible that
we may be better off not using the journal?  How likely is that?

The standard use of the journal is to make the filesystem consistent
after an unclean shutdown without having to run a time-consuming fsck.
Since grub is not writing anything to the disk, consistency is not
really important.  What's important got grub is reliability and ability
to access all files on the filesystem.

> btw, the reiserfs driver is a little strange, it don't follow the
> normal path of grub_fshelp_read_file, perhaps we just disable the
> journal at the moment.

My impression is that we need to make journal support experimental and
disabled by default for both ext3 and reiserfs.  It should only be
enabled by default if there are a good arguments why it's useful and a
testsuite to prove that it's reliable.

-- 
Regards,
Pavel Roskin