[patch] grub incorrectly identifies ext3 as fat

[patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi all,

grub (both the boot loader, and grub-probe) incorrectly identifies my
ext3 partition as containing a fat file system. This means it can't
boot without manual tweaking.  The problem is caused by stale fat
signatures... this is probably a common problem, as mke2fs often
doesn't wipe old signatures.

I wrote a patch.  In order for a file system to be considered
detected, dir() must not only succeed, it also must find at least one
file or directory. This is pretty effective at ruling out misdetecting
a filesystem based on a stale signature that wasn't wiped by mkfs.
This shouldn't have any collateral damage, because empty file systems
are useless to Grub.

I filed a bug report here:
https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/463015

--- grub-1.97~beta4/kern/fs.c 2009-07-16 18:14:09.000000000 -0400
+++ grub-1.97~beta4-fixed/kern/fs.c 2009-10-28 22:36:49.000000000 -0400
@@ -65,12 +65,14 @@
 grub_fs_probe (grub_device_t device)
 {
   grub_fs_t p;
+ int found_file;
   auto int dummy_func (const char *filename,
          const struct grub_dirhook_info *info);

   int dummy_func (const char *filename __attribute__ ((unused)),
     const struct grub_dirhook_info *info __attribute__ ((unused)))
     {
+ found_file = 1;
       return 1;
     }

@@ -82,8 +84,10 @@
       for (p = grub_fs_list; p; p = p->next)
  {
    grub_dprintf ("fs", "Detecting %s...\n", p->name);
+
+ found_file = 0;
    (p->dir) (device, "/", dummy_func);
- if (grub_errno == GRUB_ERR_NONE)
+ if (grub_errno == GRUB_ERR_NONE && found_file)
      return p;

    grub_error_push ();

Cheers,
Andrew

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Thu, Oct 29, 2009 at 02:58:09PM +0000, Andrew Clausen wrote:
> Hi all,
> 
> grub (both the boot loader, and grub-probe) incorrectly identifies my
> ext3 partition as containing a fat file system. This means it can't
> boot without manual tweaking.  The problem is caused by stale fat
> signatures... this is probably a common problem, as mke2fs often
> doesn't wipe old signatures.
> 
> I wrote a patch.  In order for a file system to be considered
> detected, dir() must not only succeed, it also must find at least one
> file or directory. This is pretty effective at ruling out misdetecting
> a filesystem based on a stale signature that wasn't wiped by mkfs.

We already had code for this, see:

2009-09-05  Robert Millan  <rmh.grub@aybabtu.com>

        * util/grub-probe.c (probe): Comment out buggy codepath, which  
        was unexpectedly enabled by Colin Watson's 2009-09-02 fix.  This
        should be re-enabled after 1.97.

> This shouldn't have any collateral damage, because empty file systems
> are useless to Grub.

Unlike the filesystem driver, grub-probe already knows we want to probe for
a file.  Then this assumption won't be necessary.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Felix Zielcke]

Am Freitag, den 30.10.2009, 19:57 +0100 schrieb Robert Millan:
> On Thu, Oct 29, 2009 at 02:58:09PM +0000, Andrew Clausen wrote:
> > Hi all,
> > 
> > grub (both the boot loader, and grub-probe) incorrectly identifies my
> > ext3 partition as containing a fat file system. This means it can't
> > boot without manual tweaking.  The problem is caused by stale fat
> > signatures... this is probably a common problem, as mke2fs often
> > doesn't wipe old signatures.
> > 
> > I wrote a patch.  In order for a file system to be considered
> > detected, dir() must not only succeed, it also must find at least one
> > file or directory. This is pretty effective at ruling out misdetecting
> > a filesystem based on a stale signature that wasn't wiped by mkfs.
> 
> We already had code for this, see:
> 
> 2009-09-05  Robert Millan  <rmh.grub@aybabtu.com>
> 
>         * util/grub-probe.c (probe): Comment out buggy codepath, which  
>         was unexpectedly enabled by Colin Watson's 2009-09-02 fix.  This
>         should be re-enabled after 1.97.

The problem with this (well not the problem why this was reverted) is
that would just abort with an error if grub-probe detects the wrong
filesystem.
Ok would still be better then as currently embeding the wrong fs module
to core.img

-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi Robert,

2009/10/30 Robert Millan <rmh@aybabtu.com>:
>> I wrote a patch.  In order for a file system to be considered
>> detected, dir() must not only succeed, it also must find at least one
>> file or directory. This is pretty effective at ruling out misdetecting
>> a filesystem based on a stale signature that wasn't wiped by mkfs.
>
> We already had code for this, see:
>
> 2009-09-05  Robert Millan  <rmh.grub@aybabtu.com>
>
>        * util/grub-probe.c (probe): Comment out buggy codepath, which
>        was unexpectedly enabled by Colin Watson's 2009-09-02 fix.  This
>        should be re-enabled after 1.97.

Yes, that's a similar idea.

>> This shouldn't have any collateral damage, because empty file systems
>> are useless to Grub.
>
> Unlike the filesystem driver, grub-probe already knows we want to probe for
> a file.  Then this assumption won't be necessary.

What about probing in the boot-loader?  My system was still unbootable
when I manually told grub to include the ext2 driver core.img, since
the fat driver was erroneously included as well, and it couldn't read
the root file system.  (You can manually do "rmmod fat", but this is
nasty!  Recall that I grub couldn't load the extra modules for the
menus, etc. at this point)

Cheers,
Andrew

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Fri, Oct 30, 2009 at 03:19:31PM -0400, Andrew Clausen wrote:
> >> This shouldn't have any collateral damage, because empty file systems
> >> are useless to Grub.
> >
> > Unlike the filesystem driver, grub-probe already knows we want to probe for
> > a file.  Then this assumption won't be necessary.
> 
> What about probing in the boot-loader?

It's too late then.  grub-install needs to know which filesystem module has
to be included in core.img.

It also should verify (though currently doesn't, as noted before) that the
files it wants are indeed readable.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi,

2009/10/30 Robert Millan <rmh@aybabtu.com>:
>> What about probing in the boot-loader?
>
> It's too late then.  grub-install needs to know which filesystem module has
> to be included in core.img.

What if the core.img contains modules for 2 or more file systems?

Cheers,
Andrew

Re: [patch] grub incorrectly identifies ext3 as fat

[Felix Zielcke]

Am Samstag, den 31.10.2009, 02:44 -0400 schrieb Andrew Clausen:
> Hi,
> 
> 2009/10/30 Robert Millan <rmh@aybabtu.com>:
> >> What about probing in the boot-loader?
> >
> > It's too late then.  grub-install needs to know which filesystem module has
> > to be included in core.img.
> 
> What if the core.img contains modules for 2 or more file systems?
> 

That only happens if you use grub-install with --modules or you directly
create it with grub-mkimage.
By default there's just one module and there only needs to be one.
The one to access /boot/grub.
It doestn't make sense to include more then one fs module into core.img
With grub.efi the situation seems to be different though. But IMO it's a
bug there.


-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Sat, Oct 31, 2009 at 02:44:08AM -0400, Andrew Clausen wrote:
> Hi,
> 
> 2009/10/30 Robert Millan <rmh@aybabtu.com>:
> >> What about probing in the boot-loader?
> >
> > It's too late then.  grub-install needs to know which filesystem module has
> > to be included in core.img.
> 
> What if the core.img contains modules for 2 or more file systems?

This should be a last ressort.  Fortunately, we have a better solution,
which you pointed out youself: make sure the filesystem works by actually
reading the file.

Felix, did you have some code to do make_path_relative_to_its_root() in C?
Then we can re-enable the check in grub-probe, fixing this problem.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Felix Zielcke]

Am Samstag, den 31.10.2009, 11:10 +0100 schrieb Robert Millan:
> 
> Felix, did you have some code to do make_path_relative_to_its_root()
> in C?
> Then we can re-enable the check in grub-probe, fixing this problem. 

Yes, the only missing thing is that you wanted to have the function
imported from gnulib for the systems which don't have realpath()
Would be easier if we didn't support mingw already, but then I can also
look into including asprintf() from there.

-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Sat, Oct 31, 2009 at 11:34:22AM +0100, Felix Zielcke wrote:
> Am Samstag, den 31.10.2009, 11:10 +0100 schrieb Robert Millan:
> > 
> > Felix, did you have some code to do make_path_relative_to_its_root()
> > in C?
> > Then we can re-enable the check in grub-probe, fixing this problem. 
> 
> Yes, the only missing thing is that you wanted to have the function
> imported from gnulib for the systems which don't have realpath()

But realpath() is posix AFAIK.  Or you mean the realpath(name, NULL) GNU
extension?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Felix Zielcke]

Am Samstag, den 31.10.2009, 12:30 +0100 schrieb Robert Millan:
> On Sat, Oct 31, 2009 at 11:34:22AM +0100, Felix Zielcke wrote:
> > Am Samstag, den 31.10.2009, 11:10 +0100 schrieb Robert Millan:
> > > 
> > > Felix, did you have some code to do make_path_relative_to_its_root()
> > > in C?
> > > Then we can re-enable the check in grub-probe, fixing this problem. 
> > 
> > Yes, the only missing thing is that you wanted to have the function
> > imported from gnulib for the systems which don't have realpath()
> 
> But realpath() is posix AFAIK.  Or you mean the realpath(name, NULL) GNU
> extension?
> 

realpath(name,NULL) is POSIX too now.
The only problem I know about is that we support mingw32, i.e. plain
windows not cygwin.
If we say we don't support this anymore then I don't think we need to
import the gnulib function.


-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi Felix,

2009/10/31 Felix Zielcke <fzielcke@z-51.de>:
> That only happens if you use grub-install with --modules or you directly
> create it with grub-mkimage.
> By default there's just one module and there only needs to be one.
> The one to access /boot/grub.
> It doestn't make sense to include more then one fs module into core.img
> With grub.efi the situation seems to be different though. But IMO it's a
> bug there.

What if you have a dual boot setup, with say ntfs and ext3?

When you go to boot from a kernel from ext3, it might break if the
user loaded the ntfs module.

Isn't it easy to just fix the bug?

Cheers,
Andrew

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Sat, Oct 31, 2009 at 11:39:16AM -0400, Andrew Clausen wrote:
> Hi Felix,
> 
> 2009/10/31 Felix Zielcke <fzielcke@z-51.de>:
> > That only happens if you use grub-install with --modules or you directly
> > create it with grub-mkimage.
> > By default there's just one module and there only needs to be one.
> > The one to access /boot/grub.
> > It doestn't make sense to include more then one fs module into core.img
> > With grub.efi the situation seems to be different though. But IMO it's a
> > bug there.
> 
> What if you have a dual boot setup, with say ntfs and ext3?

The filesystem module that is embedded in core.img is only for bootstrap
purposes.  Once GRUB can access /boot/grub/, it automatically loads the
modules required for menu entries.

> Isn't it easy to just fix the bug?

First of all, it's not a bug.  Filesystems weren't designed to be identifiable
reliably.  They could have been, but they weren't, and now we're stuck with
that. Everything GRUB does to archieve filesystem detection is on a BEST
EFFORT basis.

With that in mind, we can find ways in which GRUB will be more succesful at
identifiing them.  For example (and we already do this), the search command
gives priority to filesystem modules that are already loaded.

Or we can attempt to read a given file when we expect it's there.  For
example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
filesystem layer, so that it will require it as a precondition.

There are many ways to improve this, but making arbitrary assumptions about
the content of a filesystem (e.g. non-emptyness) doesn't sound like the best
solution.  In this particular case, you can be hit by both false positives
and false negatives.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[rubisher]

Hello Felix,

Felix Zielcke wrote:
> Am Samstag, den 31.10.2009, 02:44 -0400 schrieb Andrew Clausen:
>> Hi,
>>
>> 2009/10/30 Robert Millan <rmh@aybabtu.com>:
>>>> What about probing in the boot-loader?
>>> It's too late then.  grub-install needs to know which filesystem module has
>>> to be included in core.img.
>> What if the core.img contains modules for 2 or more file systems?
>>
> 
> That only happens if you use grub-install with --modules or you directly
> create it with grub-mkimage.
> By default there's just one module and there only needs to be one.
> The one to access /boot/grub.
> It doestn't make sense to include more then one fs module into core.img
> With grub.efi the situation seems to be different though. But IMO it's a
> bug there.
> 
> 
To boot my debian installation on an ibm p5 lpar, I am trying to install grub2 i.e. working with grub-ieee1275.
I try first to rebuild grub-1.97~beta4 but it was failing.
I finaly find out that I can build and boot grub2 (only) with another gcc optimization option (-O0 in place of default -Os).

But it still failed to boot the linux kernel after grub-install:
1/ here is the chrooted mount points:
/dev/sdb6 on /mnt/NewInst type ext3 (rw)
/dev/sdb3 on /mnt/NewInst/boot type ext3 (rw)
/dev/sdb1 on /mnt/NewInst/boot/grub type vfat (rw)
/dev/mapper/p5tst001vg-var on /mnt/NewInst/var type ext3 (rw)

ps: /mnt/NewInst/boot/grub have to be a mount point for grub-install;
if it wasn't: # grub-install hd1
/boot/grub must be a mount point.
and must be a fat16 (or iirc hfs) for ofs

2/ a typical menu entry:
### BEGIN /etc/grub.d/10_linux ###
menuentry "Debian GNU/Linux, with Linux 2.6.30-2-powerpc64" {
         insmod ext2
         set root=(hd1,3)
         search --no-floppy --fs-uuid --set 16ef0754-c845-46e9-a948-b9669ee68329
         linux   /vmlinux-2.6.30-2-powerpc64 root=UUID=3c51c43e-63a7-4ff1-9b1c-cf98addcb7ed ro  quiet
         initrd  /initrd.img-2.6.30-2-powerpc64
}

and at the grub sh prompt, ls doesn't report any disk?

Even thought: # more /boot/grub/device.map
(hd0)	/dev/sda
(hd1)	/dev/sdb

Could it be for the reason you explained above?

I am worry of it because when I copy vmlinux-2.6.30-2-powerpc64 and initrd.img-2.6.30-2-powerpc64 in the 
/mnt/NewInst/boot/grub fs and this simple menuentry:
### BEGIN /etc/grub.d/10_linux ###
menuentry "Debian GNU/Linux, with Linux 2.6.30-2-powerpc64" {
         linux   /vmlinux-2.6.30-2-powerpc64 root=UUID=3c51c43e-63a7-4ff1-9b1c-cf98addcb7ed ro sysrq=1 insmod=sym53c8xx 
insmod=ipr quiet
         initrd  /initrd.img-2.6.30-2-powerpc64
}

the debian installation finaly boot fine ;<)
The problem is that this vfat fs afaik must be < then 24Mb and btw couldn't contained more then one kernel image (and its 
initrd), though.

