[PATCH] Fix prefix problem with LVM on RAID

[PATCH] Fix prefix problem with LVM on RAID

[Michael Guntsche]

[-- Attachment #1.1: Type: text/plain, Size: 604 bytes --]

Hello,

While testing a soon to be deployed setup here I noticed that grub- 
setup is setting the prefix wrong if you run
LVM on Software RAID

grub-setup -v /dev/md0
...
grub-setup: info: dos partition is -2, bsd partition is -2, prefix is  
(md0)(fatoftheland-main)/boot/grub
...

As you can see the there is also (md0) in the prefix which does not  
work.
Attached a small patch that fixes this problem for me. I am not sure  
if it is the best way to fix it so please take a look.
If the root device is on LVM only add the LVM part if it is on RAID  
only add the RAID part.

Kind regards,
Michael



[-- Attachment #1.2: lvm-on-raid.patch --]
[-- Type: application/octet-stream, Size: 928 bytes --]

Index: util/i386/pc/grub-setup.c
===================================================================
RCS file: /sources/grub/grub2/util/i386/pc/grub-setup.c,v
retrieving revision 1.34
diff -u -r1.34 grub-setup.c
--- util/i386/pc/grub-setup.c	19 Feb 2008 14:00:11 -0000	1.34
+++ util/i386/pc/grub-setup.c	23 Feb 2008 19:10:34 -0000
@@ -610,6 +610,7 @@
   char *prefix;
   char *dest_dev;
   int must_embed = 0;
+  int is_lvm = 0;
   
   progname = "grub-setup";
 
@@ -742,6 +743,7 @@
     {
       char *newprefix;
       must_embed = 1;
+      is_lvm = 1;
 
       newprefix = xmalloc (1 + strlen (root_dev) + 1 + strlen (prefix) + 1);
       sprintf (newprefix, "(%s)%s", root_dev, prefix);
@@ -764,7 +766,7 @@
 
       for (i = 0; devicelist[i]; i++)
 	{
-	  setup (raid_prefix,
+	  setup (is_lvm ? prefix : raid_prefix,
 		 dir ? : DEFAULT_DIRECTORY,
 		 boot_file ? : DEFAULT_BOOT_FILE,
 		 core_file ? : DEFAULT_CORE_FILE,

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2417 bytes --]

Re: [PATCH] Fix prefix problem with LVM on RAID

[Michael Guntsche]

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


On Feb 23, 2008, at 21:54, Michael Guntsche wrote:

> Hello,
>
> While testing a soon to be deployed setup here I noticed that grub- 
> setup is setting the prefix wrong if you run
> LVM on Software RAID
>
> grub-setup -v /dev/md0
> ...
> grub-setup: info: dos partition is -2, bsd partition is -2, prefix  
> is (md0)(fatoftheland-main)/boot/grub
> ...
>

Some more information regarding the patch I submitted.

What is happening exactly.

grub-setup first checks if we are on a LVM, we are so it is ok to  
prepend the (vg-lv). The next step is to check if we are on RAID.  
Unfortunately we are on RAID too and (md0) gets prepended as well. In  
the end we end up with a prefix that looks like

(md0)(vg-lv)/boot/grub

which obviously won't work.
grub-setup calls "setup" on every disk that is part of the Raid. All  
the patch does is to pass "prefix" instead of "raid_prefix" if we are  
on LVM. Raid_prefix is only used for this setup call and freed later  
on, so I did not bother to if that out as well.

I can really test this setup next week, when the final hardware  
arrives (hopefully) but grub-emu is already doing the right thing and  
for grub it will be still a boot off a LVM device which already works.

Kind regards,
Michael



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2417 bytes --]

Re: [PATCH] Fix prefix problem with LVM on RAID

[Robert Millan]

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

On Sat, Feb 23, 2008 at 09:54:35PM +0100, Michael Guntsche wrote:
> Hello,
> 
> While testing a soon to be deployed setup here I noticed that grub- 
> setup is setting the prefix wrong if you run
> LVM on Software RAID
> 
> grub-setup -v /dev/md0
> ...
> grub-setup: info: dos partition is -2, bsd partition is -2, prefix is  
> (md0)(fatoftheland-main)/boot/grub
> ...
> 
> As you can see the there is also (md0) in the prefix which does not  
> work.
> Attached a small patch that fixes this problem for me. I am not sure  
> if it is the best way to fix it so please take a look.
> If the root device is on LVM only add the LVM part if it is on RAID  
> only add the RAID part.

Sorry for the delay in repliing.  I think it's more correct this way.  Does
it work for you?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[-- Attachment #2: lvm_on_raid.diff --]
[-- Type: text/x-diff, Size: 474 bytes --]

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/util/i386/pc/grub-setup.c ./util/i386/pc/grub-setup.c
--- ../grub2/util/i386/pc/grub-setup.c	2008-02-19 15:00:11.000000000 +0100
+++ ./util/i386/pc/grub-setup.c	2008-04-13 17:03:21.000000000 +0200
@@ -749,6 +749,7 @@ main (int argc, char *argv[])
       prefix = newprefix;
     }
     
+  else
   if (dest_dev[0] == 'm' && dest_dev[1] == 'd'
       && dest_dev[2] >= '0' && dest_dev[2] <= '9')
     {

Re: [PATCH] Fix prefix problem with LVM on RAID

[Guntsche Michael]

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


On Apr 13, 2008, at 17:04, Robert Millan wrote:
> On Sat, Feb 23, 2008 at 09:54:35PM +0100, Michael Guntsche wrote:
>> Hello,
>>
>> While testing a soon to be deployed setup here I noticed that grub-
>> setup is setting the prefix wrong if you run
>> LVM on Software RAID
>>
>> grub-setup -v /dev/md0
>> ...
>> grub-setup: info: dos partition is -2, bsd partition is -2, prefix is
>> (md0)(fatoftheland-main)/boot/grub
>> ...
>>
>> As you can see the there is also (md0) in the prefix which does not
>> work.
>> Attached a small patch that fixes this problem for me. I am not sure
>> if it is the best way to fix it so please take a look.
>> If the root device is on LVM only add the LVM part if it is on RAID
>> only add the RAID part.
>
> Sorry for the delay in repliing.  I think it's more correct this  
> way.  Does
> it work for you?

Hello Robert,

Take a look at the for loop later on in the MD case, that is not being  
called with your patch.
The code gets all RAID disks and calls setup on each of them. I do not  
think the setup call further down is doing the same thing, haven't  
checked it though. With my patch "setup" is still called for each RAID  
member with the correct prefix. In the case of a faulty disk you can  
still boot from the degraded array, by choosing a different disk.

Kind regards,
Michael

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2417 bytes --]

Re: [PATCH] Fix prefix problem with LVM on RAID

[Robert Millan]

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

On Sun, Apr 13, 2008 at 06:07:22PM +0200, Guntsche Michael wrote:
> 
> On Apr 13, 2008, at 17:04, Robert Millan wrote:
> >On Sat, Feb 23, 2008 at 09:54:35PM +0100, Michael Guntsche wrote:
> >>Hello,
> >>
> >>While testing a soon to be deployed setup here I noticed that grub-
> >>setup is setting the prefix wrong if you run
> >>LVM on Software RAID
> >>
> >>grub-setup -v /dev/md0
> >>...
> >>grub-setup: info: dos partition is -2, bsd partition is -2, prefix is
> >>(md0)(fatoftheland-main)/boot/grub
> >>...
> >>
> >>As you can see the there is also (md0) in the prefix which does not
> >>work.
> >>Attached a small patch that fixes this problem for me. I am not sure
> >>if it is the best way to fix it so please take a look.
> >>If the root device is on LVM only add the LVM part if it is on RAID
> >>only add the RAID part.
> >
> >Sorry for the delay in repliing.  I think it's more correct this  
> >way.  Does
> >it work for you?
> 
> Hello Robert,
> 
> Take a look at the for loop later on in the MD case, that is not being  
> called with your patch.
> The code gets all RAID disks and calls setup on each of them. I do not  
> think the setup call further down is doing the same thing, haven't  
> checked it though. With my patch "setup" is still called for each RAID  
> member with the correct prefix. In the case of a faulty disk you can  
> still boot from the degraded array, by choosing a different disk.

Ok, second try.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[-- Attachment #2: raid.diff --]
[-- Type: text/x-diff, Size: 2078 bytes --]


	* util/i386/pc/grub-setup.c (main): When LVM is used on top of RAID,
	don't prepend the RAID device in setup()'s `prefix'.

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/util/i386/pc/grub-setup.c ./util/i386/pc/grub-setup.c
--- ../grub2/util/i386/pc/grub-setup.c	2008-02-19 15:00:11.000000000 +0100
+++ ./util/i386/pc/grub-setup.c	2008-04-13 18:36:42.000000000 +0200
@@ -609,7 +609,7 @@ main (int argc, char *argv[])
   char *root_dev = 0;
   char *prefix;
   char *dest_dev;
-  int must_embed = 0;
+  int root_in_lvm = 0;
   
   progname = "grub-setup";
 
@@ -741,7 +741,7 @@ main (int argc, char *argv[])
   if (grub_util_lvm_isvolume (root_dev))
     {
       char *newprefix;
-      must_embed = 1;
+      root_in_lvm = 1;
 
       newprefix = xmalloc (1 + strlen (root_dev) + 1 + strlen (prefix) + 1);
       sprintf (newprefix, "(%s)%s", root_dev, prefix);
@@ -753,25 +753,26 @@ main (int argc, char *argv[])
       && dest_dev[2] >= '0' && dest_dev[2] <= '9')
     {
       char **devicelist;
-      char *raid_prefix;
       int i;
 
-      raid_prefix = xmalloc (1 + strlen (dest_dev) + 1 + strlen (prefix) + 1);
+      if (! root_in_lvm)
+	{
+	  char *newprefix = xmalloc (1 + strlen (dest_dev) + 1 + strlen (prefix) + 1);
+	  sprintf (newprefix, "(%s)%s", dest_dev, prefix);
+	  free (prefix);
+	  prefix = newprefix;
+	}
 
-      sprintf (raid_prefix, "(%s)%s", dest_dev, prefix);
-      
       devicelist = grub_util_raid_getmembers (dest_dev);
 
       for (i = 0; devicelist[i]; i++)
 	{
-	  setup (raid_prefix,
+	  setup (prefix,
 		 dir ? : DEFAULT_DIRECTORY,
 		 boot_file ? : DEFAULT_BOOT_FILE,
 		 core_file ? : DEFAULT_CORE_FILE,
 		 root_dev, grub_util_get_grub_dev (devicelist[i]), 1);
 	}
-
-      free (raid_prefix);
     }
   else
 #endif
@@ -780,7 +781,7 @@ main (int argc, char *argv[])
 	   dir ? : DEFAULT_DIRECTORY,
 	   boot_file ? : DEFAULT_BOOT_FILE,
 	   core_file ? : DEFAULT_CORE_FILE,
-	   root_dev, dest_dev, must_embed);
+	   root_dev, dest_dev, root_in_lvm);
 
   /* Free resources.  */
   grub_fini_all ();

Re: [PATCH] Fix prefix problem with LVM on RAID

[Guntsche Michael]

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


On Apr 13, 2008, at 18:40, Robert Millan wrote:
>
> Ok, second try.

This one looks ok for me did not try it out yet though.

/Michael


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2417 bytes --]