From f2e8b49e1dd70f3735ff556f3f7296078a057a5e Mon Sep 17 00:00:00 2001 From: Peter Rajnoha 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 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