Tia for further advise,
	J.

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Sat, Oct 31, 2009 at 12:38:41PM +0100, Felix Zielcke wrote:
> Am Samstag, den 31.10.2009, 12:30 +0100 schrieb Robert Millan:
> > On Sat, Oct 31, 2009 at 11:34:22AM +0100, Felix Zielcke wrote:
> > > Am Samstag, den 31.10.2009, 11:10 +0100 schrieb Robert Millan:
> > > > 
> > > > Felix, did you have some code to do make_path_relative_to_its_root()
> > > > in C?
> > > > Then we can re-enable the check in grub-probe, fixing this problem. 
> > > 
> > > Yes, the only missing thing is that you wanted to have the function
> > > imported from gnulib for the systems which don't have realpath()
> > 
> > But realpath() is posix AFAIK.  Or you mean the realpath(name, NULL) GNU
> > extension?
> > 
> 
> realpath(name,NULL) is POSIX too now.
> The only problem I know about is that we support mingw32, i.e. plain
> windows not cygwin.
> If we say we don't support this anymore then I don't think we need to
> import the gnulib function.

We can continue supporting it, but we don't have to put bugfixes on hold
because of it.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi Robert,

2009/10/31 Robert Millan <rmh@aybabtu.com>:
>> What if you have a dual boot setup, with say ntfs and ext3?
>
> The filesystem module that is embedded in core.img is only for bootstrap
> purposes.  Once GRUB can access /boot/grub/, it automatically loads the
> modules required for menu entries.

