Re: grub 0.96 bug

Re: grub 0.96 bug

[Molle Bestefich]

[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]

> grub> device (hd0) /dev/mapper/hpt37x_ehgjaggaf
> grub> device (hd0,5) /dev/mapper/hpt37x_ehgjaggaf6
> grub> root (hd0,5)/boot
> Attempt to open drive 0x80 (/dev/mapper/hpt37x_ehgjaggaf6)
> Error 5: Partition table invalid or corrupt
> grub> root (hd0,5)/boot
> Attempt to open drive 0x80 (/dev/mapper/hpt37x_ehgjaggaf6)
> Error 5: Partition table invalid or corrupt

Think it tries to read a partition table out of a logical partition, humn...
No idea where to look for this bug; hints appreciated.

> grub> root (hd0,5)/boot
> Attempt to open drive 0x80 (/dev/mapper/hpt37x_ehgjaggaf)
> Error 18: Selected cylinder exceeds maximum supported by BIOS

Examining further, GRUB thinks the disk has only 4999680 sectors.
It corresponds to about 2.441 MB, while the disk is really about 160.000 MB.

Looking through the source code, the above sector value is some insane
default in GRUB.
The code tries to use 3 different ways of getting the sector count:

1.) ioctl(fd, HDIO_GETGEO) call in lib/device.c
Fails with error -1.
Documentation/hdio.txt from kernel sources has this to say with
regards to that function:
"Not particularly useful with modern disk drives, whose geometry is a
polite fiction anyway. "

2.) fstat (fd, &st)
Fails with error 0.
No idea what this should do :-).

3.) ioctl(fd, BLKGETSIZE) call in lib/device.c
This works for device-mapped devices!
Seems to be a widely used function, there's even a 64-bit version.
Unfortunately, GRUB only attempts this approach after having
*successfully* retrieved cylinder, head and sector values through
HDIO_GETGEO.


I've gone ahead and solved my problems myself; the attached patch
fixes GRUB 0.96 to work correctly (at least for me) with device-mapped
devices.

This is my first hack ever with anything Linux-ie.
So go ahead and flame at will :-).

[-- Attachment #2: grub-0.96-devicemapper.patch --]
[-- Type: application/octet-stream, Size: 1895 bytes --]

diff -aur grub-0.96-orig/lib/device.c grub-0.96/lib/device.c
--- grub-0.96-orig/lib/device.c	2005-03-21 15:07:17.394333000 +0100
+++ grub-0.96/lib/device.c	2005-03-21 15:47:56.695502856 +0100
@@ -147,6 +147,8 @@
 
   /* XXX This is the default size.  */
   geom->sector_size = SECTOR_SIZE;
+  /* Use 0 to test later on if sector value has already been correctly retrieved. */
+  geom->total_sectors = 0;
   
 #if defined(__linux__)
   /* Linux */
@@ -154,17 +156,16 @@
     struct hd_geometry hdg;
     unsigned long nr;
     
+    if (! ioctl (fd, BLKGETSIZE, &nr))
+      geom->total_sectors = nr;
+    
     if (ioctl (fd, HDIO_GETGEO, &hdg))
       goto fail;
 
-    if (ioctl (fd, BLKGETSIZE, &nr))
-      goto fail;
-    
     /* Got the geometry, so save it. */
     geom->cylinders = hdg.cylinders;
     geom->heads = hdg.heads;
     geom->sectors = hdg.sectors;
-    geom->total_sectors = nr;
     
     goto success;
   }
@@ -236,9 +237,17 @@
        given a proper st_blocks size. */
     if (drive & 0x80)
       {
+       /* If a total sector count has been found and it exceeds CHS capacities,
+	* use large drive placeholder values. */
+       if (geom->total_sectors && (geom->total_sectors >= 255*63*1024)) {
+	geom->cylinders = 1024;
+	geom->heads = 255;
+	geom->sectors = 63;
+       } else {
 	geom->cylinders = DEFAULT_HD_CYLINDERS;
 	geom->heads = DEFAULT_HD_HEADS;
 	geom->sectors = DEFAULT_HD_SECTORS;
+       }
       }
     else
       {
@@ -247,11 +256,13 @@
 	geom->sectors = DEFAULT_FD_SECTORS;
       }
 
-    /* Set the total sectors properly, if we can. */
+   /* If total sectors hasn't been set yet, try and get a proper value. */
+   if (! geom->total_sectors) {
     if (! fstat (fd, &st) && st.st_blocks)
       geom->total_sectors = st.st_blocks;
     else
       geom->total_sectors = geom->cylinders * geom->heads * geom->sectors;
+   }
   }
 
  success:

