Thankfully, no further changes were actually necessary, so here is my
attempt to convert it to two patches

I attached the diffs, but also committed them to
https://github.com/byteit101/grub2/tree/grub-lvmintegrity


Patrick

On Thu, Feb 8, 2024 at 3:06 PM Daniel Kiper <dki...@net-space.pl> wrote:
>
> On Thu, Feb 08, 2024 at 02:52:37PM -0500, Patrick Plenefisch wrote:
> > Hmm, what would the logical parts be? The solution for both cachevol and
> > integrity is the same.
>
> It seems to me at least code refactoring which you are doing could be
> taken out to a separate patch. Maybe something else... Anyway, in general
> smaller patches ease reviewing...
>
> > I do know another part needs to be added as I still need to investigate some
> > warnings, but I'll likely need to do that this weekend when I have some more
> > time.
>
> Cool! Thanks!
>
> Daniel
diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 21e239511..dc3bd943b 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -966,8 +966,6 @@ grub_diskfilter_vg_register (struct grub_diskfilter_vg *vg)
 
   for (lv = vg->lvs; lv; lv = lv->next)
     {
-      grub_err_t err;
-
       /* RAID 1 and single-disk RAID 0 don't use a chunksize but code
          assumes one so set one. */
       for (i = 0; i < lv->segment_count; i++)
@@ -979,6 +977,10 @@ grub_diskfilter_vg_register (struct grub_diskfilter_vg *vg)
 	      && lv->segments[i].stripe_size == 0)
 	    lv->segments[i].stripe_size = 64;
 	}
+    }
+  for (lv = vg->lvs; lv; lv = lv->next)
+    {
+      grub_err_t err;
 
       err = validate_lv(lv);
       if (err)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 10bc965a4..7615a507a 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -805,13 +805,27 @@ grub_lvm_detect (grub_disk_t disk,
 			  seg->nodes[seg->node_count - 1].name = tmp;
 			}
 		    }
+		  /* Cache and integrity LVs have extra parts that
+		     we can ignore for our read-only access */
 		  else if (grub_memcmp (p, "cache\"",
-				   sizeof ("cache\"") - 1) == 0)
+				   sizeof ("cache\"") - 1) == 0
+				   || grub_memcmp (p, "cache+CACHE_USES_CACHEVOL\"",
+				   sizeof ("cache+CACHE_USES_CACHEVOL\"") - 1) == 0
+				   || grub_memcmp (p, "integrity\"",
+				   sizeof ("integrity\"") - 1) == 0)
 		    {
 		      struct ignored_feature_lv *ignored_feature = NULL;
 
 		      char *p2, *p3;
 		      grub_size_t sz;
+#ifdef GRUB_UTIL
+		      p2 = grub_strchr (p, '"');
+		      if (p2)
+			*p2 = 0;
+		      grub_util_info ("Ignoring extra metadata type '%s' for %s", p, lv->name);
+		      if (p2)
+			*p2 ='"';
+#endif
 
 		      ignored_feature = grub_zalloc (sizeof (*ignored_feature));
 		      if (!ignored_feature)
@@ -936,36 +950,6 @@ grub_lvm_detect (grub_disk_t disk,
 	    }
 	}
 
-      /* Match lvs.  */
-      {
-	struct grub_diskfilter_lv *lv1;
-	struct grub_diskfilter_lv *lv2;
-	for (lv1 = vg->lvs; lv1; lv1 = lv1->next)
-	  for (i = 0; i < lv1->segment_count; i++)
-	    for (j = 0; j < lv1->segments[i].node_count; j++)
-	      {
-		if (vg->pvs)
-		  for (pv = vg->pvs; pv; pv = pv->next)
-		    {
-		      if (! grub_strcmp (pv->name,
-					 lv1->segments[i].nodes[j].name))
-			{
-			  lv1->segments[i].nodes[j].pv = pv;
-			  break;
-			}
-		    }
-		if (lv1->segments[i].nodes[j].pv == NULL)
-		  for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
-		    {
-		      if (lv1 == lv2)
-		        continue;
-		      if (grub_strcmp (lv2->name,
-				       lv1->segments[i].nodes[j].name) == 0)
-			lv1->segments[i].nodes[j].lv = lv2;
-		    }
-	      }
-
-      }
 
       {
 	struct ignored_feature_lv *ignored_feature;
@@ -1014,9 +998,46 @@ grub_lvm_detect (grub_disk_t disk,
 		    ignored_feature->lv = NULL;
 		  }
 	      }