OK, here's a realistic use case that could hit lots of users:
* grub is installed on an ext3 partition
* the user has OSX installed on an HFS file system (or whatever they
use now) that has stale ext3 signatures
* when grub tries to load the OSX kernel, it has two filesystem
modules loaded: ext2 and hfs.
* the stale signature causes it to misdetect it as hfs, and it fails.

I suppose you could solve this problem by unloading all filesystem
modules except the ones you need at that moment?  But Grub doesn't do
that now, right?

>> Isn't it easy to just fix the bug?
>
> First of all, it's not a bug.  Filesystems weren't designed to be identifiable
> reliably.  They could have been, but they weren't, and now we're stuck with
> that. Everything GRUB does to archieve filesystem detection is on a BEST
> EFFORT basis.

I think this is where we disagree... I think that with good
heuristics, you can cover all use cases without any problems.

(For instance, GNU Parted has a fairly short list of heuristics that
seem to be very reliable -- or they were when I maintained it.  The
relevant function is ped_file_system_probe().)

Alternatively, you could allow the user to specify the file system?

Or, you could warn when multiple file systems have matching signatures.

> With that in mind, we can find ways in which GRUB will be more succesful at
> identifiing them.  For example (and we already do this), the search command
> gives priority to filesystem modules that are already loaded.