[-- Attachment #3: oh joy it works.txt --]
[-- Type: text/plain, Size: 248 bytes --]

GNU GRUB  version 0.96  (640K lower / 3072K upper memory)
grub> device (hd0) /dev/mapper/hpt37x_ehgjaggaf

grub> root (hd0,5)
Attempt to open drive 0x80 (/dev/mapper/hpt37x_ehgjaggaf)
Filesystem type is reiserfs, partition type 0x83

grub>

[-- Attachment #4: Type: text/plain, Size: 0 bytes --]

Re: Re: grub 0.96 bug

[Wilfried Weissmann]

> Chris Nicola wrote:
>> Yes I am having the exact same problem.  I am actually just going to try
>> and lower version of grub for now but I will try this patch if that
>> still happens.
> 
> Just did this:
> ==========
> grub> device (hd0) /dev/mapper/hpt37x_ehgjaggaf
> grub> root (hd0,5)/boot
> Filesystem type is reiserfs, partition type 0x83
> grub> setup (hd0)
> ==========
> 
> and it destroyed my RAID0.
> GRUB is such a piece of crap :-).
> 
> It must be severely broken; since if it just sticked to writing to the
> device I pointed it at (hpt37x_ehgjaggaf) which is a RAID0 "virtual
> disk" / whatever, it should never be able to overwrite metadata out on
> the physical disk.

HPT desided to make the metadata accessable from the virtual disk. Just 
look at sector 9 of a working array and enjoy the show...
Grub is fine. At least the version that I am using. You have to do some 
tricks to get grub running on the old hpt controllers (I think the new 
ones are better, but I cannot tell for sure).

Just copy the stage 1.5 reiserfs module to sector 16 instead of sector 8 
which is the grub default that destroys the metadata in case of a hpt.

# dd if=/boot/grub/reiserfs_stage1_5 of=/dev/mapper/hpt37x_cjafdedchg 
seek=16 conv=sync

Then do the following in grub:

grub> root (hd0,1)
grub> geometry (hd0) 9964 255 63
grub> install --stage2=/boot/grub/stage2 /boot/grub/stage1 (hd0) 
(hd0)16+18 (hd0,1)/boot/grub/stage2 (hd0,1)/boot/grub/menu.lst

You have to use the correct geometry for your harddisk and your (hd...) 
values. The "(hd0)16+18" parameter of the "install" command tells grub 
where to find the stage 1.5 loader. "(hd0)" is the disk. "16" is the 
offset of the sector where the module is located and "+18" is the size 
in sectors of the module. Your size may vary. And do not forget that you 
have to round up to the next full sector.

> 
> Anyway, just wanted to tell you to beware.
> Gerte has a GRUB example on his page where he does setup (hd0,5)
> instead of setup (hd0), perhaps that's how it's meant to work.
> 

But you still need a MBR that invokes the loader in (hd0,5) then!

Greetings,
Wilfried

Re: [dm-devel] Re: grub 0.96 bug

[Molle Bestefich]

Wilfried Weissmann wrote:
>> It must be severely broken; since if it just sticked to writing to the
>> device I pointed it at (hpt37x_ehgjaggaf) which is a RAID0 "virtual
>> disk" / whatever, it should never be able to overwrite metadata out on
>> the physical disk.
> 
> HPT desided to make the metadata accessable from the virtual disk. Just
> look at sector 9 of a working array and enjoy the show...

That's an awful decision.

> Grub is fine. At least the version that I am using. You have to do some
> tricks to get grub running on the old hpt controllers (I think the new
> ones are better, but I cannot tell for sure).
> 
> Just copy the stage 1.5 reiserfs module to sector 16 instead of sector 8
> which is the grub default that destroys the metadata in case of a hpt.
> [explanation snipped]

Thanks for the explanation!
There should be an option in the GRUB configuration to do this, so
that the RAID won't get overwritten the next time I do a grub-install,
I think.

Or even better, dmraid could protect the metadata blocks by some magic
flag to dm-mod.
This isn't possible as is, is it?


>> Gerte has a GRUB example on his page where he does setup (hd0,5)
>> instead of setup (hd0), perhaps that's how it's meant to work.
> 
> But you still need a MBR that invokes the loader in (hd0,5) then!

Hrm, I think LILO has an option to slam out a generic MBR that does
something like that?
But perhaps it requires an active flag on (hd0,5), which violates the specs..

Re: Re: grub 0.96 bug

[Wilfried Weissmann]

Molle Bestefich wrote:
> Wilfried Weissmann wrote:
> 
>>>It must be severely broken; since if it just sticked to writing to the
>>>device I pointed it at (hpt37x_ehgjaggaf) which is a RAID0 "virtual
>>>disk" / whatever, it should never be able to overwrite metadata out on
>>>the physical disk.
>>
>>HPT desided to make the metadata accessable from the virtual disk. Just
>>look at sector 9 of a working array and enjoy the show...
> 
> 
> That's an awful decision.

It is, isn't it?

>>Grub is fine. At least the version that I am using. You have to do some
>>tricks to get grub running on the old hpt controllers (I think the new
>>ones are better, but I cannot tell for sure).
>>
>>Just copy the stage 1.5 reiserfs module to sector 16 instead of sector 8
>>which is the grub default that destroys the metadata in case of a hpt.
>>[explanation snipped]
> 
> 
> Thanks for the explanation!
> There should be an option in the GRUB configuration to do this, so
> that the RAID won't get overwritten the next time I do a grub-install,
> I think.

That would make my life a lot easier! On the other hand, the most 
straight forward thing to do would be to move the stage 1.5 to another 
default sector. So you can not forget to set any option that ruins your 
array if missing. Also there is no need to add some autodetection code 
for hpt controllers then.

> 
> Or even better, dmraid could protect the metadata blocks by some magic
> flag to dm-mod.
> This isn't possible as is, is it?

One can make any I/O to this block fail. But I would like something like 
discarding any writes and only perform reads. "dd" backups would still 
work then.

Greetings,
Wilfried

Re: Re: grub 0.96 bug

[Molle Bestefich]

>> Thanks for the explanation!
>> There should be an option in the GRUB configuration to do this, so
>> that the RAID won't get overwritten the next time I do a grub-install,
>> I think.
> 
> That would make my life a lot easier! On the other hand, the most
> straight forward thing to do would be to move the stage 1.5 to another
> default sector. So you can not forget to set any option that ruins your
> array if missing. Also there is no need to add some autodetection code
> for hpt controllers then.

One step ahead of me, you are.

I was just thinking of doing autodetection of HPT magic in GRUB.

The autodetection has a plus side too, though.
It can detect other stuff that should not be touched (Windows/Veritas
dynamic partition information comes to mind?) and not touch sectors
based on what is actually there on a particular system.  Would perhaps
save the implementor the pain of choosing once there's just too many
reserved sectors and the stage1.5 loader wont fit.

I think I've read that the stage1.5 loader is not really necessary in
some cases, think I'll go doc-hunting.

>> Or even better, dmraid could protect the metadata blocks by some magic
>> flag to dm-mod.
>> This isn't possible as is, is it?
> 
> One can make any I/O to this block fail. But I would like something like
> discarding any writes and only perform reads. "dd" backups would still
> work then.

Right.  And perhaps a user-definable flag to turn the protection on or off :-).
Is it possible without adding new code to dm-mod?