+		  else
+		  {
+
+#ifdef GRUB_UTIL
+		      grub_util_info ("Couldn't find LVM part of ignored feature on %s", ignored_feature->origin);
+#endif
+		  }
 	  }
       }
 
+      /* Match lvs. Must be done after cache and integrity are found  */
+      {
+	struct grub_diskfilter_lv *lv1;
+	struct grub_diskfilter_lv *lv2;
+	for (lv1 = vg->lvs; lv1; lv1 = lv1->next)
+	  for (i = 0; i < lv1->segment_count; i++)
+	    for (j = 0; j < lv1->segments[i].node_count; j++)
+	      {
+		if (vg->pvs)
+		  for (pv = vg->pvs; pv; pv = pv->next)
+		    {
+		      if (! grub_strcmp (pv->name,
+					 lv1->segments[i].nodes[j].name))
+			{
+			  lv1->segments[i].nodes[j].pv = pv;
+			  break;
+			}
+		    }
+		if (lv1->segments[i].nodes[j].pv == NULL)
+		  for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
+		    {
+		      if (lv1 == lv2)
+		        continue;
+		      if (grub_strcmp (lv2->name,
+				       lv1->segments[i].nodes[j].name) == 0)
+			lv1->segments[i].nodes[j].lv = lv2;
+		    }
+	      }
+      }
+
 	  grub_lvm_free_ignored_feature_lvs (ignored_feature_lvs);
       if (grub_diskfilter_vg_register (vg))
 	goto fail4;
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 794248540..10bc965a4 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -34,12 +34,11 @@
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
-struct cache_lv
+struct ignored_feature_lv
 {
   struct grub_diskfilter_lv *lv;
-  char *cache_pool;
   char *origin;
-  struct cache_lv *next;
+  struct ignored_feature_lv *next;
 };
 
 
@@ -105,30 +104,29 @@ grub_lvm_check_flag (const char *p, const char *str, const char *flag)
 }
 
 static void
-grub_lvm_free_cache_lvs (struct cache_lv *cache_lvs)
+grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv *ignored_feature_lvs)
 {
-  struct cache_lv *cache;
+  struct ignored_feature_lv *ignored_feature;
 
-  while ((cache = cache_lvs))
+  while ((ignored_feature = ignored_feature_lvs))
     {
-      cache_lvs = cache_lvs->next;
+      ignored_feature_lvs = ignored_feature_lvs->next;
 
-      if (cache->lv)
+      if (ignored_feature->lv)
 	{
 	  unsigned int i;
 
-	  for (i = 0; i < cache->lv->segment_count; ++i)
-	    if (cache->lv->segments)
-	      grub_free (cache->lv->segments[i].nodes);
-	  grub_free (cache->lv->segments);
-	  grub_free (cache->lv->fullname);
-	  grub_free (cache->lv->idname);
-	  grub_free (cache->lv->name);
+	  for (i = 0; i < ignored_feature->lv->segment_count; ++i)
+	    if (ignored_feature->lv->segments)
+	      grub_free (ignored_feature->lv->segments[i].nodes);
+	  grub_free (ignored_feature->lv->segments);
+	  grub_free (ignored_feature->lv->fullname);
+	  grub_free (ignored_feature->lv->idname);
+	  grub_free (ignored_feature->lv->name);
 	}
-      grub_free (cache->lv);
-      grub_free (cache->origin);
-      grub_free (cache->cache_pool);
-      grub_free (cache);
+      grub_free (ignored_feature->lv);
+      grub_free (ignored_feature->origin);
+      grub_free (ignored_feature);
     }
 }
 