This heuristic could have a lot of problems.  (See my example above.)

> Or we can attempt to read a given file when we expect it's there.  For
> example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
> filesystem layer, so that it will require it as a precondition.

I can see that that would work will for some use cases...

> There are many ways to improve this, but making arbitrary assumptions about
> the content of a filesystem (e.g. non-emptyness) doesn't sound like the best
> solution.  In this particular case, you can be hit by both false positives
> and false negatives.

Huh?  I can't see any way to get "hit" by either for this particular
heuristic.  It reduces the number of false positives, and the false
negatives are irrelevant (because an undetected filesystem is
equivalent to an empty one.)

Big picture: I'm sorry for being irritating... I know that odd
heuristics are an unpleasant technology to determine whether or not a
computer can boot or not.
We both have similar approaches: try to pick something that works well
under difficult circumstances.  I think we differ in the way we think
about use cases.  You don't like my heuristic because it has false
positives and false negatives; but I claim that there is no use case
(realistic or unrealistic) in which my heuristic makes things worse.
Some of your proposed heuristics seem reasonable in addition to my own
one, but I think it's clear that adding my heuristic will always make
Grub work better.

I think this is very important: I spent hours trying to get my
computer to boot.  The error messages ("error: file not found", and ls
coming up empty, etc.) were uninformative, and I really had to think
hard to figure out the problem.  Realistically, I think almost all
users would never have figured out the problem without reporting a
bug.  Being unable to boot Linux is quite a show-stopper.