Re: Re: grub 0.96 bug

[Wilfried Weissmann]

Molle Bestefich wrote:
>>That would make my life a lot easier! On the other hand, the most
>>straight forward thing to do would be to move the stage 1.5 to another
>>default sector. So you can not forget to set any option that ruins your
>>array if missing. Also there is no need to add some autodetection code
>>for hpt controllers then.
> 
> 
> One step ahead of me, you are.
> 
> I was just thinking of doing autodetection of HPT magic in GRUB.
> 
> The autodetection has a plus side too, though.
> It can detect other stuff that should not be touched (Windows/Veritas
> dynamic partition information comes to mind?) and not touch sectors
> based on what is actually there on a particular system.  Would perhaps
> save the implementor the pain of choosing once there's just too many
> reserved sectors and the stage1.5 loader wont fit.

The problem with the autodetection code is that if you want to play safe 
then you have to check the pci-id of the controller. Even if the BIOS of 
the controller supports RAID if you are picky enough to do this. You 
cannot rely on the contents of sector 9 alone since the disk may have 
been installed on a hpt controller and then moved to another system. In 
this case the signature is still intact and will get picked up by grub.
There are also other things you might want to consider, like MBR + 
loaders for disk encryption software that is also located somewhere in 
this area.
My opinion is that a higher default offset gets the job done. A 
configuration option helps the people who know what they are doing in 
weird corner cases. I would not like to add some autodetection that 
breaks things for some people and make things compilcated when something 
goes wrong.

But I have to admit that I am getting tired of this controller.

I wrote my HPT370 RAID support for kernel 2.2 based on MD.
I debugged and extended the HPT370 ataraid module.
I wrote a EVMS plugin for HPT370 RAID support.
I write bug-reports for dmraid HPT370 support.
I write bug-reports for HALd which has autodetection problems.

> 
> I think I've read that the stage1.5 loader is not really necessary in
> some cases, think I'll go doc-hunting.

I think you can also use the blocks of the stage 1.5 loader _in_ the 
filesystem if you specify the file instead of the (hd0)16+18 blocklist. 
But for doing things like that I hate lilo. ;)

> 
> 
>>>Or even better, dmraid could protect the metadata blocks by some magic
>>>flag to dm-mod.
>>>This isn't possible as is, is it?
>>
>>One can make any I/O to this block fail. But I would like something like
>>discarding any writes and only perform reads. "dd" backups would still
>>work then.
> 
> 
> Right.  And perhaps a user-definable flag to turn the protection on or off :-).
> Is it possible without adding new code to dm-mod?

No, you have to add code that treats write request different than read 
requests. Should not be too complicated though...

Greetings,
Wilfried