From 8270ff5702e0fa846714ac7366aade84b3e85209 Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen Date: Fri, 28 Oct 2016 15:54:27 +0200 Subject: [PATCH] lvconvert: prevent non-synced raid1 primary leg repair (Automatic) repair may not be allowed during the initial sync of an upconverted linear LV, because the data on the failing, primary leg hasn't been completely synchronized to the N-1 other legs of the raid1 LV (replacing failed legs during repair involves discontinuing access to any replaced legs data, thus preventing data recovery on the primary leg e.g. via dd_rescue). Even though repair would not cause data loss when adding legs to a fully synced raid1 LV, we don't have information yet defining this state yet (e.g. a raid1 LV flag telling the fully synchronized status before any legs were added), hence can't automatically decide to allow to repair. If nonetheless a repair on a non-synced raid1 LVs is intended, the "--force" option has to be provided. Resolves: rhbz1311765 --- WHATS_NEW | 1 + lib/metadata/metadata-exported.h | 4 +-- lib/metadata/raid_manip.c | 60 ++++++++++++++++++++++---------- tools/lvconvert.c | 4 +-- 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 1d0d6dc70..49af38a58 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.167 - ====================================== + Prevent non-synced raid1 repair unless --force Prevent raid4 creation/conversion on non-supporting kernels Add direct striped -> raid4 conversion Fix raid4 parity image pair position on conversions from striped/raid0* diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 37db89c10..cdd4984c1 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -1208,8 +1208,8 @@ int lv_raid_convert(struct logical_volume *lv, const uint32_t new_region_size, struct dm_list *allocate_pvs); int lv_raid_rebuild(struct logical_volume *lv, struct dm_list *rebuild_pvs); -int lv_raid_replace(struct logical_volume *lv, struct dm_list *remove_pvs, - struct dm_list *allocate_pvs); +int lv_raid_replace(struct logical_volume *lv, int force, + struct dm_list *remove_pvs, struct dm_list *allocate_pvs); int lv_raid_remove_missing(struct logical_volume *lv); int partial_raid_lv_supports_degraded_activation(const struct logical_volume *lv); /* -- metadata/raid_manip.c */ diff --git a/lib/metadata/raid_manip.c b/lib/metadata/raid_manip.c index 25547c463..d15baf611 100644 --- a/lib/metadata/raid_manip.c +++ b/lib/metadata/raid_manip.c @@ -266,19 +266,16 @@ static int _deactivate_and_remove_lvs(struct volume_group *vg, struct dm_list *r * * Returns: 1 if in-sync, 0 otherwise. */ +#define _RAID_IN_SYNC_RETRIES 6 static int _raid_in_sync(struct logical_volume *lv) { + int retries = _RAID_IN_SYNC_RETRIES; dm_percent_t sync_percent; if (seg_is_striped(first_seg(lv))) return 1; - if (!lv_raid_percent(lv, &sync_percent)) { - log_error("Unable to determine sync status of %s/%s.", - lv->vg->name, lv->name); - return 0; - } - if (sync_percent == DM_PERCENT_0) { + do { /* * FIXME We repeat the status read here to workaround an * unresolved kernel bug when we see 0 even though the @@ -290,14 +287,31 @@ static int _raid_in_sync(struct logical_volume *lv) lv->vg->name, lv->name); return 0; } - if (sync_percent == DM_PERCENT_100) + if (sync_percent > DM_PERCENT_0) + break; + if (retries == _RAID_IN_SYNC_RETRIES) log_warn("WARNING: Sync status for %s is inconsistent.", display_lvname(lv)); - } + usleep(500000); + } while (--retries); return (sync_percent == DM_PERCENT_100) ? 1 : 0; } +/* Check if RaidLV @lv is synced or any raid legs of @lv are not synced */ +static int _raid_devs_sync_healthy(struct logical_volume *lv) +{ + char *raid_health; + + if (!_raid_in_sync(lv)) + return 0; + + if (!lv_raid_dev_health(lv, &raid_health)) + return_0; + + return (strchr(raid_health, 'a') || strchr(raid_health, 'D')) ? 0 : 1; +} + /* * _raid_remove_top_layer * @lv @@ -1054,6 +1068,7 @@ static int _extract_image_components(struct lv_segment *seg, uint32_t idx, /* * _raid_extract_images * @lv + * @force: force a replacement in case of primary mirror leg * @new_count: The absolute count of images (e.g. '2' for a 2-way mirror) * @target_pvs: The list of PVs that are candidates for removal * @shift: If set, use _shift_and_rename_image_components(). @@ -1068,7 +1083,8 @@ static int _extract_image_components(struct lv_segment *seg, uint32_t idx, * * Returns: 1 on success, 0 on failure */ -static int _raid_extract_images(struct logical_volume *lv, uint32_t new_count, +static int _raid_extract_images(struct logical_volume *lv, + int force, uint32_t new_count, struct dm_list *target_pvs, int shift, struct dm_list *extracted_meta_lvs, struct dm_list *extracted_data_lvs) @@ -1136,11 +1152,16 @@ static int _raid_extract_images(struct logical_volume *lv, uint32_t new_count, !lv_is_on_pvs(seg_metalv(seg, s), target_pvs)) continue; - if (!_raid_in_sync(lv) && - (!seg_is_mirrored(seg) || (s == 0))) { + /* + * Kernel may report raid LV in-sync but still + * image devices may not be in-sync or faulty. + */ + if (!_raid_devs_sync_healthy(lv) && + (!seg_is_mirrored(seg) || (s == 0 && !force))) { log_error("Unable to extract %sRAID image" - " while RAID array is not in-sync", - seg_is_mirrored(seg) ? "primary " : ""); + " while RAID array is not in-sync%s", + seg_is_mirrored(seg) ? "primary " : "", + seg_is_mirrored(seg) ? " (use --force option to replace)" : ""); return 0; } } @@ -1185,7 +1206,7 @@ static int _raid_remove_images(struct logical_volume *lv, if (!removal_lvs) removal_lvs = &removed_lvs; - if (!_raid_extract_images(lv, new_count, allocate_pvs, 1, + if (!_raid_extract_images(lv, 0, new_count, allocate_pvs, 1, removal_lvs, removal_lvs)) { log_error("Failed to extract images from %s/%s", lv->vg->name, lv->name); @@ -1375,7 +1396,7 @@ int lv_raid_split(struct logical_volume *lv, const char *split_name, return_0; } - if (!_raid_extract_images(lv, new_count, splittable_pvs, 1, + if (!_raid_extract_images(lv, 0, new_count, splittable_pvs, 1, &removal_lvs, &data_list)) { log_error("Failed to extract images from %s/%s", lv->vg->name, lv->name); @@ -3869,6 +3890,7 @@ has_enough_space: * new SubLVS are allocated on PVs on list @allocate_pvs. */ static int _lv_raid_rebuild_or_replace(struct logical_volume *lv, + int force, struct dm_list *remove_pvs, struct dm_list *allocate_pvs, int rebuild) @@ -4043,7 +4065,8 @@ try_again: * supplied - knowing that only the image with the error target * will be affected. */ - if (!_raid_extract_images(lv, raid_seg->area_count - match_count, + if (!_raid_extract_images(lv, force, + raid_seg->area_count - match_count, partial_segment_removed ? &lv->vg->pvs : remove_pvs, 0, &old_lvs, &old_lvs)) { @@ -4148,7 +4171,7 @@ skip_alloc: int lv_raid_rebuild(struct logical_volume *lv, struct dm_list *rebuild_pvs) { - return _lv_raid_rebuild_or_replace(lv, rebuild_pvs, NULL, 1); + return _lv_raid_rebuild_or_replace(lv, 0, rebuild_pvs, NULL, 1); } /* @@ -4161,10 +4184,11 @@ int lv_raid_rebuild(struct logical_volume *lv, * allocating new SubLVs from PVs on list @allocate_pvs. */ int lv_raid_replace(struct logical_volume *lv, + int force, struct dm_list *remove_pvs, struct dm_list *allocate_pvs) { - return _lv_raid_rebuild_or_replace(lv, remove_pvs, allocate_pvs, 0); + return _lv_raid_rebuild_or_replace(lv, force, remove_pvs, allocate_pvs, 0); } int lv_raid_remove_missing(struct logical_volume *lv) diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 23b5906a5..8376565c1 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -1994,7 +1994,7 @@ static int _lvconvert_raid(struct logical_volume *lv, struct lvconvert_params *l } if (lp->replace) - return lv_raid_replace(lv, lp->replace_pvh, lp->pvh); + return lv_raid_replace(lv, lp->force, lp->replace_pvh, lp->pvh); if (lp->repair) { if (!lv_is_active_exclusive_locally(lv_lock_holder(lv))) { @@ -2017,7 +2017,7 @@ static int _lvconvert_raid(struct logical_volume *lv, struct lvconvert_params *l if (!(failed_pvs = _failed_pv_list(lv->vg))) return_0; - if (!lv_raid_replace(lv, failed_pvs, lp->pvh)) { + if (!lv_raid_replace(lv, lp->force, failed_pvs, lp->pvh)) { log_error("Failed to replace faulty devices in %s.", display_lvname(lv)); return 0;