All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bug fix for LVM
@ 2009-07-18 14:11 Bean
  2009-07-18 19:11 ` Robert Millan
  2009-07-19 19:11 ` Patrik Horník
  0 siblings, 2 replies; 28+ messages in thread
From: Bean @ 2009-07-18 14:11 UTC (permalink / raw)
  To: The development of GRUB 2

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

Hi,

This patch fixes a few bug in lvm.

There can be multiple copies of meta data, it's not an error, ignore
the extra copy instead of quit.

In case of LVM on RAID, the raid device and first disk would have the
same lvm header ! The raid device would be found first, so we don't
replace pv->disk if it has been set.

grub-probe doesn't work in LVM on RAID, as it only assume one layer of
abstraction, the fix is not to rely on os, but uses grub to get the
correct abstraction information.

Fix grub-fstest to support lvm.

-- 
Bean

[-- Attachment #2: lvm.diff --]
[-- Type: text/x-patch, Size: 4117 bytes --]

diff --git a/disk/lvm.c b/disk/lvm.c
index 6707a40..126b494 100644
--- a/disk/lvm.c
+++ b/disk/lvm.c
@@ -271,15 +271,9 @@ grub_lvm_scan_device (const char *name)
   dlocn++;
   mda_offset = grub_le_to_cpu64 (dlocn->offset);
   mda_size = grub_le_to_cpu64 (dlocn->size);
-  dlocn++;
 
-  if (dlocn->offset)
-    {
-      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "We don't support multiple LVM metadata areas");
-
-      goto fail;
-    }
+  /* It's possible to have multiple copies of metadata areas, we just use the
+     first one.  */
 
   /* Allocate buffer space for the circular worst-case scenario. */
   metadatabuf = grub_malloc (2 * mda_size);
@@ -564,7 +558,10 @@ grub_lvm_scan_device (const char *name)
       {
 	if (! grub_memcmp (pv->id, pv_id, GRUB_LVM_ID_STRLEN))
 	  {
-	    pv->disk = grub_disk_open (name);
+	    /* This could happen to LVM on RAID, pv->disk points to the
+	       raid device, we shouldn't change it.  */
+	    if (! pv->disk)
+	      pv->disk = grub_disk_open (name);
 	    break;
 	  }
       }
diff --git a/util/grub-fstest.c b/util/grub-fstest.c
index 4722269..c39529a 100644
--- a/util/grub-fstest.c
+++ b/util/grub-fstest.c
@@ -294,6 +294,8 @@ fstest (char **images, int num_disks, int cmd, int n, char **args)
     }
 
   grub_raid_rescan ();
+  grub_lvm_fini ();
+  grub_lvm_init ();
   switch (cmd)
     {
     case CMD_LS:
diff --git a/util/grub-probe.c b/util/grub-probe.c
index 97d3860..a187859 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -106,7 +106,6 @@ probe (const char *path, char *device_name)
   char *drive_name = NULL;
   char *grub_path = NULL;
   char *filebuf_via_grub = NULL, *filebuf_via_sys = NULL;
-  int abstraction_type;
   grub_device_t dev = NULL;
   grub_fs_t fs;
 
@@ -114,10 +113,10 @@ probe (const char *path, char *device_name)
     {
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
       if (! grub_util_check_char_device (device_name))
-        grub_util_error ("%s is not a character device.\n", device_name);
+	grub_util_error ("%s is not a character device.\n", device_name);
 #else
       if (! grub_util_check_block_device (device_name))
-        grub_util_error ("%s is not a block device.\n", device_name);
+	grub_util_error ("%s is not a block device.\n", device_name);
 #endif
     }
   else
@@ -132,28 +131,6 @@ probe (const char *path, char *device_name)
       goto end;
     }
 
-  abstraction_type = grub_util_get_dev_abstraction (device_name);
-  /* No need to check for errors; lack of abstraction is permissible.  */
-
-  if (print == PRINT_ABSTRACTION)
-    {
-      char *abstraction_name;
-      switch (abstraction_type)
-	{
-	case GRUB_DEV_ABSTRACTION_LVM:
-	  abstraction_name = "lvm";
-	  break;
-	case GRUB_DEV_ABSTRACTION_RAID:
-	  abstraction_name = "raid mdraid";
-	  break;
-	default:
-	  grub_util_info ("did not find LVM/RAID in %s, assuming raw device", device_name);
-	  goto end;
-	}
-      printf ("%s\n", abstraction_name);
-      goto end;
-    }
-
   drive_name = grub_util_get_grub_dev (device_name);
   if (! drive_name)
     grub_util_error ("Cannot find a GRUB drive for %s.  Check your device.map.\n", device_name);
@@ -169,6 +146,38 @@ probe (const char *path, char *device_name)
   if (! dev)
     grub_util_error ("%s", grub_errmsg);
 
+  if (print == PRINT_ABSTRACTION)
+    {
+      char buf[30];
+      grub_disk_memberlist_t list = NULL, tmp;
+      int is_lvm = 0;
+      int is_raid = 0;
+
+      is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
+      is_raid |= (dev->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);
+
+      if ((is_lvm) && (dev->disk->dev->memberlist))
+	list = dev->disk->dev->memberlist (dev->disk);
+      while (list)
+	{
+	  is_raid |= (list->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);
+	  tmp = list->next;
+	  free (list);
+	  list = tmp;
+	}
+
+      buf[0] = buf[1] = 0;
+      if (is_raid)
+	strcat (buf, " raid mdraid");
+
+      if (is_lvm)
+	strcat (buf, " lvm");
+
+      printf ("%s\n", &buf[1]);	
+      
+      goto end;
+    }
+
   if (print == PRINT_PARTMAP)
     {
       grub_disk_memberlist_t list = NULL, tmp;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 14:11 [PATCH] Bug fix for LVM Bean
@ 2009-07-18 19:11 ` Robert Millan
  2009-07-18 19:43   ` Bean
  2009-07-19 19:11 ` Patrik Horník
  1 sibling, 1 reply; 28+ messages in thread
From: Robert Millan @ 2009-07-18 19:11 UTC (permalink / raw)
  To: The development of GRUB 2

On Sat, Jul 18, 2009 at 10:11:19PM +0800, Bean wrote:
>      }
>  
>    grub_raid_rescan ();
> +  grub_lvm_fini ();
> +  grub_lvm_init ();

This is aside from this patch, but I don't see the purpose of this
grub_raid_rescan() function.  It's in raid.mod but only used by
grub-fstest, so at least it should be #ifdef'ed out, but looking at
what it does, it seems very ad-hoc.  It basically amounts to the
same you're doing with grub_lvm_fini() and grub_lvm_init().  Could
you fix this while at it?

>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
>        if (! grub_util_check_char_device (device_name))
> -        grub_util_error ("%s is not a character device.\n", device_name);
> +	grub_util_error ("%s is not a character device.\n", device_name);
>  #else
>        if (! grub_util_check_block_device (device_name))
> -        grub_util_error ("%s is not a block device.\n", device_name);
> +	grub_util_error ("%s is not a block device.\n", device_name);
>  #endif

Looks like this slipped in.

> +  if (print == PRINT_ABSTRACTION)
> +    {
> +      char buf[30];

This is a buffer overflow waiting to happen.  Please use asprintf(), this
will help you avoid the "&buf[1]" hack.

> +      grub_disk_memberlist_t list = NULL, tmp;
> +      int is_lvm = 0;
> +      int is_raid = 0;

I think you can add const qualifier in the is_lvm one.

> +      is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
> +      is_raid |= (dev->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);

No need for logic OR here.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 19:11 ` Robert Millan
@ 2009-07-18 19:43   ` Bean
  2009-07-18 19:57     ` Robert Millan
  2009-07-18 19:59     ` Vladimir 'phcoder' Serbinenko
  0 siblings, 2 replies; 28+ messages in thread
From: Bean @ 2009-07-18 19:43 UTC (permalink / raw)
  To: The development of GRUB 2

On Sun, Jul 19, 2009 at 3:11 AM, Robert Millan<rmh@aybabtu.com> wrote:
> On Sat, Jul 18, 2009 at 10:11:19PM +0800, Bean wrote:
>>      }
>>
>>    grub_raid_rescan ();
>> +  grub_lvm_fini ();
>> +  grub_lvm_init ();
>
> This is aside from this patch, but I don't see the purpose of this
> grub_raid_rescan() function.  It's in raid.mod but only used by
> grub-fstest, so at least it should be #ifdef'ed out, but looking at
> what it does, it seems very ad-hoc.  It basically amounts to the
> same you're doing with grub_lvm_fini() and grub_lvm_init().  Could
> you fix this while at it?

This is required. As raid and lvm scan device in init function, but
grub-fstest uses loopback device, which hasn't been setup in
grub_init_all. This code rescan raid and lvm, otherwise there won't be
available.

>> +  if (print == PRINT_ABSTRACTION)
>> +    {
>> +      char buf[30];
>
> This is a buffer overflow waiting to happen.  Please use asprintf(), this
> will help you avoid the "&buf[1]" hack.
>
>> +      grub_disk_memberlist_t list = NULL, tmp;
>> +      int is_lvm = 0;
>> +      int is_raid = 0;
>
> I think you can add const qualifier in the is_lvm one.
>
>> +      is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
>> +      is_raid |= (dev->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);
>
> No need for logic OR here.

Ok.

-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 19:43   ` Bean
@ 2009-07-18 19:57     ` Robert Millan
  2009-07-18 19:59     ` Vladimir 'phcoder' Serbinenko
  1 sibling, 0 replies; 28+ messages in thread
From: Robert Millan @ 2009-07-18 19:57 UTC (permalink / raw)
  To: The development of GRUB 2

On Sun, Jul 19, 2009 at 03:43:10AM +0800, Bean wrote:
> On Sun, Jul 19, 2009 at 3:11 AM, Robert Millan<rmh@aybabtu.com> wrote:
> > On Sat, Jul 18, 2009 at 10:11:19PM +0800, Bean wrote:
> >>      }
> >>
> >>    grub_raid_rescan ();
> >> +  grub_lvm_fini ();
> >> +  grub_lvm_init ();
> >
> > This is aside from this patch, but I don't see the purpose of this
> > grub_raid_rescan() function.  It's in raid.mod but only used by
> > grub-fstest, so at least it should be #ifdef'ed out, but looking at
> > what it does, it seems very ad-hoc.  It basically amounts to the
> > same you're doing with grub_lvm_fini() and grub_lvm_init().  Could
> > you fix this while at it?
> 
> This is required. As raid and lvm scan device in init function, but
> grub-fstest uses loopback device, which hasn't been setup in
> grub_init_all. This code rescan raid and lvm, otherwise there won't be
> available.

