[lvm-devel] master - fix duplicate pv size check

[David Teigland]

Gitweb:        https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=dcbed38b3339ce4da722bccec8eaf7b8d775a6c2
Commit:        dcbed38b3339ce4da722bccec8eaf7b8d775a6c2
Parent:        32a8865a272d31d5bc12332a4da0309ce3af9243
Author:        David Teigland <teigland at redhat.com>
AuthorDate:    Tue Aug 27 15:40:24 2019 -0500
Committer:     David Teigland <teigland at redhat.com>
CommitterDate: Tue Aug 27 15:40:24 2019 -0500

fix duplicate pv size check

Fixes a segfault in the recent commit e01fddc57:
"improve duplicate pv handling for md components"

While choosing between duplicates, the info struct is
not always valid; it may have been dropped already.

Remove the code that was still using the info struct for
size comparisons.  The size comparisons were a bogus check
anyway because it was just preferring the dev that had
already been chosen, it wasn't actually comparing the
dev size to the PV size.  It would be good to use a
dev/PV size comparison in the duplicate handling code, but
the PV size is not available until after vg_read, not
from the scan.
---
 lib/cache/lvmcache.c |   24 ------------------------
 1 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 87c0021..29d6446 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -491,12 +491,10 @@ static void _choose_duplicates(struct cmd_context *cmd,
 	struct lvmcache_info *info;
 	struct device *dev1, *dev2;
 	uint32_t dev1_major, dev1_minor, dev2_major, dev2_minor;
-	uint64_t info_size, dev1_size, dev2_size;
 	int in_subsys1, in_subsys2;
 	int is_dm1, is_dm2;
 	int has_fs1, has_fs2;
 	int has_lv1, has_lv2;
-	int same_size1, same_size2;
 	int prev_unchosen1, prev_unchosen2;
 	int change;
 
@@ -613,11 +611,6 @@ next:
 		dev2_major = MAJOR(dev2->dev);
 		dev2_minor = MINOR(dev2->dev);
 
-		if (!dev_get_size(dev1, &dev1_size))
-			dev1_size = 0;
-		if (!dev_get_size(dev2, &dev2_size))
-			dev2_size = 0;
-
 		has_lv1 = (dev1->flags & DEV_USED_FOR_LV) ? 1 : 0;
 		has_lv2 = (dev2->flags & DEV_USED_FOR_LV) ? 1 : 0;
 
@@ -630,21 +623,11 @@ next:
 		has_fs1 = dm_device_has_mounted_fs(dev1_major, dev1_minor);
 		has_fs2 = dm_device_has_mounted_fs(dev2_major, dev2_minor);
 
-		info_size = info->device_size >> SECTOR_SHIFT;
-		same_size1 = (dev1_size == info_size);
-		same_size2 = (dev2_size == info_size);
-
 		log_debug_cache("PV %s compare duplicates: %s %u:%u. %s %u:%u.",
 				devl->dev->pvid,
 				dev_name(dev1), dev1_major, dev1_minor,
 				dev_name(dev2), dev2_major, dev2_minor);
 
-		log_debug_cache("PV %s: wants size %llu. %s is %llu. %s is %llu.",
-				devl->dev->pvid,
-				(unsigned long long)info_size,
-				dev_name(dev1), (unsigned long long)dev1_size,
-				dev_name(dev2), (unsigned long long)dev2_size);
-
 		log_debug_cache("PV %s: %s was prev %s. %s was prev %s.",
 				devl->dev->pvid,
 				dev_name(dev1), prev_unchosen1 ? "not chosen" : "<none>",
@@ -686,13 +669,6 @@ next:
 			/* change to 2 */
 			change = 1;
 			reason = "device is used by LV";
-		} else if (same_size1 && !same_size2) {
-			/* keep 1 */
-			reason = "device size is correct";
-		} else if (same_size2 && !same_size1) {
-			/* change to 2 */
-			change = 1;
-			reason = "device size is correct";
 		} else if (has_fs1 && !has_fs2) {
 			/* keep 1 */
 			reason = "device has fs mounted";