@@ -325,7 +323,7 @@ grub_lvm_detect (grub_disk_t disk,
 
   if (! vg)
     {
-      struct cache_lv *cache_lvs = NULL;
+      struct ignored_feature_lv *ignored_feature_lvs = NULL;
 
       /* First time we see this volume group. We've to create the
 	 whole volume group structure. */
@@ -810,105 +808,82 @@ grub_lvm_detect (grub_disk_t disk,
 		  else if (grub_memcmp (p, "cache\"",
 				   sizeof ("cache\"") - 1) == 0)
 		    {
-		      struct cache_lv *cache = NULL;
+		      struct ignored_feature_lv *ignored_feature = NULL;
 
 		      char *p2, *p3;
 		      grub_size_t sz;
 
-		      cache = grub_zalloc (sizeof (*cache));
-		      if (!cache)
-			goto cache_lv_fail;
-		      cache->lv = grub_zalloc (sizeof (*cache->lv));
-		      if (!cache->lv)
-			goto cache_lv_fail;
-		      grub_memcpy (cache->lv, lv, sizeof (*cache->lv));
+		      ignored_feature = grub_zalloc (sizeof (*ignored_feature));
+		      if (!ignored_feature)
+			goto ignored_feature_lv_fail;
+		      ignored_feature->lv = grub_zalloc (sizeof (*ignored_feature->lv));
+		      if (!ignored_feature->lv)
+			goto ignored_feature_lv_fail;
+		      grub_memcpy (ignored_feature->lv, lv, sizeof (*ignored_feature->lv));
 
 		      if (lv->fullname)
 			{
-			  cache->lv->fullname = grub_strdup (lv->fullname);
-			  if (!cache->lv->fullname)
-			    goto cache_lv_fail;
+			  ignored_feature->lv->fullname = grub_strdup (lv->fullname);
+			  if (!ignored_feature->lv->fullname)
+			    goto ignored_feature_lv_fail;
 			}
 		      if (lv->idname)
 			{
-			  cache->lv->idname = grub_strdup (lv->idname);
-			  if (!cache->lv->idname)
-			    goto cache_lv_fail;
+			  ignored_feature->lv->idname = grub_strdup (lv->idname);
+			  if (!ignored_feature->lv->idname)
+			    goto ignored_feature_lv_fail;
 			}
 		      if (lv->name)
 			{
-			  cache->lv->name = grub_strdup (lv->name);
-			  if (!cache->lv->name)
-			    goto cache_lv_fail;
+			  ignored_feature->lv->name = grub_strdup (lv->name);
+			  if (!ignored_feature->lv->name)
+			    goto ignored_feature_lv_fail;
 			}
 
 		      skip_lv = 1;
 
-		      p2 = grub_strstr (p, "cache_pool = \"");
-		      if (!p2)
-			goto cache_lv_fail;
-
-		      p2 = grub_strchr (p2, '"');
-		      if (!p2)
-			goto cache_lv_fail;
-
-		      p3 = ++p2;
-		      if (p3 == mda_end)
-			goto cache_lv_fail;
-		      p3 = grub_strchr (p3, '"');
-		      if (!p3)
-			goto cache_lv_fail;
-
-		      sz = p3 - p2;
-
-		      cache->cache_pool = grub_malloc (sz + 1);
-		      if (!cache->cache_pool)
-			goto cache_lv_fail;
-		      grub_memcpy (cache->cache_pool, p2, sz);
-		      cache->cache_pool[sz] = '\0';
 
 		      p2 = grub_strstr (p, "origin = \"");
 		      if (!p2)
-			goto cache_lv_fail;
+			goto ignored_feature_lv_fail;
 
 		      p2 = grub_strchr (p2, '"');
 		      if (!p2)
-			goto cache_lv_fail;
+			goto ignored_feature_lv_fail;
 
 		      p3 = ++p2;
 		      if (p3 == mda_end)
-			goto cache_lv_fail;
+			goto ignored_feature_lv_fail;
 		      p3 = grub_strchr (p3, '"');
 		      if (!p3)
-			goto cache_lv_fail;
+			goto ignored_feature_lv_fail;
 
 		      sz = p3 - p2;
 
-		      cache->origin = grub_malloc (sz + 1);
-		      if (!cache->origin)
-			goto cache_lv_fail;
-		      grub_memcpy (cache->origin, p2, sz);
-		      cache->origin[sz] = '\0';
+		      ignored_feature->origin = grub_malloc (sz + 1);
+		      if (!ignored_feature->origin)
+			goto ignored_feature_lv_fail;
+		      grub_memcpy (ignored_feature->origin, p2, sz);
+		      ignored_feature->origin[sz] = '\0';
 
-		      cache->next = cache_lvs;
-		      cache_lvs = cache;
+		      ignored_feature->next = ignored_feature_lvs;
+		      ignored_feature_lvs = ignored_feature;
 		      break;
 
-		    cache_lv_fail:
-		      if (cache)
+		    ignored_feature_lv_fail:
+		      if (ignored_feature)
 			{
-			  grub_free (cache->origin);
-			  grub_free (cache->cache_pool);
-			  if (cache->lv)
+			  grub_free (ignored_feature->origin);
+			  if (ignored_feature->lv)
 			    {
-			      grub_free (cache->lv->fullname);
-			      grub_free (cache->lv->idname);
-			      grub_free (cache->lv->name);
+			      grub_free (ignored_feature->lv->fullname);
+			      grub_free (ignored_feature->lv->idname);
+			      grub_free (ignored_feature->lv->name);
 			    }
-			  grub_free (cache->lv);
-			  grub_free (cache);
+			  grub_free (ignored_feature->lv);
+			  grub_free (ignored_feature);
 			}
-		      grub_lvm_free_cache_lvs (cache_lvs);
+		      grub_lvm_free_ignored_feature_lvs (ignored_feature_lvs);
 		      goto fail4;
 		    }
 		  else
@@ -993,56 +968,56 @@ grub_lvm_detect (grub_disk_t disk,
       }
 
       {
-	struct cache_lv *cache;
+	struct ignored_feature_lv *ignored_feature;
 
-	for (cache = cache_lvs; cache; cache = cache->next)
+	for (ignored_feature = ignored_feature_lvs; ignored_feature; ignored_feature = ignored_feature->next)
 	  {
 	    struct grub_diskfilter_lv *lv;
 
 	    for (lv = vg->lvs; lv; lv = lv->next)
-	      if (grub_strcmp (lv->name, cache->origin) == 0)
+	      if (grub_strcmp (lv->name, ignored_feature->origin) == 0)
 		break;
 	    if (lv)
 	      {
-		cache->lv->segments = grub_calloc (lv->segment_count, sizeof (*lv->segments));
-		if (!cache->lv->segments)
+		ignored_feature->lv->segments = grub_calloc (lv->segment_count, sizeof (*lv->segments));
+		if (!ignored_feature->lv->segments)
 		  {
-		    grub_lvm_free_cache_lvs (cache_lvs);
+		    grub_lvm_free_ignored_feature_lvs (ignored_feature_lvs);
 		    goto fail4;
 		  }
-		grub_memcpy (cache->lv->segments, lv->segments, lv->segment_count * sizeof (*lv->segments));
+		grub_memcpy (ignored_feature->lv->segments, lv->segments, lv->segment_count * sizeof (*lv->segments));
 
 		for (i = 0; i < lv->segment_count; ++i)
 		  {
 		    struct grub_diskfilter_node *nodes = lv->segments[i].nodes;
 		    grub_size_t node_count = lv->segments[i].node_count;
 
-		    cache->lv->segments[i].nodes = grub_calloc (node_count, sizeof (*nodes));
-		    if (!cache->lv->segments[i].nodes)
+		    ignored_feature->lv->segments[i].nodes = grub_calloc (node_count, sizeof (*nodes));
+		    if (!ignored_feature->lv->segments[i].nodes)
 		      {
 			for (j = 0; j < i; ++j)
-			  grub_free (cache->lv->segments[j].nodes);
-			grub_free (cache->lv->segments);
-			cache->lv->segments = NULL;
-			grub_lvm_free_cache_lvs (cache_lvs);
+			  grub_free (ignored_feature->lv->segments[j].nodes);
+			grub_free (ignored_feature->lv->segments);
+			ignored_feature->lv->segments = NULL;
+			grub_lvm_free_ignored_feature_lvs (ignored_feature_lvs);
 			goto fail4;
 		      }
-		    grub_memcpy (cache->lv->segments[i].nodes, nodes, node_count * sizeof (*nodes));
+		    grub_memcpy (ignored_feature->lv->segments[i].nodes, nodes, node_count * sizeof (*nodes));
 		  }
 
-		if (cache->lv->segments)
+		if (ignored_feature->lv->segments)
 		  {
-		    cache->lv->segment_count = lv->segment_count;
-		    cache->lv->vg = vg;
-		    cache->lv->next = vg->lvs;
-		    vg->lvs = cache->lv;
-		    cache->lv = NULL;
+		    ignored_feature->lv->segment_count = lv->segment_count;
+		    ignored_feature->lv->vg = vg;
+		    ignored_feature->lv->next = vg->lvs;
+		    vg->lvs = ignored_feature->lv;
+		    ignored_feature->lv = NULL;
 		  }
 	      }
 	  }
       }
 
-      grub_lvm_free_cache_lvs (cache_lvs);
+	  grub_lvm_free_ignored_feature_lvs (ignored_feature_lvs);
       if (grub_diskfilter_vg_register (vg))
 	goto fail4;
     }
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to