commit d40cd5403abe4c8297a2b0de10bf2f7481c5489e Author: Peter Rajnoha Date: Tue Mar 24 13:57:04 2015 +0100 0 --- WHATS_NEW | 4 ++++ lib/format_text/flags.c | 1 + lib/metadata/lv_manip.c | 4 +++- lib/metadata/metadata-exported.h | 13 ++++++++++++- lib/metadata/metadata.c | 14 ++++++++++++++ lib/metadata/vg.c | 1 + lib/metadata/vg.h | 5 +++++ test/shell/select-tools-thin.sh | 35 +++++++++++++++++++++++++++++++++++ tools/toollib.c | 28 ++++++++++++++++++++++++---- 9 files changed, 99 insertions(+), 6 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 558bdb5..4010e67 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,3 +1,7 @@ +Version 2.02.119 - +================================== + Fix LV processing with selection to always do the selection on initial state. + Version 2.02.118 - 23rd March 2015 ================================== Store metadata size + checksum in lvmcache and add struct lvmcache_vgsummary. diff --git a/lib/format_text/flags.c b/lib/format_text/flags.c index a975606..bbd47c3 100644 --- a/lib/format_text/flags.c +++ b/lib/format_text/flags.c @@ -92,6 +92,7 @@ static const struct flag _lv_flags[] = { {CACHE_POOL_DATA, NULL, 0}, {CACHE_POOL_METADATA, NULL, 0}, {LV_PENDING_DELETE, NULL, 0}, /* FIXME Display like COMPATIBLE_FLAG */ + {LV_REMOVED, NULL, 0}, {0, NULL, 0} }; diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index c9c1145..99fb91f 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -5338,6 +5338,7 @@ int link_lv_to_vg(struct volume_group *vg, struct logical_volume *lv) lvl->lv = lv; lv->vg = vg; dm_list_add(&vg->lvs, &lvl->list); + lv->status &= ~LV_REMOVED; return 1; } @@ -5349,7 +5350,8 @@ int unlink_lv_from_vg(struct logical_volume *lv) if (!(lvl = find_lv_in_vg(lv->vg, lv->name))) return_0; - dm_list_del(&lvl->list); + dm_list_move(&lv->vg->removed_lvs, &lvl->list); + lv->status |= LV_REMOVED; return 1; } diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 1555c87..47fb9ae 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -119,11 +119,20 @@ #define CACHE UINT64_C(0x0001000000000000) /* LV - Internal use only */ #define LV_PENDING_DELETE UINT64_C(0x0004000000000000) /* LV - Internal use only */ +#define LV_REMOVED UINT64_C(0x0040000000000000) /* LV - Internal use only + This flag is used to mark an LV once it has + been removed from the VG. It might still + be referenced on internal lists of LVs. + Any remaining references should check for + this flag and ignore the LV is set. + FIXME: Remove this flag once we have indexed + vg->removed_lvs for quick lookup. + */ #define LV_ERROR_WHEN_FULL UINT64_C(0x0008000000000000) /* LV - error when full */ #define PV_ALLOCATION_PROHIBITED UINT64_C(0x0010000000000000) /* PV - internal use only - allocation prohibited e.g. to prohibit allocation of a RAID image on a PV already holing an image of the RAID set */ -/* Next unused flag: UINT64_C(0x0040000000000000) */ +/* Next unused flag: UINT64_C(0x0080000000000000) */ /* Format features flags */ #define FMT_SEGMENTS 0x00000001U /* Arbitrary segment params? */ @@ -222,6 +231,8 @@ #define lv_is_rlog(lv) (((lv)->status & REPLICATOR_LOG) ? 1 : 0) +#define lv_is_removed(lv) (((lv)->status & LV_REMOVED) ? 1 : 0) + int lv_layout_and_role(struct dm_pool *mem, const struct logical_volume *lv, struct dm_list **layout, struct dm_list **role); diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 33ce370..a1a31eb 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -2568,12 +2568,26 @@ int vg_validate(struct volume_group *vg) r = 0; } + dm_list_iterate_items(lvl, &vg->removed_lvs) { + if (!(lvl->lv->status & LV_REMOVED)) { + log_error(INTERNAL_ERROR "LV %s is not marked as removed while it's part " + "of removed LV list for VG %s", lvl->lv->name, vg->name); + r = 0; + } + } + /* * Count all non-snapshot invisible LVs */ dm_list_iterate_items(lvl, &vg->lvs) { lv_count++; + if (lvl->lv->status & LV_REMOVED) { + log_error(INTERNAL_ERROR "LV %s is marked as removed while it's " + "still part of the VG %s", lvl->lv->name, vg->name); + r = 0; + } + if (lvl->lv->status & LVM_WRITE_LOCKED) { log_error(INTERNAL_ERROR "LV %s has external flag LVM_WRITE_LOCKED set internally.", lvl->lv->name); diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c index 404cc6f..c9a7e9e 100644 --- a/lib/metadata/vg.c +++ b/lib/metadata/vg.c @@ -63,6 +63,7 @@ struct volume_group *alloc_vg(const char *pool_name, struct cmd_context *cmd, dm_list_init(&vg->pvs_to_create); dm_list_init(&vg->lvs); dm_list_init(&vg->tags); + dm_list_init(&vg->removed_lvs); dm_list_init(&vg->removed_pvs); log_debug_mem("Allocated VG %s at %p.", vg->name, vg); diff --git a/lib/metadata/vg.h b/lib/metadata/vg.h index 67a04a0..b0ab122 100644 --- a/lib/metadata/vg.h +++ b/lib/metadata/vg.h @@ -113,6 +113,11 @@ struct volume_group { */ /* + * List of removed logical volumes by _lv_reduce. + */ + struct dm_list removed_lvs; + + /* * List of removed physical volumes by pvreduce. * They have to get cleared on vg_commit. */ diff --git a/test/shell/select-tools-thin.sh b/test/shell/select-tools-thin.sh new file mode 100644 index 0000000..d70289f --- /dev/null +++ b/test/shell/select-tools-thin.sh @@ -0,0 +1,35 @@ +#!/bin/sh +# Copyright (C) 2015 Red Hat, Inc. All rights reserved. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions +# of the GNU General Public License v.2. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +. lib/inittest + +aux have_thin 1 0 0 || skip + +aux prepare_pvs 1 16 + +######################### +# special cases to test # +######################### + +# if calling lvremove and an LV is removed that is related to other LV +# and we're doing selection based on this relation, check if we're +# selecting on initial state (here, thin origin LV thin_orig is removed +# first, but thin snap should be still selectable based on origin=thin_orig +# condition even though thin_orig has just been removed) +vgcreate -s 4m $vg1 $dev1 +lvcreate -l100%FREE -T $vg1/pool +lvcreate -V4m -T $vg1/pool -n thin_orig +lvcreate -s $vg1/thin_orig -n thin_snap +lvremove -ff -S 'lv_name=thin_orig || origin=thin_orig' > out +grep "Logical volume \"thin_orig\" successfully removed" out +grep "Logical volume \"thin_snap\" successfully removed" out +not lvs $vg1/thin_orig +not lvs $vg1/thin_snap diff --git a/tools/toollib.c b/tools/toollib.c index 4bd2339..142ff33 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -1957,6 +1957,10 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg, unsigned lvargs_supplied = 0; struct lv_list *lvl; struct dm_str_list *sl; + struct dm_list final_lvs; + struct lv_list *final_lvl; + + dm_list_init(&final_lvs); if (!vg_check_status(vg, EXPORTED_VG)) { ret_max = ECMD_FAILED; @@ -1986,10 +1990,6 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg, (tags_supplied && str_list_match_list(tags_in, &vg->tags, NULL))) process_all = 1; - /* - * FIXME: In case of remove it goes through deleted entries, - * but it works since entries are allocated from vg mem pool. - */ dm_list_iterate_items(lvl, &vg->lvs) { if (sigint_caught()) { ret_max = ECMD_FAILED; @@ -2049,6 +2049,26 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg, if (!process_lv) continue; + log_very_verbose("Adding %s/%s to the list of LVs to be processed.", vg->name, lvl->lv->name); + + if (!(final_lvl = dm_pool_zalloc(vg->vgmem, sizeof(struct lv_list)))) { + log_error("Failed to allocate final LV list item."); + ret_max = ECMD_FAILED; + goto_out; + } + final_lvl->lv = lvl->lv; + dm_list_add(&final_lvs, &final_lvl->list); + } + + dm_list_iterate_items(lvl, &final_lvs) { + /* + * FIXME: Once we have index over vg->removed_lvs, check directly + * LV presence there and remove LV_REMOVE flag/lv_is_removed fn + * as they won't be needed anymore. + */ + if (lv_is_removed(lvl->lv)) + continue; + log_very_verbose("Processing LV %s in VG %s.", lvl->lv->name, vg->name); ret = process_single_lv(cmd, lvl->lv, handle);