Cheers,
Andrew

Re: [patch] grub incorrectly identifies ext3 as fat

[Felix Zielcke]

Am Samstag, den 31.10.2009, 18:02 +0000 schrieb rubisher:
> and at the grub sh prompt, ls doesn't report any disk?
> 
> Even thought: # more /boot/grub/device.map
> (hd0)   /dev/sda
> (hd1)   /dev/sdb

device.map is only used for the utilities.
real GRUB never uses it.

> Could it be for the reason you explained above? 

Sorry but I don't know anything at all about Openfirmware.

-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] grub incorrectly identifies ext3 as fat

[Vladimir 'phcoder' Serbinenko]

Andrew Clausen wrote:
> Hi Robert,
>
> 2009/10/31 Robert Millan <rmh@aybabtu.com>:
>   
>>> What if you have a dual boot setup, with say ntfs and ext3?
>>>       
>> The filesystem module that is embedded in core.img is only for bootstrap
>> purposes.  Once GRUB can access /boot/grub/, it automatically loads the
>> modules required for menu entries.
>>     
>
> OK, here's a realistic use case that could hit lots of users:
> * grub is installed on an ext3 partition
> * the user has OSX installed on an HFS file system (or whatever they
> use now) that has stale ext3 signatures
> * when grub tries to load the OSX kernel, it has two filesystem
> modules loaded: ext2 and hfs.
> * the stale signature causes it to misdetect it as hfs, and it fails.
>
>   
hfs+ and ext2 use same sector as superblock so it's not a problem
> I suppose you could solve this problem by unloading all filesystem
> modules except the ones you need at that moment?  But Grub doesn't do
> that now, right?
>
>   
>>> Isn't it easy to just fix the bug?
>>>       
>> First of all, it's not a bug.  Filesystems weren't designed to be identifiable
>> reliably.  They could have been, but they weren't, and now we're stuck with
>> that. Everything GRUB does to archieve filesystem detection is on a BEST
>> EFFORT basis.
>>     
>
> I think this is where we disagree... I think that with good
> heuristics, you can cover all use cases without any problems.
>
> (For instance, GNU Parted has a fairly short list of heuristics that
> seem to be very reliable -- or they were when I maintained it.  The
> relevant function is ped_file_system_probe().)
>
> Alternatively, you could allow the user to specify the file system?
>
> Or, you could warn when multiple file systems have matching signatures.
>
>   
Or you can modify the tools to clear first and last MiB on mkfs which
solves problem at the root
>> With that in mind, we can find ways in which GRUB will be more succesful at
>> identifiing them.  For example (and we already do this), the search command
>> gives priority to filesystem modules that are already loaded.
>>     
>
> This heuristic could have a lot of problems.  (See my example above.)
>
>   
Any heuristic means that something is wrong.
>> Or we can attempt to read a given file when we expect it's there.  For
>> example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
>> filesystem layer, so that it will require it as a precondition.
>>     
>
> I can see that that would work will for some use cases...
>
>   
It breaks encapsulation and makes code a lot less maintainable. And just
offset clear bug to a lot of strange and weird bugs
>> There are many ways to improve this, but making arbitrary assumptions about
>> the content of a filesystem (e.g. non-emptyness) doesn't sound like the best
>> solution.  In this particular case, you can be hit by both false positives
>> and false negatives.
>>     
>
> Huh?  I can't see any way to get "hit" by either for this particular
> heuristic.  It reduces the number of false positives, and the false
> negatives are irrelevant (because an undetected filesystem is
> equivalent to an empty one.)
>   
Many filesystems would look having some weird filenames but still having
a directory if they are false positives.
> Big picture: I'm sorry for being irritating... I know that odd
> heuristics are an unpleasant technology to determine whether or not a
> computer can boot or not.
> We both have similar approaches: try to pick something that works well
> under difficult circumstances.  I think we differ in the way we think
> about use cases.  You don't like my heuristic because it has false
> positives and false negatives; but I claim that there is no use case
> (realistic or unrealistic) in which my heuristic makes things worse.
> Some of your proposed heuristics seem reasonable in addition to my own
> one, but I think it's clear that adding my heuristic will always make
> Grub work better.
>   
Every heurisitic makes code less clear and more prone to bugs and in
long run results only in more heurisitc failures.

