[PATCH] diskfilter: use nodes in logical volume's segment as member device

* [PATCH] diskfilter: use nodes in logical volume's segment as member device
@ 2021-08-02  9:40 Michael Chang
  2021-08-09 15:34 ` Daniel Kiper
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Chang @ 2021-08-02  9:40 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Michael Chang, Olav Reinert

Currently the grub_diskfilter_memberlist function returns all physical
volumes added to a volume group to which a logical volume (LV) belongs.
However this is suboptimal as it doesn't fit the intended behavior of
returning underlying devices that make up the LV. To give a clear
picture, the result should be identical to running commands below to
display the logical volumes with underlying physical volumes in use.

  lvs -o +devices /dev/system/root
  lvdisplay --maps /dev/system/root

This change is required if any part of the PV not used by root LV is
encrypted. Using this lvm setup as an example:

  localhost:~ # lsblk
  NAME            MAJ:MIN RM  SIZE RO TYPE  MOUNTPOINT
  vda             253:0    0   20G  0 disk
  ├─vda1          253:1    0    8M  0 part
  └─vda2          253:2    0   20G  0 part
    ├─system-swap 254:0    0  1.4G  0 lvm   [SWAP]
    └─system-root 254:1    0 18.6G  0 lvm   /
  vdb             253:16   0   10M  0 disk
  └─data          254:2    0    8M  0 crypt
    └─system-data 254:3    0    4M  0 lvm   /data

Running grub-install would end up with error because "system" VG
contains /dev/vdb which is encrypted.

  error: attempt to install to encrypted disk without cryptodisk
  enabled. Set `GRUB_ENABLE_CRYPTODISK=y' in file `/etc/default/grub'.

Certainly we can enable GRUB_ENABLE_CRYPTODISK=y and move on, but that
is not always acceptable since the server may need to boot unattended,
in addition typing passphase for every system startup can be a big
hassle of which most users would like to avoid.

This patch solves the problem by returning physical volumes, /dev/vda2,
rightly used by system-root in the example above, thus changing how
grub-install perceives the underlying block device to boot and avoids
the error from happening.

Signed-off-by: Michael Chang <mchang@suse.de>
Tested-by: Olav Reinert <seroton10@gmail.com>
---
 grub-core/disk/diskfilter.c | 44 +++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 6eb2349a6..595f5f70f 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -300,6 +300,8 @@ grub_diskfilter_memberlist (grub_disk_t disk)
   grub_disk_dev_t p;
   struct grub_diskfilter_vg *vg;
   struct grub_diskfilter_lv *lv2 = NULL;
+  struct grub_diskfilter_segment *seg;
+  unsigned int i, j;
 
   if (!lv->vg->pvs)
     return NULL;
@@ -331,25 +333,33 @@ grub_diskfilter_memberlist (grub_disk_t disk)
 	    }
     }
 
-  for (pv = lv->vg->pvs; pv; pv = pv->next)
-    {
-      if (!pv->disk)
+  for (i = 0, seg = lv->segments; i < lv->segment_count; i++, seg++)
+    for (j =0; j < seg->node_count; ++j)
+      if ((pv = seg->nodes[j].pv))
 	{
-	  /* TRANSLATORS: This message kicks in during the detection of
-	     which modules needs to be included in core image. This happens
-	     in the case of degraded RAID and means that autodetection may
-	     fail to include some of modules. It's an installation time
-	     message, not runtime message.  */
-	  grub_util_warn (_("Couldn't find physical volume `%s'."
-			    " Some modules may be missing from core image."),
-			  pv->name);
-	  continue;
+
+	  if (!pv->disk)
+	    {
+	      /* TRANSLATORS: This message kicks in during the detection of
+		 which modules needs to be included in core image. This happens
+		 in the case of degraded RAID and means that autodetection may
+		 fail to include some of modules. It's an installation time
+		 message, not runtime message.  */
+	      grub_util_warn (_("Couldn't find physical volume `%s'."
+				" Some modules may be missing from core image."),
+			      pv->name);
+	      continue;
+	    }
+
+	  for (tmp = list; tmp; tmp = tmp->next)
+	    if (grub_strcmp (tmp->disk->name, pv->disk->name) == 0)
+	      continue;
+
+	  tmp = grub_malloc (sizeof (*tmp));
+	  tmp->disk = pv->disk;
+	  tmp->next = list;
+	  list = tmp;
 	}
-      tmp = grub_malloc (sizeof (*tmp));
-      tmp->disk = pv->disk;
-      tmp->next = list;
-      list = tmp;
-    }
 
   return list;
 }
-- 
2.31.1



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