Ok, then please could you ifdef it for GRUB_UTIL ?

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 19:43   ` Bean
  2009-07-18 19:57     ` Robert Millan
@ 2009-07-18 19:59     ` Vladimir 'phcoder' Serbinenko
  2009-07-18 20:05       ` Robert Millan
  1 sibling, 1 reply; 28+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2009-07-18 19:59 UTC (permalink / raw)
  To: The development of GRUB 2

On Sat, Jul 18, 2009 at 9:43 PM, Bean<bean123ch@gmail.com> wrote:
> On Sun, Jul 19, 2009 at 3:11 AM, Robert Millan<rmh@aybabtu.com> wrote:
>> On Sat, Jul 18, 2009 at 10:11:19PM +0800, Bean wrote:
>>>      }
>>>
>>>    grub_raid_rescan ();
>>> +  grub_lvm_fini ();
>>> +  grub_lvm_init ();
>>
>> This is aside from this patch, but I don't see the purpose of this
>> grub_raid_rescan() function.  It's in raid.mod but only used by
>> grub-fstest, so at least it should be #ifdef'ed out, but looking at
>> what it does, it seems very ad-hoc.  It basically amounts to the
>> same you're doing with grub_lvm_fini() and grub_lvm_init().  Could
>> you fix this while at it?
>
> This is required. As raid and lvm scan device in init function, but
> grub-fstest uses loopback device, which hasn't been setup in
> grub_init_all. This code rescan raid and lvm, otherwise there won't be
> available.
>
This situation isn't strictly speaking restricted to grub-fstest then.
One would sensibly want to loopmount lvm image in grub shell. Is there
a way to do it cleanly?
>>> +  if (print == PRINT_ABSTRACTION)
>>> +    {
>>> +      char buf[30];
>>
>> This is a buffer overflow waiting to happen.  Please use asprintf(), this
>> will help you avoid the "&buf[1]" hack.
>>
>>> +      grub_disk_memberlist_t list = NULL, tmp;
>>> +      int is_lvm = 0;
>>> +      int is_raid = 0;
>>
>> I think you can add const qualifier in the is_lvm one.
>>
>>> +      is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
>>> +      is_raid |= (dev->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);
>>
>> No need for logic OR here.
>
> Ok.
>
> --
> Bean
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 19:59     ` Vladimir 'phcoder' Serbinenko
@ 2009-07-18 20:05       ` Robert Millan
  2009-07-19  9:41         ` Bean
  0 siblings, 1 reply; 28+ messages in thread
From: Robert Millan @ 2009-07-18 20:05 UTC (permalink / raw)
  To: The development of GRUB 2

On Sat, Jul 18, 2009 at 09:59:55PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> On Sat, Jul 18, 2009 at 9:43 PM, Bean<bean123ch@gmail.com> wrote:
> > On Sun, Jul 19, 2009 at 3:11 AM, Robert Millan<rmh@aybabtu.com> wrote:
> >> On Sat, Jul 18, 2009 at 10:11:19PM +0800, Bean wrote:
> >>>      }
> >>>
> >>>    grub_raid_rescan ();
> >>> +  grub_lvm_fini ();
> >>> +  grub_lvm_init ();
> >>
> >> This is aside from this patch, but I don't see the purpose of this
> >> grub_raid_rescan() function.  It's in raid.mod but only used by
> >> grub-fstest, so at least it should be #ifdef'ed out, but looking at
> >> what it does, it seems very ad-hoc.  It basically amounts to the
> >> same you're doing with grub_lvm_fini() and grub_lvm_init().  Could
> >> you fix this while at it?
> >
> > This is required. As raid and lvm scan device in init function, but
> > grub-fstest uses loopback device, which hasn't been setup in
> > grub_init_all. This code rescan raid and lvm, otherwise there won't be
> > available.
> >
> This situation isn't strictly speaking restricted to grub-fstest then.
> One would sensibly want to loopmount lvm image in grub shell. Is there
> a way to do it cleanly?

I think a good long-term solution is to move the scan function from
grub_raid_init() to wheereever appropiate to make the scan happen
dynamically everytime e.g. the iterate() function is called.  However,
doing this efficiently needs some work.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 20:05       ` Robert Millan
@ 2009-07-19  9:41         ` Bean
  2009-07-22 17:29           ` Robert Millan
  2009-07-22 17:33           ` Robert Millan
  0 siblings, 2 replies; 28+ messages in thread
From: Bean @ 2009-07-19  9:41 UTC (permalink / raw)
  To: The development of GRUB 2

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

On Sun, Jul 19, 2009 at 4:05 AM, Robert Millan<rmh@aybabtu.com> wrote:
> On Sat, Jul 18, 2009 at 09:59:55PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> On Sat, Jul 18, 2009 at 9:43 PM, Bean<bean123ch@gmail.com> wrote:
>> > On Sun, Jul 19, 2009 at 3:11 AM, Robert Millan<rmh@aybabtu.com> wrote:
>> >> On Sat, Jul 18, 2009 at 10:11:19PM +0800, Bean wrote:
>> >>>      }
>> >>>
>> >>>    grub_raid_rescan ();
>> >>> +  grub_lvm_fini ();
>> >>> +  grub_lvm_init ();
>> >>
>> >> This is aside from this patch, but I don't see the purpose of this
>> >> grub_raid_rescan() function.  It's in raid.mod but only used by
>> >> grub-fstest, so at least it should be #ifdef'ed out, but looking at
>> >> what it does, it seems very ad-hoc.  It basically amounts to the
>> >> same you're doing with grub_lvm_fini() and grub_lvm_init().  Could
>> >> you fix this while at it?
>> >
>> > This is required. As raid and lvm scan device in init function, but
>> > grub-fstest uses loopback device, which hasn't been setup in
>> > grub_init_all. This code rescan raid and lvm, otherwise there won't be
>> > available.
>> >
>> This situation isn't strictly speaking restricted to grub-fstest then.
>> One would sensibly want to loopmount lvm image in grub shell. Is there
>> a way to do it cleanly?
>
> I think a good long-term solution is to move the scan function from
> grub_raid_init() to wheereever appropiate to make the scan happen
> dynamically everytime e.g. the iterate() function is called.  However,
> doing this efficiently needs some work.

Hi,

I've come up an alternative solution to grub_raid_rescan, we can use
the same trick as lvm, call fini and then init function, this would
have the same effect of rescanning. Now that grub_raid_rescan is gone,
I can simply raid.c further by moving grub_raid_scan_device into
grub_raid_register.

As for dynamically scanning, I think it's not common to mount lvm/raid
in loopback files, we could just ignore it for now.

-- 
Bean

[-- Attachment #2: lvm_2.diff --]
[-- Type: text/x-patch, Size: 6896 bytes --]

diff --git a/disk/lvm.c b/disk/lvm.c
index 6707a40..126b494 100644
--- a/disk/lvm.c
+++ b/disk/lvm.c
@@ -271,15 +271,9 @@ grub_lvm_scan_device (const char *name)
   dlocn++;
   mda_offset = grub_le_to_cpu64 (dlocn->offset);
   mda_size = grub_le_to_cpu64 (dlocn->size);
-  dlocn++;
 
-  if (dlocn->offset)
-    {
-      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "We don't support multiple LVM metadata areas");
-
-      goto fail;
-    }
+  /* It's possible to have multiple copies of metadata areas, we just use the
+     first one.  */
 
   /* Allocate buffer space for the circular worst-case scenario. */
   metadatabuf = grub_malloc (2 * mda_size);
@@ -564,7 +558,10 @@ grub_lvm_scan_device (const char *name)
       {
 	if (! grub_memcmp (pv->id, pv_id, GRUB_LVM_ID_STRLEN))
 	  {
-	    pv->disk = grub_disk_open (name);
+	    /* This could happen to LVM on RAID, pv->disk points to the
+	       raid device, we shouldn't change it.  */
+	    if (! pv->disk)
+	      pv->disk = grub_disk_open (name);
 	    break;
 	  }
       }
diff --git a/disk/raid.c b/disk/raid.c
index c4d0857..23b69ad 100644
--- a/disk/raid.c
+++ b/disk/raid.c
@@ -590,56 +590,6 @@ insert_array (grub_disk_t disk, struct grub_raid_array *new_array,
 static grub_raid_t grub_raid_list;
 
 static void
-grub_raid_scan_device (int head_only)
-{
-  auto int hook (const char *name);
-  int hook (const char *name)
-    {
-      grub_disk_t disk;
-      struct grub_raid_array array;
-      struct grub_raid *p;
-
-      grub_dprintf ("raid", "Scanning for RAID devices on disk %s\n", name);
-
-      disk = grub_disk_open (name);
-      if (!disk)
-        return 0;
-
-      if (disk->total_sectors == GRUB_ULONG_MAX)
-        {
-          grub_disk_close (disk);
-          return 0;
-        }
-
-      for (p = grub_raid_list; p; p = p->next)
-        {
-          if (! p->detect (disk, &array))
-            {
-              if (! insert_array (disk, &array, p->name))
-                return 0;
-
-              break;
-            }
-
-          /* This error usually means it's not raid, no need to display
-             it.  */
-          if (grub_errno != GRUB_ERR_OUT_OF_RANGE)
-            grub_print_error ();
-
-          grub_errno = GRUB_ERR_NONE;
-          if (head_only)
-            break;
-        }
-
-      grub_disk_close (disk);
-
-      return 0;
-    }
-
-  grub_device_iterate (&hook);
-}
-
-static void
 free_array (void)
 {
   struct grub_raid_array *array;
@@ -668,9 +618,38 @@ free_array (void)
 void
 grub_raid_register (grub_raid_t raid)
 {
+  auto int hook (const char *name);
+  int hook (const char *name)
+    {
+      grub_disk_t disk;
+      struct grub_raid_array array;
+
+      grub_dprintf ("raid", "Scanning for RAID devices on disk %s\n", name);
+
+      disk = grub_disk_open (name);
+      if (!disk)
+        return 0;
+
+      if ((disk->total_sectors != GRUB_ULONG_MAX) &&
+	  (! grub_raid_list->detect (disk, &array)) &&
+	  (! insert_array (disk, &array, grub_raid_list->name)))
+	return 0;
+
+      /* This error usually means it's not raid, no need to display
+	 it.  */
+      if (grub_errno != GRUB_ERR_OUT_OF_RANGE)
+	grub_print_error ();
+
+      grub_errno = GRUB_ERR_NONE;
+      
+      grub_disk_close (disk);
+
+      return 0;
+    }
+
   raid->next = grub_raid_list;
   grub_raid_list = raid;
-  grub_raid_scan_device (1);
+  grub_device_iterate (&hook);
 }
 
 void
@@ -686,13 +665,6 @@ grub_raid_unregister (grub_raid_t raid)
       }
 }
 
-void
-grub_raid_rescan (void)
-{
-  free_array ();
-  grub_raid_scan_device (0);
-}
-
 static struct grub_disk_dev grub_raid_dev =
   {
     .name = "raid",
diff --git a/include/grub/raid.h b/include/grub/raid.h
index 595ced1..8fa4c38 100644
--- a/include/grub/raid.h
+++ b/include/grub/raid.h
@@ -67,7 +67,6 @@ typedef struct grub_raid *grub_raid_t;
 void grub_raid_register (grub_raid_t raid);
 void grub_raid_unregister (grub_raid_t raid);
 
-void grub_raid_rescan (void);
 void grub_raid_block_xor (char *buf1, const char *buf2, int size);
 
 typedef grub_err_t (*grub_raid5_recover_func_t) (struct grub_raid_array *array,
diff --git a/util/grub-fstest.c b/util/grub-fstest.c
index 4722269..a4de2ae 100644
--- a/util/grub-fstest.c
+++ b/util/grub-fstest.c
@@ -28,7 +28,6 @@
 #include <grub/env.h>
 #include <grub/term.h>
 #include <grub/mm.h>
-#include <grub/raid.h>
 #include <grub/lib/hexdump.h>
 #include <grub/lib/crc.h>
 #include <grub/command.h>
@@ -293,7 +292,13 @@ fstest (char **images, int num_disks, int cmd, int n, char **args)
         grub_util_error ("loopback command fails.");
     }
 
-  grub_raid_rescan ();
+  grub_lvm_fini ();
+  grub_mdraid_fini ();
+  grub_raid_fini ();  
+  grub_raid_init ();
+  grub_mdraid_init ();
+  grub_lvm_init ();
+
   switch (cmd)
     {
     case CMD_LS:
diff --git a/util/grub-probe.c b/util/grub-probe.c
index 97d3860..df920af 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -106,7 +106,6 @@ probe (const char *path, char *device_name)
   char *drive_name = NULL;
   char *grub_path = NULL;
   char *filebuf_via_grub = NULL, *filebuf_via_sys = NULL;
-  int abstraction_type;
   grub_device_t dev = NULL;
   grub_fs_t fs;
 
@@ -132,28 +131,6 @@ probe (const char *path, char *device_name)
       goto end;
     }
 
-  abstraction_type = grub_util_get_dev_abstraction (device_name);
-  /* No need to check for errors; lack of abstraction is permissible.  */
-
-  if (print == PRINT_ABSTRACTION)
-    {
-      char *abstraction_name;
-      switch (abstraction_type)
-	{
-	case GRUB_DEV_ABSTRACTION_LVM:
-	  abstraction_name = "lvm";
-	  break;
-	case GRUB_DEV_ABSTRACTION_RAID:
-	  abstraction_name = "raid mdraid";
-	  break;
-	default:
-	  grub_util_info ("did not find LVM/RAID in %s, assuming raw device", device_name);
-	  goto end;
-	}
-      printf ("%s\n", abstraction_name);
-      goto end;
-    }
-
   drive_name = grub_util_get_grub_dev (device_name);
   if (! drive_name)
     grub_util_error ("Cannot find a GRUB drive for %s.  Check your device.map.\n", device_name);
@@ -169,6 +146,33 @@ probe (const char *path, char *device_name)
   if (! dev)
     grub_util_error ("%s", grub_errmsg);
 
+  if (print == PRINT_ABSTRACTION)
+    {
+      grub_disk_memberlist_t list = NULL, tmp;
+      const int is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
+      int is_raid = (dev->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);
+
+      if ((is_lvm) && (dev->disk->dev->memberlist))
+	list = dev->disk->dev->memberlist (dev->disk);
+      while (list)
+	{
+	  is_raid |= (list->disk->dev->id == GRUB_DISK_DEVICE_RAID_ID);
+	  tmp = list->next;
+	  free (list);
+	  list = tmp;
+	}
+
+      if (is_raid)
+	printf ("raid mdraid");
+
+      if (is_lvm)
+	printf ((is_raid) ? " lvm" : "lvm");
+	
+      printf ("\n");
+
+      goto end;
+    }
+
   if (print == PRINT_PARTMAP)
     {
       grub_disk_memberlist_t list = NULL, tmp;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-18 14:11 [PATCH] Bug fix for LVM Bean
  2009-07-18 19:11 ` Robert Millan
@ 2009-07-19 19:11 ` Patrik Horník
  2009-07-19 19:34   ` Bean
  1 sibling, 1 reply; 28+ messages in thread
From: Patrik Horník @ 2009-07-19 19:11 UTC (permalink / raw)
  To: The development of GRUB 2

Bean, should Grub2 with this patch work if the boot partition is LVM
volume and LVM is placed on top of RAID 5 arrays?

Thanks,

Patrik


On Sat, Jul 18, 2009 at 16:11, Bean<bean123ch@gmail.com> wrote:
> Hi,
>
> This patch fixes a few bug in lvm.
>
> There can be multiple copies of meta data, it's not an error, ignore
> the extra copy instead of quit.
>
> In case of LVM on RAID, the raid device and first disk would have the
> same lvm header ! The raid device would be found first, so we don't
> replace pv->disk if it has been set.
>
> grub-probe doesn't work in LVM on RAID, as it only assume one layer of
> abstraction, the fix is not to rely on os, but uses grub to get the
> correct abstraction information.
>
> Fix grub-fstest to support lvm.
>
> --
> Bean
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-19 19:11 ` Patrik Horník
@ 2009-07-19 19:34   ` Bean
  2009-07-25 20:37     ` Felix Zielcke
  2009-07-27 20:58     ` Patrik Horník
  0 siblings, 2 replies; 28+ messages in thread
From: Bean @ 2009-07-19 19:34 UTC (permalink / raw)
  To: The development of GRUB 2

On Mon, Jul 20, 2009 at 3:11 AM, Patrik Horník<patrik@dsl.sk> wrote:
> Bean, should Grub2 with this patch work if the boot partition is LVM
> volume and LVM is placed on top of RAID 5 arrays?

Hi,

Yeah, it should work, please try it out.

-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-19  9:41         ` Bean
@ 2009-07-22 17:29           ` Robert Millan
  2009-07-22 17:33           ` Robert Millan
  1 sibling, 0 replies; 28+ messages in thread
From: Robert Millan @ 2009-07-22 17:29 UTC (permalink / raw)
  To: The development of GRUB 2

On Sun, Jul 19, 2009 at 05:41:04PM +0800, Bean wrote:
> 
> Hi,
> 
> I've come up an alternative solution to grub_raid_rescan, we can use
> the same trick as lvm, call fini and then init function, this would
> have the same effect of rescanning. Now that grub_raid_rescan is gone,
> I can simply raid.c further by moving grub_raid_scan_device into
> grub_raid_register.

Good idea.  I was certain this could be simplified :-)

> As for dynamically scanning, I think it's not common to mount lvm/raid
> in loopback files, we could just ignore it for now.

Sure.  Just an idea we can discuss sometime.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-19  9:41         ` Bean
  2009-07-22 17:29           ` Robert Millan
@ 2009-07-22 17:33           ` Robert Millan
  2009-07-22 18:58             ` Bean
  1 sibling, 1 reply; 28+ messages in thread
From: Robert Millan @ 2009-07-22 17:33 UTC (permalink / raw)
  To: The development of GRUB 2

On Sun, Jul 19, 2009 at 05:41:04PM +0800, Bean wrote:
> +      if (is_raid)
> +	printf ("raid mdraid");
> +
> +      if (is_lvm)
> +	printf ((is_raid) ? " lvm" : "lvm");

Is there a better way to handle this?  Perhaps we could make the list
newline separated instead of space separated and avoid the problem
altogether.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-22 17:33           ` Robert Millan
@ 2009-07-22 18:58             ` Bean
  2009-07-25 16:05               ` Robert Millan
  0 siblings, 1 reply; 28+ messages in thread
From: Bean @ 2009-07-22 18:58 UTC (permalink / raw)
  To: The development of GRUB 2

On Thu, Jul 23, 2009 at 1:33 AM, Robert Millan<rmh@aybabtu.com> wrote:
> On Sun, Jul 19, 2009 at 05:41:04PM +0800, Bean wrote:
>> +      if (is_raid)
>> +     printf ("raid mdraid");
>> +
>> +      if (is_lvm)
>> +     printf ((is_raid) ? " lvm" : "lvm");
>
> Is there a better way to handle this?  Perhaps we could make the list
> newline separated instead of space separated and avoid the problem
> altogether.

Hi,

Actually, if we allows an extra space at the end of line, it can be
written like this:

if (is_raid)
     printf ("raid mdraid ");

if (is_lvm)
     printf ("lvm ");

The space is not visible, and ignored by grub-install anyway.

-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-22 18:58             ` Bean
@ 2009-07-25 16:05               ` Robert Millan
  0 siblings, 0 replies; 28+ messages in thread
From: Robert Millan @ 2009-07-25 16:05 UTC (permalink / raw)
  To: The development of GRUB 2

On Thu, Jul 23, 2009 at 02:58:50AM +0800, Bean wrote:
> On Thu, Jul 23, 2009 at 1:33 AM, Robert Millan<rmh@aybabtu.com> wrote:
> > On Sun, Jul 19, 2009 at 05:41:04PM +0800, Bean wrote:
> >> +      if (is_raid)
> >> +     printf ("raid mdraid");
> >> +
> >> +      if (is_lvm)
> >> +     printf ((is_raid) ? " lvm" : "lvm");
> >
> > Is there a better way to handle this?  Perhaps we could make the list
> > newline separated instead of space separated and avoid the problem
> > altogether.
> 
> Hi,
> 
> Actually, if we allows an extra space at the end of line, it can be
> written like this:
> 
> if (is_raid)
>      printf ("raid mdraid ");
> 
> if (is_lvm)
>      printf ("lvm ");
> 
> The space is not visible, and ignored by grub-install anyway.

Ok.  I'd prefer a newline, but it can always be changed later on.

Your patch seems fine to me.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-19 19:34   ` Bean
@ 2009-07-25 20:37     ` Felix Zielcke
  2009-07-27 20:58     ` Patrik Horník
  1 sibling, 0 replies; 28+ messages in thread
From: Felix Zielcke @ 2009-07-25 20:37 UTC (permalink / raw)
  To: The development of GRUB 2

Am Montag, den 20.07.2009, 03:34 +0800 schrieb Bean:
> On Mon, Jul 20, 2009 at 3:11 AM, Patrik Horník<patrik@dsl.sk> wrote:
> > Bean, should Grub2 with this patch work if the boot partition is LVM
> > volume and LVM is placed on top of RAID 5 arrays?
> 
> Hi,
> 
> Yeah, it should work, please try it out.
> 

I just tried it out with a RAID 5 over 3 partitions on one disk.
grub-probe -t abstraction shows `raid mdraid lvm', but if I boot from
the disk ls doestn't (md0) nor (vg-lvol0)
If I insmod raid; insmod mdraid; insmod lvm from a normal grub both
devices get shown.

-- 
Felix Zielcke
Proud Debian Maintainer




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-19 19:34   ` Bean
  2009-07-25 20:37     ` Felix Zielcke
@ 2009-07-27 20:58     ` Patrik Horník
  2009-07-28  3:27       ` Bean
  1 sibling, 1 reply; 28+ messages in thread
From: Patrik Horník @ 2009-07-27 20:58 UTC (permalink / raw)
  To: The development of GRUB 2

Hello Bean,

it still does not work, I've tested your patch today against revision
2448 from SVN.

- grub-probe works and detects ex2 filesystem on LVM volume on RAID5 arrays.

- However grub on boot still does not detect filesystem on root/boot
volume. It knows root LVM volume according ls command from grub
rescue, but still goes to rescue mode with error "unknown filesystem".

- grub-install also does not work, it creates core.img without some of
needed modules. It seems that raid modules are missing, because ls in
grub rescue shows only native partitions. I've created core.img
manually to test your patch, with modules "ext2 chain pc biosdisk lvm
raid mdraid".

Patrik Horník


On Sun, Jul 19, 2009 at 21:34, Bean<bean123ch@gmail.com> wrote:
> On Mon, Jul 20, 2009 at 3:11 AM, Patrik Horník<patrik@dsl.sk> wrote:
>> Bean, should Grub2 with this patch work if the boot partition is LVM
>> volume and LVM is placed on top of RAID 5 arrays?
>
> Hi,
>
> Yeah, it should work, please try it out.
>
> --
> Bean
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-27 20:58     ` Patrik Horník
@ 2009-07-28  3:27       ` Bean
  2009-07-28 13:57         ` Bean
  0 siblings, 1 reply; 28+ messages in thread
From: Bean @ 2009-07-28  3:27 UTC (permalink / raw)
  To: The development of GRUB 2

On Tue, Jul 28, 2009 at 4:58 AM, Patrik Horník<patrik@dsl.sk> wrote:
> Hello Bean,
>
> it still does not work, I've tested your patch today against revision
> 2448 from SVN.
>
> - grub-probe works and detects ex2 filesystem on LVM volume on RAID5 arrays.
>
> - However grub on boot still does not detect filesystem on root/boot
> volume. It knows root LVM volume according ls command from grub
> rescue, but still goes to rescue mode with error "unknown filesystem".
>
> - grub-install also does not work, it creates core.img without some of
> needed modules. It seems that raid modules are missing, because ls in
> grub rescue shows only native partitions. I've created core.img
> manually to test your patch, with modules "ext2 chain pc biosdisk lvm
> raid mdraid".

Hi,

Perhaps the install script still has problem to determine the correct
modules, maybe you can try to this list manually:

"ext2 chain pc biosdisk raid mdraid lvm".

The order is important, lvm must be loaded after raid and mdraid.

-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28  3:27       ` Bean
@ 2009-07-28 13:57         ` Bean
  2009-07-28 15:15           ` Bean
  0 siblings, 1 reply; 28+ messages in thread
From: Bean @ 2009-07-28 13:57 UTC (permalink / raw)
  To: The development of GRUB 2

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

Hi,

Oh, I've found the problem. The abstraction is ok, but now partmap is
wrong. The original code assumes at most one level of abstraction when
detecting partmap, and it doesn't work on two level abstraction like
LVM on RAID. If you add module minicmd, you can use lsmod and see pc
module is missing.

I also make some improvement to the patch. Now it also detect the raid
level, and add raid5rec or raid6rec accordingly.

-- 
Bean

[-- Attachment #2: lvm_3.diff --]
[-- Type: application/octet-stream, Size: 8202 bytes --]

diff --git a/disk/lvm.c b/disk/lvm.c
index 6707a40..126b494 100644
--- a/disk/lvm.c
+++ b/disk/lvm.c
@@ -271,15 +271,9 @@ grub_lvm_scan_device (const char *name)
   dlocn++;
   mda_offset = grub_le_to_cpu64 (dlocn->offset);
   mda_size = grub_le_to_cpu64 (dlocn->size);
-  dlocn++;
 
-  if (dlocn->offset)
-    {
-      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "We don't support multiple LVM metadata areas");
-
-      goto fail;
-    }
+  /* It's possible to have multiple copies of metadata areas, we just use the
+     first one.  */
 
   /* Allocate buffer space for the circular worst-case scenario. */
   metadatabuf = grub_malloc (2 * mda_size);
@@ -564,7 +558,10 @@ grub_lvm_scan_device (const char *name)
       {
 	if (! grub_memcmp (pv->id, pv_id, GRUB_LVM_ID_STRLEN))
 	  {
-	    pv->disk = grub_disk_open (name);
+	    /* This could happen to LVM on RAID, pv->disk points to the
+	       raid device, we shouldn't change it.  */
+	    if (! pv->disk)
+	      pv->disk = grub_disk_open (name);
 	    break;
 	  }
       }
diff --git a/disk/raid.c b/disk/raid.c
index 941620c..6acda11 100644
--- a/disk/raid.c
+++ b/disk/raid.c
@@ -596,56 +596,6 @@ insert_array (grub_disk_t disk, struct grub_raid_array *new_array,
 static grub_raid_t grub_raid_list;
 
 static void
-grub_raid_scan_device (int head_only)
-{
-  auto int hook (const char *name);
-  int hook (const char *name)
-    {
-      grub_disk_t disk;
-      struct grub_raid_array array;
-      struct grub_raid *p;
-
-      grub_dprintf ("raid", "Scanning for RAID devices on disk %s\n", name);
-
-      disk = grub_disk_open (name);
-      if (!disk)
-        return 0;
-
-      if (disk->total_sectors == GRUB_ULONG_MAX)
-        {
-          grub_disk_close (disk);
-          return 0;
-        }
-
-      for (p = grub_raid_list; p; p = p->next)
-        {
-          if (! p->detect (disk, &array))
-            {
-              if (! insert_array (disk, &array, p->name))
-                return 0;
-
-              break;
-            }
-
-          /* This error usually means it's not raid, no need to display
-             it.  */
-          if (grub_errno != GRUB_ERR_OUT_OF_RANGE)
-            grub_print_error ();
-
-          grub_errno = GRUB_ERR_NONE;
-          if (head_only)
-            break;
-        }
-
-      grub_disk_close (disk);
-
-      return 0;
-    }
-
-  grub_device_iterate (&hook);
-}
-
-static void
 free_array (void)
 {
   struct grub_raid_array *array;
@@ -674,9 +624,38 @@ free_array (void)
 void
 grub_raid_register (grub_raid_t raid)
 {
+  auto int hook (const char *name);
+  int hook (const char *name)
+    {
+      grub_disk_t disk;
+      struct grub_raid_array array;
+
+      grub_dprintf ("raid", "Scanning for RAID devices on disk %s\n", name);
+
+      disk = grub_disk_open (name);
+      if (!disk)
+        return 0;
+
+      if ((disk->total_sectors != GRUB_ULONG_MAX) &&
+	  (! grub_raid_list->detect (disk, &array)) &&
+	  (! insert_array (disk, &array, grub_raid_list->name)))
+	return 0;
+
+      /* This error usually means it's not raid, no need to display
+	 it.  */
+      if (grub_errno != GRUB_ERR_OUT_OF_RANGE)
+	grub_print_error ();
+
+      grub_errno = GRUB_ERR_NONE;
+
+      grub_disk_close (disk);
+
+      return 0;
+    }
+
   raid->next = grub_raid_list;
   grub_raid_list = raid;
-  grub_raid_scan_device (1);
+  grub_device_iterate (&hook);
 }
 
 void
@@ -692,13 +671,6 @@ grub_raid_unregister (grub_raid_t raid)
       }
 }
 
-void
-grub_raid_rescan (void)
-{
-  free_array ();
-  grub_raid_scan_device (0);
-}
-
 static struct grub_disk_dev grub_raid_dev =
   {
     .name = "raid",
diff --git a/include/grub/raid.h b/include/grub/raid.h
index 595ced1..8fa4c38 100644
--- a/include/grub/raid.h
+++ b/include/grub/raid.h
@@ -67,7 +67,6 @@ typedef struct grub_raid *grub_raid_t;
 void grub_raid_register (grub_raid_t raid);
 void grub_raid_unregister (grub_raid_t raid);
 
-void grub_raid_rescan (void);
 void grub_raid_block_xor (char *buf1, const char *buf2, int size);
 
 typedef grub_err_t (*grub_raid5_recover_func_t) (struct grub_raid_array *array,
diff --git a/util/grub-fstest.c b/util/grub-fstest.c
index 4722269..a4de2ae 100644
--- a/util/grub-fstest.c
+++ b/util/grub-fstest.c
@@ -28,7 +28,6 @@
 #include <grub/env.h>
 #include <grub/term.h>
 #include <grub/mm.h>
-#include <grub/raid.h>
 #include <grub/lib/hexdump.h>
 #include <grub/lib/crc.h>
 #include <grub/command.h>
@@ -293,7 +292,13 @@ fstest (char **images, int num_disks, int cmd, int n, char **args)
         grub_util_error ("loopback command fails.");
     }
 
-  grub_raid_rescan ();
+  grub_lvm_fini ();
+  grub_mdraid_fini ();
+  grub_raid_fini ();
+  grub_raid_init ();
+  grub_mdraid_init ();
+  grub_lvm_init ();
+
   switch (cmd)
     {
     case CMD_LS:
diff --git a/util/grub-probe.c b/util/grub-probe.c
index 97d3860..0042f0b 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -30,6 +30,7 @@
 #include <grub/util/getroot.h>
 #include <grub/term.h>
 #include <grub/env.h>
+#include <grub/raid.h>
 
 #include <grub_probe_init.h>
 
@@ -100,13 +101,21 @@ probe_partmap (grub_disk_t disk)
   free (name);
 }
 
+static int
+probe_raid_level (grub_disk_t disk)
+{
+  if (disk->dev->id != GRUB_DISK_DEVICE_RAID_ID)
+    return -1;
+
+  return ((struct grub_raid_array *) disk->data)->level;
+}
+
 static void
 probe (const char *path, char *device_name)
 {
   char *drive_name = NULL;
   char *grub_path = NULL;
   char *filebuf_via_grub = NULL, *filebuf_via_sys = NULL;
-  int abstraction_type;
   grub_device_t dev = NULL;
   grub_fs_t fs;
 
@@ -132,28 +141,6 @@ probe (const char *path, char *device_name)
       goto end;
     }
 
-  abstraction_type = grub_util_get_dev_abstraction (device_name);
-  /* No need to check for errors; lack of abstraction is permissible.  */
-
-  if (print == PRINT_ABSTRACTION)
-    {
-      char *abstraction_name;
-      switch (abstraction_type)
-	{
-	case GRUB_DEV_ABSTRACTION_LVM:
-	  abstraction_name = "lvm";
-	  break;
-	case GRUB_DEV_ABSTRACTION_RAID:
-	  abstraction_name = "raid mdraid";
-	  break;
-	default:
-	  grub_util_info ("did not find LVM/RAID in %s, assuming raw device", device_name);
-	  goto end;
-	}
-      printf ("%s\n", abstraction_name);
-      goto end;
-    }
-
   drive_name = grub_util_get_grub_dev (device_name);
   if (! drive_name)
     grub_util_error ("Cannot find a GRUB drive for %s.  Check your device.map.\n", device_name);
@@ -169,6 +156,56 @@ probe (const char *path, char *device_name)
   if (! dev)
     grub_util_error ("%s", grub_errmsg);
 
+  if (print == PRINT_ABSTRACTION)
+    {
+      grub_disk_memberlist_t list = NULL, tmp;
+      const int is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
+      int is_raid = 0;
+      int is_raid5 = 0;
+      int is_raid6 = 0;
+      int raid_level;
+
+      raid_level = probe_raid_level (dev->disk);
+      if (raid_level >= 0)
+	{
+	  is_raid = 1;
+	  is_raid5 |= (raid_level == 5);
+	  is_raid6 |= (raid_level == 6);
+	}
+
+      if ((is_lvm) && (dev->disk->dev->memberlist))
+	list = dev->disk->dev->memberlist (dev->disk);
+      while (list)
+	{
+	  raid_level = probe_raid_level (list->disk);
+	  if (raid_level >= 0)
+	    {
+	      is_raid = 1;
+	      is_raid5 |= (raid_level == 5);
+	      is_raid6 |= (raid_level == 6);
+	    }
+
+	  tmp = list->next;
+	  free (list);
+	  list = tmp;
+	}
+
+      if (is_raid)
+	{
+	  printf ("raid\n");
+	  if (is_raid5)
+	    printf ("raid5rec\n");
+	  if (is_raid6)
+	    printf ("raid6rec\n");
+	  printf ("mdraid\n");
+	}
+
+      if (is_lvm)
+	printf ("lvm\n");
+
+      goto end;
+    }
+
   if (print == PRINT_PARTMAP)
     {
       grub_disk_memberlist_t list = NULL, tmp;
@@ -182,6 +219,20 @@ probe (const char *path, char *device_name)
       while (list)
 	{
 	  probe_partmap (list->disk);
+	  /* LVM on RAID  */
+	  if (list->disk->dev->memberlist)
+	    {
+	      grub_disk_memberlist_t sub_list;
+
+	      sub_list = list->disk->dev->memberlist (list->disk);
+	      while (sub_list)
+		{
+		  probe_partmap (sub_list->disk);
+		  tmp = sub_list->next;
+		  free (sub_list);
+		  sub_list = tmp;
+		}
+	    }
 	  tmp = list->next;
 	  free (list);
 	  list = tmp;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 13:57         ` Bean
@ 2009-07-28 15:15           ` Bean
  2009-07-28 15:16             ` Bean
                               ` (3 more replies)
  0 siblings, 4 replies; 28+ messages in thread
From: Bean @ 2009-07-28 15:15 UTC (permalink / raw)
  To: The development of GRUB 2

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

Hi,

BTW, I found a regression on r2421 that cause raid5rec to fail:

http://svn.savannah.gnu.org/viewvc/trunk/grub2/disk/raid6_recover.c?root=grub&r1=2197&r2=2421

buf and buf2 are two different variable.

This new patch include the revert raid5_recover.c to r1828.

On Tue, Jul 28, 2009 at 9:57 PM, Bean<bean123ch@gmail.com> wrote:
> Hi,
>
> Oh, I've found the problem. The abstraction is ok, but now partmap is
> wrong. The original code assumes at most one level of abstraction when
> detecting partmap, and it doesn't work on two level abstraction like
> LVM on RAID. If you add module minicmd, you can use lsmod and see pc
> module is missing.
>
> I also make some improvement to the patch. Now it also detect the raid
> level, and add raid5rec or raid6rec accordingly.
>
> --
> Bean
>



-- 
Bean

[-- Attachment #2: lvm_3.diff --]
[-- Type: text/x-patch, Size: 8685 bytes --]

diff --git a/disk/lvm.c b/disk/lvm.c
index 6707a40..126b494 100644
--- a/disk/lvm.c
+++ b/disk/lvm.c
@@ -271,15 +271,9 @@ grub_lvm_scan_device (const char *name)
   dlocn++;
   mda_offset = grub_le_to_cpu64 (dlocn->offset);
   mda_size = grub_le_to_cpu64 (dlocn->size);
-  dlocn++;
 
-  if (dlocn->offset)
-    {
-      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-		  "We don't support multiple LVM metadata areas");
-
-      goto fail;
-    }
+  /* It's possible to have multiple copies of metadata areas, we just use the
+     first one.  */
 
   /* Allocate buffer space for the circular worst-case scenario. */
   metadatabuf = grub_malloc (2 * mda_size);
@@ -564,7 +558,10 @@ grub_lvm_scan_device (const char *name)
       {
 	if (! grub_memcmp (pv->id, pv_id, GRUB_LVM_ID_STRLEN))
 	  {
-	    pv->disk = grub_disk_open (name);
+	    /* This could happen to LVM on RAID, pv->disk points to the
+	       raid device, we shouldn't change it.  */
+	    if (! pv->disk)
+	      pv->disk = grub_disk_open (name);
 	    break;
 	  }
       }
diff --git a/disk/raid.c b/disk/raid.c
index 941620c..3e832ba 100644
--- a/disk/raid.c
+++ b/disk/raid.c
@@ -596,56 +596,6 @@ insert_array (grub_disk_t disk, struct grub_raid_array *new_array,
 static grub_raid_t grub_raid_list;
 
 static void
-grub_raid_scan_device (int head_only)
-{
-  auto int hook (const char *name);
-  int hook (const char *name)
-    {
-      grub_disk_t disk;
-      struct grub_raid_array array;
-      struct grub_raid *p;
-
-      grub_dprintf ("raid", "Scanning for RAID devices on disk %s\n", name);
-
-      disk = grub_disk_open (name);
-      if (!disk)
-        return 0;
-
-      if (disk->total_sectors == GRUB_ULONG_MAX)
-        {
-          grub_disk_close (disk);
-          return 0;
-        }
-
-      for (p = grub_raid_list; p; p = p->next)
-        {
-          if (! p->detect (disk, &array))
-            {
-              if (! insert_array (disk, &array, p->name))
-                return 0;
-
-              break;
-            }
-
-          /* This error usually means it's not raid, no need to display
-             it.  */
-          if (grub_errno != GRUB_ERR_OUT_OF_RANGE)
-            grub_print_error ();
-
-          grub_errno = GRUB_ERR_NONE;
-          if (head_only)
-            break;
-        }
-
-      grub_disk_close (disk);
-
-      return 0;
-    }
-
-  grub_device_iterate (&hook);
-}
-
-static void
 free_array (void)
 {
   struct grub_raid_array *array;
@@ -674,9 +624,38 @@ free_array (void)
 void
 grub_raid_register (grub_raid_t raid)
 {
+  auto int hook (const char *name);
+  int hook (const char *name)
+    {
+      grub_disk_t disk;
+      struct grub_raid_array array;
+
+      grub_dprintf ("raid", "Scanning for RAID devices on disk %s\n", name);
+
+      disk = grub_disk_open (name);
+      if (!disk)
+        return 0;
+
+      if ((disk->total_sectors != GRUB_ULONG_MAX) &&
+	  (! grub_raid_list->detect (disk, &array)) &&
+	  (! insert_array (disk, &array, grub_raid_list->name)))
+	return 0;
+
+      /* This error usually means it's not raid, no need to display
+	 it.  */
+      if (grub_errno != GRUB_ERR_OUT_OF_RANGE)
+	grub_print_error ();
+
+      grub_errno = GRUB_ERR_NONE;
+
+      grub_disk_close (disk);
+
+      return 0;
+    }
+
   raid->next = grub_raid_list;
   grub_raid_list = raid;
-  grub_raid_scan_device (1);
+  grub_device_iterate (&hook);
 }
 
 void
@@ -692,13 +671,6 @@ grub_raid_unregister (grub_raid_t raid)
       }
 }
 
-void
-grub_raid_rescan (void)
-{
-  free_array ();
-  grub_raid_scan_device (0);
-}
-
 static struct grub_disk_dev grub_raid_dev =
   {
     .name = "raid",
diff --git a/disk/raid5_recover.c b/disk/raid5_recover.c
index a280be1..31cef88 100644
--- a/disk/raid5_recover.c
+++ b/disk/raid5_recover.c
@@ -32,10 +32,12 @@ grub_raid5_recover (struct grub_raid_array *array, int disknr,
   int i;
 
   size <<= GRUB_DISK_SECTOR_BITS;
-  buf2 = grub_zalloc (size);
+  buf2 = grub_malloc (size);
   if (!buf2)
     return grub_errno;
 
+  grub_memset (buf, 0, size);
+
   for (i = 0; i < (int) array->total_devs; i++)
     {
       grub_err_t err;
diff --git a/include/grub/raid.h b/include/grub/raid.h
index 595ced1..8fa4c38 100644
--- a/include/grub/raid.h
+++ b/include/grub/raid.h
@@ -67,7 +67,6 @@ typedef struct grub_raid *grub_raid_t;
 void grub_raid_register (grub_raid_t raid);
 void grub_raid_unregister (grub_raid_t raid);
 
-void grub_raid_rescan (void);
 void grub_raid_block_xor (char *buf1, const char *buf2, int size);
 
 typedef grub_err_t (*grub_raid5_recover_func_t) (struct grub_raid_array *array,
diff --git a/util/grub-fstest.c b/util/grub-fstest.c
index 4722269..1bb3706 100644
--- a/util/grub-fstest.c
+++ b/util/grub-fstest.c
@@ -28,7 +28,6 @@
 #include <grub/env.h>
 #include <grub/term.h>
 #include <grub/mm.h>
-#include <grub/raid.h>
 #include <grub/lib/hexdump.h>
 #include <grub/lib/crc.h>
 #include <grub/command.h>
@@ -293,7 +292,13 @@ fstest (char **images, int num_disks, int cmd, int n, char **args)
         grub_util_error ("loopback command fails.");
     }
 
-  grub_raid_rescan ();
+  grub_lvm_fini ();
+  grub_mdraid_fini ();
+  grub_raid_fini ();
+  grub_raid_init ();
+  grub_mdraid_init ();
+  grub_lvm_init ();
+
   switch (cmd)
     {
     case CMD_LS:
diff --git a/util/grub-probe.c b/util/grub-probe.c
index 97d3860..147e41f 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -30,6 +30,7 @@
 #include <grub/util/getroot.h>
 #include <grub/term.h>
 #include <grub/env.h>
+#include <grub/raid.h>
 
 #include <grub_probe_init.h>
 
@@ -100,13 +101,21 @@ probe_partmap (grub_disk_t disk)
   free (name);
 }
 
+static int
+probe_raid_level (grub_disk_t disk)
+{
+  if (disk->dev->id != GRUB_DISK_DEVICE_RAID_ID)
+    return -1;
+
+  return ((struct grub_raid_array *) disk->data)->level;
+}
+
 static void
 probe (const char *path, char *device_name)
 {
   char *drive_name = NULL;
   char *grub_path = NULL;
   char *filebuf_via_grub = NULL, *filebuf_via_sys = NULL;
-  int abstraction_type;
   grub_device_t dev = NULL;
   grub_fs_t fs;
 
@@ -132,28 +141,6 @@ probe (const char *path, char *device_name)
       goto end;
     }
 
-  abstraction_type = grub_util_get_dev_abstraction (device_name);
-  /* No need to check for errors; lack of abstraction is permissible.  */
-
-  if (print == PRINT_ABSTRACTION)
-    {
-      char *abstraction_name;
-      switch (abstraction_type)
-	{
-	case GRUB_DEV_ABSTRACTION_LVM:
-	  abstraction_name = "lvm";
-	  break;
-	case GRUB_DEV_ABSTRACTION_RAID:
-	  abstraction_name = "raid mdraid";
-	  break;
-	default:
-	  grub_util_info ("did not find LVM/RAID in %s, assuming raw device", device_name);
-	  goto end;
-	}
-      printf ("%s\n", abstraction_name);
-      goto end;
-    }
-
   drive_name = grub_util_get_grub_dev (device_name);
   if (! drive_name)
     grub_util_error ("Cannot find a GRUB drive for %s.  Check your device.map.\n", device_name);
@@ -169,6 +156,56 @@ probe (const char *path, char *device_name)
   if (! dev)
     grub_util_error ("%s", grub_errmsg);
 
+  if (print == PRINT_ABSTRACTION)
+    {
+      grub_disk_memberlist_t list = NULL, tmp;
+      const int is_lvm = (dev->disk->dev->id == GRUB_DISK_DEVICE_LVM_ID);
+      int is_raid = 0;
+      int is_raid5 = 0;
+      int is_raid6 = 0;
+      int raid_level;
+
+      raid_level = probe_raid_level (dev->disk);
+      if (raid_level >= 0)
+	{
+	  is_raid = 1;
+	  is_raid5 |= (raid_level == 5);
+	  is_raid6 |= (raid_level == 6);
+	}
+
+      if ((is_lvm) && (dev->disk->dev->memberlist))
+	list = dev->disk->dev->memberlist (dev->disk);
+      while (list)
+	{
+	  raid_level = probe_raid_level (list->disk);
+	  if (raid_level >= 0)
+	    {
+	      is_raid = 1;
+	      is_raid5 |= (raid_level == 5);
+	      is_raid6 |= (raid_level == 6);
+	    }
+
+	  tmp = list->next;
+	  free (list);
+	  list = tmp;
+	}
+
+      if (is_raid)
+	{
+	  printf ("raid\n");
+	  if (is_raid5)
+	    printf ("raid5rec\n");
+	  if (is_raid6)
+	    printf ("raid6rec\n");
+	  printf ("mdraid\n");
+	}
+
+      if (is_lvm)
+	printf ("lvm\n");
+
+      goto end;
+    }
+
   if (print == PRINT_PARTMAP)
     {
       grub_disk_memberlist_t list = NULL, tmp;
@@ -182,6 +219,20 @@ probe (const char *path, char *device_name)
       while (list)
 	{
 	  probe_partmap (list->disk);
+	  /* LVM on RAID  */
+	  if (list->disk->dev->memberlist)
+	    {
+	      grub_disk_memberlist_t sub_list;
+
+	      sub_list = list->disk->dev->memberlist (list->disk);
+	      while (sub_list)
+		{
+		  probe_partmap (sub_list->disk);
+		  tmp = sub_list->next;
+		  free (sub_list);
+		  sub_list = tmp;
+		}
+	    }
 	  tmp = list->next;
 	  free (list);
 	  list = tmp;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 15:15           ` Bean
@ 2009-07-28 15:16             ` Bean
  2009-07-28 17:42             ` Robert Millan
                               ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Bean @ 2009-07-28 15:16 UTC (permalink / raw)
  To: The development of GRUB 2

Hi,

Oh, the link should be:

http://svn.savannah.gnu.org/viewvc/trunk/grub2/disk/raid5_recover.c?root=grub&r1=2197&r2=2421

On Tue, Jul 28, 2009 at 11:15 PM, Bean<bean123ch@gmail.com> wrote:
> Hi,
>
> BTW, I found a regression on r2421 that cause raid5rec to fail:
>
> http://svn.savannah.gnu.org/viewvc/trunk/grub2/disk/raid6_recover.c?root=grub&r1=2197&r2=2421
>
> buf and buf2 are two different variable.
>
> This new patch include the revert raid5_recover.c to r1828.
>
> On Tue, Jul 28, 2009 at 9:57 PM, Bean<bean123ch@gmail.com> wrote:
>> Hi,
>>
>> Oh, I've found the problem. The abstraction is ok, but now partmap is
>> wrong. The original code assumes at most one level of abstraction when
>> detecting partmap, and it doesn't work on two level abstraction like
>> LVM on RAID. If you add module minicmd, you can use lsmod and see pc
>> module is missing.
>>
>> I also make some improvement to the patch. Now it also detect the raid
>> level, and add raid5rec or raid6rec accordingly.
>>
>> --
>> Bean
>>
>
>
>
> --
> Bean
>



-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 15:15           ` Bean
  2009-07-28 15:16             ` Bean
@ 2009-07-28 17:42             ` Robert Millan
  2009-07-28 21:37               ` Felix Zielcke
  2009-07-29  3:22               ` Bean
  2009-07-30 16:47             ` Patrik Horník
  2009-07-31  4:41             ` Pavel Roskin
  3 siblings, 2 replies; 28+ messages in thread
From: Robert Millan @ 2009-07-28 17:42 UTC (permalink / raw)
  To: The development of GRUB 2

On Tue, Jul 28, 2009 at 11:15:09PM +0800, Bean wrote:
> -  buf2 = grub_zalloc (size);
> +  buf2 = grub_malloc (size);
>    if (!buf2)
>      return grub_errno;
>  
> +  grub_memset (buf, 0, size);

We just received 'buf' as parameter.  Why do we have to zero it here?

> +static int
> +probe_raid_level (grub_disk_t disk)
> +{
> +  if (disk->dev->id != GRUB_DISK_DEVICE_RAID_ID)
> +    return -1;
> +
> +  return ((struct grub_raid_array *) disk->data)->level;
> +}

Since this an ad-hoc function, could you put it in the same block that
needs it?  If 'static' qualifier is present, it won't result in nested
function AFAICT.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 17:42             ` Robert Millan
@ 2009-07-28 21:37               ` Felix Zielcke
  2009-07-29  3:22               ` Bean
  1 sibling, 0 replies; 28+ messages in thread
From: Felix Zielcke @ 2009-07-28 21:37 UTC (permalink / raw)
  To: The development of GRUB 2

Am Dienstag, den 28.07.2009, 19:42 +0200 schrieb Robert Millan:
> On Tue, Jul 28, 2009 at 11:15:09PM +0800, Bean wrote:
> 
> > +static int
> > +probe_raid_level (grub_disk_t disk)
> > +{
> > +  if (disk->dev->id != GRUB_DISK_DEVICE_RAID_ID)
> > +    return -1;
> > +
> > +  return ((struct grub_raid_array *) disk->data)->level;
> > +}
> 
> Since this an ad-hoc function, could you put it in the same block that
> needs it?  If 'static' qualifier is present, it won't result in nested
> function AFAICT.
> 

GCC manual [0] says:

A nested function always has no linkage. Declaring one with extern or
static is erroneous.

[0]
http://gcc.gnu.org/onlinedocs/gcc-4.4.1/gcc/Nested-Functions.html#Nested-Functions

-- 
Felix Zielcke
Proud Debian Maintainer




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 17:42             ` Robert Millan
  2009-07-28 21:37               ` Felix Zielcke
@ 2009-07-29  3:22               ` Bean
  2009-07-31 15:44                 ` Robert Millan
  1 sibling, 1 reply; 28+ messages in thread
From: Bean @ 2009-07-29  3:22 UTC (permalink / raw)
  To: The development of GRUB 2

On Wed, Jul 29, 2009 at 1:42 AM, Robert Millan<rmh@aybabtu.com> wrote:
> On Tue, Jul 28, 2009 at 11:15:09PM +0800, Bean wrote:
>> -  buf2 = grub_zalloc (size);
>> +  buf2 = grub_malloc (size);
>>    if (!buf2)
>>      return grub_errno;
>>
>> +  grub_memset (buf, 0, size);
>
> We just received 'buf' as parameter.  Why do we have to zero it here?

Hi,

Because we are in a recovery function, the original content may not be
correct, we need to clear it out before continue.

>
>> +static int
>> +probe_raid_level (grub_disk_t disk)
>> +{
>> +  if (disk->dev->id != GRUB_DISK_DEVICE_RAID_ID)
>> +    return -1;
>> +
>> +  return ((struct grub_raid_array *) disk->data)->level;
>> +}
>
> Since this an ad-hoc function, could you put it in the same block that
> needs it?  If 'static' qualifier is present, it won't result in nested
> function AFAICT.

It's used by the next function probe, so unless we use nested
function, it can't be placed closer.

-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 15:15           ` Bean
  2009-07-28 15:16             ` Bean
  2009-07-28 17:42             ` Robert Millan
@ 2009-07-30 16:47             ` Patrik Horník
  2009-07-31  4:41             ` Pavel Roskin
  3 siblings, 0 replies; 28+ messages in thread
From: Patrik Horník @ 2009-07-30 16:47 UTC (permalink / raw)
  To: The development of GRUB 2

Hello Bean,

great, it is working now.

However, "grub-probe -t abstraction" separates different modules with
newline character, so grub-mkconfig generates incorrect grub.cfg.

Patrik Horník


On Tue, Jul 28, 2009 at 17:15, Bean<bean123ch@gmail.com> wrote:
> Hi,
>
> BTW, I found a regression on r2421 that cause raid5rec to fail:
>
> http://svn.savannah.gnu.org/viewvc/trunk/grub2/disk/raid6_recover.c?root=grub&r1=2197&r2=2421
>
> buf and buf2 are two different variable.
>
> This new patch include the revert raid5_recover.c to r1828.
>
> On Tue, Jul 28, 2009 at 9:57 PM, Bean<bean123ch@gmail.com> wrote:
>> Hi,
>>
>> Oh, I've found the problem. The abstraction is ok, but now partmap is
>> wrong. The original code assumes at most one level of abstraction when
>> detecting partmap, and it doesn't work on two level abstraction like
>> LVM on RAID. If you add module minicmd, you can use lsmod and see pc
>> module is missing.
>>
>> I also make some improvement to the patch. Now it also detect the raid
>> level, and add raid5rec or raid6rec accordingly.
>>
>> --
>> Bean
>>
>
>
>
> --
> Bean
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-28 15:15           ` Bean
                               ` (2 preceding siblings ...)
  2009-07-30 16:47             ` Patrik Horník
@ 2009-07-31  4:41             ` Pavel Roskin
  2009-07-31 14:28               ` Bean
  3 siblings, 1 reply; 28+ messages in thread
From: Pavel Roskin @ 2009-07-31  4:41 UTC (permalink / raw)
  To: The development of GRUB 2

On Tue, 2009-07-28 at 23:15 +0800, Bean wrote:
> Hi,
> 
> BTW, I found a regression on r2421 that cause raid5rec to fail:
> 
> http://svn.savannah.gnu.org/viewvc/trunk/grub2/disk/raid6_recover.c?root=grub&r1=2197&r2=2421
> 
> buf and buf2 are two different variable.
> 
> This new patch include the revert raid5_recover.c to r1828.

Sorry for that!  I have reverted that part of the patch, and I'm
reviewing the whole commit for possible other errors of that kind.

There is no need to combine reverts with other patches.  An obvious
mistake can be reverted separately.

-- 
Regards,
Pavel Roskin



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-31  4:41             ` Pavel Roskin
@ 2009-07-31 14:28               ` Bean
  2009-07-31 18:30                 ` Felix Zielcke
  0 siblings, 1 reply; 28+ messages in thread
From: Bean @ 2009-07-31 14:28 UTC (permalink / raw)
  To: The development of GRUB 2

Hi,

Committed, grub-probe now now uses space as separator instead of newline.

-- 
Bean



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-29  3:22               ` Bean
@ 2009-07-31 15:44                 ` Robert Millan
  0 siblings, 0 replies; 28+ messages in thread
From: Robert Millan @ 2009-07-31 15:44 UTC (permalink / raw)
  To: The development of GRUB 2

On Wed, Jul 29, 2009 at 11:22:56AM +0800, Bean wrote:
> On Wed, Jul 29, 2009 at 1:42 AM, Robert Millan<rmh@aybabtu.com> wrote:
> > On Tue, Jul 28, 2009 at 11:15:09PM +0800, Bean wrote:
> >> -  buf2 = grub_zalloc (size);
> >> +  buf2 = grub_malloc (size);
> >>    if (!buf2)
> >>      return grub_errno;
> >>
> >> +  grub_memset (buf, 0, size);
> >
> > We just received 'buf' as parameter.  Why do we have to zero it here?
> 
> Hi,
> 
> Because we are in a recovery function, the original content may not be
> correct, we need to clear it out before continue.

Alright.  I think a comment would help, it's not obvious why it's done this
way.

-- 
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."



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-31 14:28               ` Bean
@ 2009-07-31 18:30                 ` Felix Zielcke
  2009-08-06  7:07                   ` Felix Zielcke
  0 siblings, 1 reply; 28+ messages in thread
From: Felix Zielcke @ 2009-07-31 18:30 UTC (permalink / raw)
  To: The development of GRUB 2

Am Freitag, den 31.07.2009, 22:28 +0800 schrieb Bean:
> Hi,
> 
> Committed, grub-probe now now uses space as separator instead of newline.
> 

grub-mkconfig is still broken with it even though you use now spaces
instead of newlines.
insmod doestn't support multiple modules at once.

-- 
Felix Zielcke
Proud Debian Maintainer




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] Bug fix for LVM
  2009-07-31 18:30                 ` Felix Zielcke
@ 2009-08-06  7:07                   ` Felix Zielcke
  0 siblings, 0 replies; 28+ messages in thread
From: Felix Zielcke @ 2009-08-06  7:07 UTC (permalink / raw)
  To: The development of GRUB 2

Am Freitag, den 31.07.2009, 20:30 +0200 schrieb Felix Zielcke:
> Am Freitag, den 31.07.2009, 22:28 +0800 schrieb Bean:
> > Hi,
> > 
> > Committed, grub-probe now now uses space as separator instead of newline.
> > 
> 
> grub-mkconfig is still broken with it even though you use now spaces
> instead of newlines.
> insmod doestn't support multiple modules at once.
> 

I just fixed it now. Now grub-mkconfig prints multiple insmod lines for
them.

-- 
Felix Zielcke
Proud Debian Maintainer




^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2009-08-06  7:06 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-18 14:11 [PATCH] Bug fix for LVM Bean
2009-07-18 19:11 ` Robert Millan
2009-07-18 19:43   ` Bean
2009-07-18 19:57     ` Robert Millan
2009-07-18 19:59     ` Vladimir 'phcoder' Serbinenko
2009-07-18 20:05       ` Robert Millan
2009-07-19  9:41         ` Bean
2009-07-22 17:29           ` Robert Millan
2009-07-22 17:33           ` Robert Millan
2009-07-22 18:58             ` Bean
2009-07-25 16:05               ` Robert Millan
2009-07-19 19:11 ` Patrik Horník
2009-07-19 19:34   ` Bean
2009-07-25 20:37     ` Felix Zielcke
2009-07-27 20:58     ` Patrik Horník
2009-07-28  3:27       ` Bean
2009-07-28 13:57         ` Bean
2009-07-28 15:15           ` Bean
2009-07-28 15:16             ` Bean
2009-07-28 17:42             ` Robert Millan
2009-07-28 21:37               ` Felix Zielcke
2009-07-29  3:22               ` Bean
2009-07-31 15:44                 ` Robert Millan
2009-07-30 16:47             ` Patrik Horník
2009-07-31  4:41             ` Pavel Roskin
2009-07-31 14:28               ` Bean
2009-07-31 18:30                 ` Felix Zielcke
2009-08-06  7:07                   ` Felix Zielcke

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.