-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Sat, Oct 31, 2009 at 02:43:19PM -0400, Andrew Clausen wrote:
> > Or we can attempt to read a given file when we expect it's there.  For
> > example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
> > filesystem layer, so that it will require it as a precondition.
> 
> I can see that that would work will for some use cases...

Most importantly, it's a net win.  If we know a file is there, there's no
harm in requiring that the filesystem driver is capable of reading it.

It's a pity, because we already had this check, and we were forced to
disable it.  Would you like to help us restore it?  I can give more details.

> I think this is very important: I spent hours trying to [...]

Nobody questions that.  It certainly is very important to me, but even then,
we should look for solutions that are good in long-term.

When using heuristics, if one's not careful can end up in a slippery slope,
in which adding an heuristic fixes a problem at the cost of introducing a
new one, and then you can only fix that by adding yet another heuristic
(which can be repeated ad nauseam).

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Robert Millan]

On Sat, Oct 31, 2009 at 08:15:56PM +0100, Robert Millan wrote:
> On Sat, Oct 31, 2009 at 02:43:19PM -0400, Andrew Clausen wrote:
> > > Or we can attempt to read a given file when we expect it's there.  For
> > > example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
> > > filesystem layer, so that it will require it as a precondition.
> > 
> > I can see that that would work will for some use cases...
> 
> Most importantly, it's a net win.  If we know a file is there, there's no
> harm in requiring that the filesystem driver is capable of reading it.
> 
> It's a pity, because we already had this check, and we were forced to
> disable it.  Would you like to help us restore it?  I can give more details.

