105 lines
3.9 KiB
Diff
105 lines
3.9 KiB
Diff
|
From f2e8b49e1dd70f3735ff556f3f7296078a057a5e Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Peter Rajnoha <prajnoha@redhat.com>
|
|||
|
|
Date: Tue, 5 Nov 2024 09:26:03 +0100
|
|||
|
|
Subject: [PATCH 08/13] lv_manip: fix stripe count and size validation for RAID
|
|||
|
|
LVs
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
Fix stripe count and size parameter validation for RAID LVs and
|
|||
|
|
include existing automatic setting of these parameters based
|
|||
|
|
on current shape of the RAID LV in case these are not set
|
|||
|
|
on command line fully.
|
|||
|
|
|
|||
|
|
Previously, this was done only to a certain subset given by this
|
|||
|
|
condition (where the 'stripes' is the '-i|--stripes' cmd line arg
|
|||
|
|
and the 'stripe_size' is actually the '-I|--stripesize' cmd line arg):
|
|||
|
|
|
|||
|
|
!(stripes == 1 || (stripes > 1 && stripe_size))
|
|||
|
|
|
|||
|
|
This condition is a bit harder to follow at first sight and there
|
|||
|
|
are no comments around with explanation for why this one is used,
|
|||
|
|
so let's analyze it a bit more.
|
|||
|
|
|
|||
|
|
First, let's convert this to an equivalent condition (De Morgan law)
|
|||
|
|
so it's easier to read for humans:
|
|||
|
|
|
|||
|
|
stripes != 1 && !(stripes > 1 && stripe_size)
|
|||
|
|
|
|||
|
|
Note: Both stripe and stripesize are unsigned integers, so they can't be negative.
|
|||
|
|
|
|||
|
|
Now, based on that condition, we were running the code to deduce the
|
|||
|
|
stripe/stripesize and do the checks ("the code") only if both of these
|
|||
|
|
are true:
|
|||
|
|
|
|||
|
|
- stripes is different from 1
|
|||
|
|
|
|||
|
|
- we don't have stripes > 1 and stripe_size defined at the same time
|
|||
|
|
|
|||
|
|
But this is not correct in all cases, because:
|
|||
|
|
|
|||
|
|
A) if someone uses stripes = 0, then "the code" is executed
|
|||
|
|
(correct)
|
|||
|
|
|
|||
|
|
B) if someone uses stripes = 1, then "the code" is not executed
|
|||
|
|
(wrong: we still need to be able to check the args against
|
|||
|
|
existing RAID LV stripes whether it matches)
|
|||
|
|
|
|||
|
|
- if someone uses stripes > 1, then "the code" is:
|
|||
|
|
|
|||
|
|
C) if stripe_size = 0, executed
|
|||
|
|
(correct)
|
|||
|
|
|
|||
|
|
D) if stripe_size > 0, not executed
|
|||
|
|
(wrong: we still want to check against existing RAID LV stripes)
|
|||
|
|
|
|||
|
|
Current issues with this condition:
|
|||
|
|
The B) ends up with segfault.
|
|||
|
|
|
|||
|
|
❯ lvextend -i 1 -l+1 vg/lvol0
|
|||
|
|
Rounding size 4.00 MiB (1 extents) up to stripe boundary size 8.00 MiB (2 extents).
|
|||
|
|
Segmentation fault (core dumped)
|
|||
|
|
|
|||
|
|
The D) ends up with errors like:
|
|||
|
|
|
|||
|
|
❯ lvextend -i 3 -l+1 -I128k vg/lvol0
|
|||
|
|
Rounding size 4.00 MiB (1 extents) up to stripe boundary size 8.00 MiB (2 extents).
|
|||
|
|
Rounding size (4 extents) up to stripe boundary size for segment (5 extents).
|
|||
|
|
Size of logical volume vg/lvol0 changed from 8.00 MiB (2 extents) to 20.00 MiB (5 extents).
|
|||
|
|
LV lvol0: segment 1 with len=5 has inconsistent area_len 3
|
|||
|
|
Couldn't read all logical volumes for volume group vg.
|
|||
|
|
Failed to write VG vg.
|
|||
|
|
|
|||
|
|
Conclusion:
|
|||
|
|
The condition needs to be removed so we always run "the code" to check
|
|||
|
|
given striping args given on command line against existing RAID LV
|
|||
|
|
striping. The reason is that we don't want to allow changing stripe
|
|||
|
|
count for RAID LVs through lvextend and we need to end up with the
|
|||
|
|
error:
|
|||
|
|
"Unable to extend <RAID segment type> segment type with different number of stripes"
|
|||
|
|
|
|||
|
|
(We do support changing the striping by lvconvert's reshaping functionality only).
|
|||
|
|
|
|||
|
|
(cherry picked from commit b5249fa3c20fe5d9e1d4811e7e5bfd957b15a820)
|
|||
|
|
---
|
|||
|
|
lib/metadata/lv_manip.c | 2 +-
|
|||
|
|
1 file changed, 1 insertion(+), 1 deletion(-)
|
|||
|
|
|
|||
|
|
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
|
|||
|
|
index a1d4f641a..e14947357 100644
|
|||
|
|
--- a/lib/metadata/lv_manip.c
|
|||
|
|
+++ b/lib/metadata/lv_manip.c
|
|||
|
|
@@ -5468,7 +5468,7 @@ static int _lvresize_adjust_extents(struct logical_volume *lv,
|
|||
|
|
} else if (seg_is_raid0(seg_last)) {
|
|||
|
|
lp->stripes = seg_last->area_count;
|
|||
|
|
lp->stripe_size = seg_last->stripe_size;
|
|||
|
|
- } else if (!(lp->stripes == 1 || (lp->stripes > 1 && lp->stripe_size))) {
|
|||
|
|
+ } else {
|
|||
|
|
/* If extending, find stripes, stripesize & size of last segment */
|
|||
|
|
/* FIXME Don't assume mirror seg will always be AREA_LV */
|
|||
|
|
/* FIXME We will need to support resize for metadata LV as well,
|
|||
|
|
--
|
|||
|
|
2.47.0
|
|||
|
|
|