Btw, the grub-probe check has just been reenabled in our experimental
branch (see http://www.gnu.org/software/grub/grub-2-download.en.html).

Perhaps you could test it and report if it fixes your problem?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi Vladmir,

Thanks for your reply.  Sorry I didn't get back to you -- I had some
mail problems.

2009/10/31 Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>:
> hfs+ and ext2 use same sector as superblock so it's not a problem

Ok, what about NTFS and ext2 then?

> Or you can modify the tools to clear first and last MiB on mkfs which
> solves problem at the root

I agree... but unfortunately there are many file systems created a
long time ago that have this signature problem.

>> This heuristic could have a lot of problems.  (See my example above.)
>>
> Any heuristic means that something is wrong.

Agreed... if there were enough partition type IDs for everyone, or
everyone agreed on where to put superblocks, we wouldn't have this
problem.  We're forced to use heuristics because there is no standard.

>>> Or we can attempt to read a given file when we expect it's there.  For
>>> example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
>>> filesystem layer, so that it will require it as a precondition.
>>>
>>
>> I can see that that would work will for some use cases...
>>
> It breaks encapsulation and makes code a lot less maintainable. And just
> offset clear bug to a lot of strange and weird bugs

Grub currently uses heuristics... grub_fs_probe() arbitrarily picks
the first match, which is a terrible heuristic which made my computer
unbootable.

How can you avoid heuristics in this situation?  You could require the
user to specify the file system?

> Many filesystems would look having some weird filenames but still having
> a directory if they are false positives.

I agree that my patch does not eliminate all false positives.  But it
does not add any new ones.

> Every heurisitic makes code less clear and more prone to bugs and in
> long run results only in more heurisitc failures.

Does this mean you want to remove grub_fs_probe() ?

Cheers,
Andrew

Re: [patch] grub incorrectly identifies ext3 as fat

[Andrew Clausen]

Hi Robert,

Sorry, I didn't follow up on your suggestion before (my mail program
didn't highlight it properly).

I had some bad luck: I can no longer reproduce the problem with any
version of grub.  This is a surprise to me, since my file system is 2
years old.

Anyway, I just want to restate some points that I think got confused
in the earlier discussion:

(1) If we improve the detection code in kern/fs.c, it fixes both the
boot time AND the grub-probe detection.

(2) I think it's important to have good detection not just in
grub-probe, but also at boot time.  When loading in the kernel,
initrd, and so on, grub needs to know which file system to use.  It
got confused when I had both fat and ext2 modules loaded.  (I
eventually got it to boot by typing "rmmod fat" first.)

Cheers,
Andrew

2009/11/9 Robert Millan <rmh@aybabtu.com>:
> On Sat, Oct 31, 2009 at 08:15:56PM +0100, Robert Millan wrote:
>> On Sat, Oct 31, 2009 at 02:43:19PM -0400, Andrew Clausen wrote:
>> > > Or we can attempt to read a given file when we expect it's there.  For
>> > > example, if we're looking for /boot/grub/, we can tell "/boot/grub" to the
>> > > filesystem layer, so that it will require it as a precondition.
>> >
>> > I can see that that would work will for some use cases...
>>
>> Most importantly, it's a net win.  If we know a file is there, there's no
>> harm in requiring that the filesystem driver is capable of reading it.
>>
>> It's a pity, because we already had this check, and we were forced to
>> disable it.  Would you like to help us restore it?  I can give more details.
>
> Btw, the grub-probe check has just been reenabled in our experimental
> branch (see http://www.gnu.org/software/grub/grub-2-download.en.html).
>
> Perhaps you could test it and report if it fixes your problem?
>
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>