master - tests: enable writecache test that uses cleaner
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=ff3945777ba6cb09317...
Commit: ff3945777ba6cb093174b99a9870df2494f4c29b
Parent: c32d7fed4f78b4537f4ada4318866786a9fab493
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Jun 22 11:52:14 2020 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Thu Oct 1 11:33:02 2020 -0500
tests: enable writecache test that uses cleaner
---
test/shell/writecache.sh | 48 ++++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/test/shell/writecache.sh b/test/shell/writecache.sh
index 99be31e6b..e549b113c 100644
--- a/test/shell/writecache.sh
+++ b/test/shell/writecache.sh
@@ -236,34 +236,30 @@ _verify_data_on_lv
lvremove $vg/$lv1
lvremove $vg/$lv2
-# FIXME: test depends on unpushed commit
-# that enables two stage flush using cleaner
-#
# Test attach while active, detach while active,
# skip cleaner so flush message is used instead
-#
-# lvcreate -n $lv1 -l 8 -an $vg "$dev1"
-# lvcreate -n $lv2 -l 4 -an $vg "$dev2"
-# lvchange -ay $vg/$lv1
-# _add_new_data_to_mnt
-# lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
-# blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
-# blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
-# _add_more_data_to_mnt
-# _verify_data_on_mnt
-# lvconvert --splitcache --cachesettings cleaner=0 $vg/$lv1
-# check lv_field $vg/$lv1 segtype linear
-# check lv_field $vg/$lv2 segtype linear
-# blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
-# blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
-# _verify_data_on_mnt
-# _verify_more_data_on_mnt
-# umount $mnt
-# lvchange -an $vg/$lv1
-# lvchange -an $vg/$lv2
-# _verify_data_on_lv
-# lvremove $vg/$lv1
-# lvremove $vg/$lv2
+lvcreate -n $lv1 -l 8 -an $vg "$dev1"
+lvcreate -n $lv2 -l 4 -an $vg "$dev2"
+lvchange -ay $vg/$lv1
+_add_new_data_to_mnt
+lvconvert --yes --type writecache --cachevol $lv2 $vg/$lv1
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_add_more_data_to_mnt
+_verify_data_on_mnt
+lvconvert --splitcache --cachesettings cleaner=0 $vg/$lv1
+check lv_field $vg/$lv1 segtype linear
+check lv_field $vg/$lv2 segtype linear
+blockdev --getss "$DM_DEV_DIR/$vg/$lv1"
+blockdev --getpbsz "$DM_DEV_DIR/$vg/$lv1"
+_verify_data_on_mnt
+_verify_more_data_on_mnt
+umount $mnt
+lvchange -an $vg/$lv1
+lvchange -an $vg/$lv2
+_verify_data_on_lv
+lvremove $vg/$lv1
+lvremove $vg/$lv2
vgremove -ff $vg
2 years, 4 months
master - writecache: use two step detach
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=c32d7fed4f78b4537f4...
Commit: c32d7fed4f78b4537f4ada4318866786a9fab493
Parent: d1b7438c9fb7ab9b0940ce433c0ece2fa17a6f03
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Thu Jun 11 13:33:40 2020 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Thu Oct 1 11:33:02 2020 -0500
writecache: use two step detach
When detaching a writecache, use the cleaner setting
by default to writeback data prior to suspending the
lv to detach the writecache. This avoids potentially
blocking for a long period with the device suspended.
Detaching a writecache first sets the cleaner option, waits
for a short period of time (less than a second), and checks
if the writecache has quickly become clean. If so, the
writecache is detached immediately. This optimizes the case
where little writeback is needed.
If the writecache does not quickly become clean, then the
detach command leaves the writecache attached with the
cleaner option set. This leaves the LV in the same state
as if the user had set the cleaner option directly with
lvchange --cachesettings cleaner=1 LV.
After leaving the LV with the cleaner option set, the
detach command will wait and watch the writeback progress,
and will finally detach the writecache when the writeback
is finished. The detach command does not need to wait
during the writeback phase, and can be canceled, in which
case the LV will remain with the writecache attached and
the cleaner option set. When the user runs the detach
command again it will complete the detach.
To detach a writecache directly, without using the cleaner
step (which has been the approach previously), add the
option --cachesettings cleaner=0 to the detach command.
---
lib/metadata/metadata-exported.h | 3 +
lib/metadata/writecache_manip.c | 63 ++++++++-
lib/writecache/writecache.c | 25 +++-
tools/command-lines.in | 4 +-
tools/lvconvert.c | 285 +++++++++++++++++++++++++++++++++++++--
5 files changed, 357 insertions(+), 23 deletions(-)
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index c4d51998b..37fe9d0ad 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -1114,6 +1114,9 @@ int lv_is_cache_origin(const struct logical_volume *lv);
int lv_is_writecache_origin(const struct logical_volume *lv);
int lv_is_writecache_cachevol(const struct logical_volume *lv);
int writecache_settings_to_str_list(struct writecache_settings *settings, struct dm_list *result, struct dm_pool *mem);
+int lv_writecache_set_cleaner(struct logical_volume *lv);
+bool lv_writecache_is_clean(struct cmd_context *cmd, struct logical_volume *lv, uint64_t *dirty_blocks);
+bool writecache_cleaner_supported(struct cmd_context *cmd);
int lv_is_integrity_origin(const struct logical_volume *lv);
diff --git a/lib/metadata/writecache_manip.c b/lib/metadata/writecache_manip.c
index 0230d6160..5004aa989 100644
--- a/lib/metadata/writecache_manip.c
+++ b/lib/metadata/writecache_manip.c
@@ -58,9 +58,9 @@ int lv_is_writecache_cachevol(const struct logical_volume *lv)
return 0;
}
-static int _get_writecache_kernel_error(struct cmd_context *cmd,
- const struct logical_volume *lv,
- uint32_t *kernel_error)
+static int _get_writecache_kernel_status(struct cmd_context *cmd,
+ struct logical_volume *lv,
+ struct dm_status_writecache *status_out)
{
struct lv_with_info_and_seg_status status;
@@ -91,7 +91,10 @@ static int _get_writecache_kernel_error(struct cmd_context *cmd,
goto fail;
}
- *kernel_error = status.seg_status.writecache->error;
+ status_out->error = status.seg_status.writecache->error;
+ status_out->total_blocks = status.seg_status.writecache->total_blocks;
+ status_out->free_blocks = status.seg_status.writecache->free_blocks;
+ status_out->writeback_blocks = status.seg_status.writecache->writeback_blocks;
dm_pool_destroy(status.seg_status.mem);
return 1;
@@ -101,6 +104,35 @@ fail:
return 0;
}
+static int _get_writecache_kernel_error(struct cmd_context *cmd,
+ struct logical_volume *lv,
+ uint32_t *kernel_error)
+{
+ struct dm_status_writecache status = { 0 };
+
+ if (!_get_writecache_kernel_status(cmd, lv, &status))
+ return_0;
+
+ *kernel_error = status.error;
+ return 1;
+}
+
+bool lv_writecache_is_clean(struct cmd_context *cmd, struct logical_volume *lv, uint64_t *dirty_blocks)
+{
+ struct dm_status_writecache status = { 0 };
+
+ if (!_get_writecache_kernel_status(cmd, lv, &status))
+ return false;
+
+ if (dirty_blocks)
+ *dirty_blocks = status.total_blocks - status.free_blocks;
+
+ if (status.total_blocks == status.free_blocks)
+ return true;
+
+ return false;
+}
+
static void _rename_detached_cvol(struct cmd_context *cmd, struct logical_volume *lv_fast)
{
struct volume_group *vg = lv_fast->vg;
@@ -319,7 +351,7 @@ static int _lv_detach_writecache_cachevol_active(struct logical_volume *lv, int
log_debug("Checking writecache errors to detach.");
- if (!_get_writecache_kernel_error(cmd, lv_old, &kernel_error)) {
+ if (!_get_writecache_kernel_error(cmd, (struct logical_volume *)lv_old, &kernel_error)) {
log_error("Failed to get writecache error status for %s.", display_lvname(lv_old));
return 0;
}
@@ -394,6 +426,27 @@ int lv_detach_writecache_cachevol(struct logical_volume *lv, int noflush)
return _lv_detach_writecache_cachevol_inactive(lv, noflush);
}
+int lv_writecache_set_cleaner(struct logical_volume *lv)
+{
+ struct lv_segment *seg = first_seg(lv);
+
+ seg->writecache_settings.cleaner = 1;
+ seg->writecache_settings.cleaner_set = 1;
+
+ if (lv_is_active(lv)) {
+ if (!lv_update_and_reload(lv)) {
+ log_error("Failed to update VG and reload LV.");
+ return 0;
+ }
+ } else {
+ if (!vg_write(lv->vg) || !vg_commit(lv->vg)) {
+ log_error("Failed to update VG.");
+ return 0;
+ }
+ }
+ return 1;
+}
+
static int _writecache_setting_str_list_add(const char *field, uint64_t val, char *val_str, struct dm_list *result, struct dm_pool *mem)
{
char buf[128];
diff --git a/lib/writecache/writecache.c b/lib/writecache/writecache.c
index c7aea286d..4ecbf50df 100644
--- a/lib/writecache/writecache.c
+++ b/lib/writecache/writecache.c
@@ -238,13 +238,20 @@ static int _target_present(struct cmd_context *cmd,
if (!_writecache_checked) {
_writecache_checked = 1;
- _writecache_present = target_present(cmd, TARGET_NAME_WRITECACHE, 1);
+ _writecache_present = target_present(cmd, TARGET_NAME_WRITECACHE, 1);
- if (!target_version(TARGET_NAME_WRITECACHE, &maj, &min, &patchlevel))
+ if (!_writecache_present) {
+ log_error("dm-writecache module not found in kernel.");
+ return 0;
+ }
+
+ if (!target_version(TARGET_NAME_WRITECACHE, &maj, &min, &patchlevel)) {
+ log_error("dm-writecache module version not found.");
return_0;
+ }
if (maj < 1) {
- log_error("writecache target version older than minimum 1.0.0");
+ log_error("dm-writecache module version older than minimum 1.0.0");
return 0;
}
@@ -257,6 +264,12 @@ static int _target_present(struct cmd_context *cmd,
return _writecache_present;
}
+bool writecache_cleaner_supported(struct cmd_context *cmd)
+{
+ _target_present(cmd, NULL, NULL);
+ return _writecache_cleaner_supported ? true : false;
+}
+
static int _modules_needed(struct dm_pool *mem,
const struct lv_segment *seg __attribute__((unused)),
struct dm_list *modules)
@@ -268,6 +281,12 @@ static int _modules_needed(struct dm_pool *mem,
return 1;
}
+
+#else
+bool writecache_cleaner_supported(struct cmd_context *cmd)
+{
+ return 0;
+}
#endif /* DEVMAPPER_SUPPORT */
#ifdef DEVMAPPER_SUPPORT
diff --git a/tools/command-lines.in b/tools/command-lines.in
index 1b0ca2227..5a8523966 100644
--- a/tools/command-lines.in
+++ b/tools/command-lines.in
@@ -620,14 +620,14 @@ FLAGS: SECONDARY_SYNTAX
---
lvconvert --splitcache LV_cachepool_cache_thinpool_vdopool_writecache
-OO: OO_LVCONVERT
+OO: OO_LVCONVERT, --cachesettings String
ID: lvconvert_split_and_keep_cache
DESC: Detach a cache from an LV.
---
lvconvert --uncache LV_cache_thinpool_vdopool_writecache
-OO: OO_LVCONVERT
+OO: OO_LVCONVERT, --cachesettings String
ID: lvconvert_split_and_remove_cache
DESC: Detach and delete a cache from an LV.
FLAGS: SECONDARY_SYNTAX
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index 60c0fca21..a4ec48ede 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -3659,7 +3659,9 @@ static struct convert_poll_id_list* _convert_poll_id_list_create(struct cmd_cont
* Data/results accumulated during processing.
*/
struct lvconvert_result {
- int need_polling;
+ unsigned need_polling:1;
+ unsigned wait_cleaner_writecache:1;
+ unsigned active_begin:1;
struct dm_list poll_idls;
};
@@ -4905,9 +4907,11 @@ int lvconvert_merge_thin_cmd(struct cmd_context *cmd, int argc, char **argv)
NULL, NULL, &_lvconvert_merge_thin_single);
}
-static int _lvconvert_detach_writecache(struct cmd_context *cmd,
+static int _lvconvert_detach_writecache(struct cmd_context *cmd, struct processing_handle *handle,
struct logical_volume *lv,
struct logical_volume *lv_fast);
+static int _lvconvert_detach_writecache_when_clean(struct cmd_context *cmd,
+ struct lvconvert_result *lr);
static int _lvconvert_split_cache_single(struct cmd_context *cmd,
struct logical_volume *lv,
@@ -4958,7 +4962,7 @@ static int _lvconvert_split_cache_single(struct cmd_context *cmd,
return ECMD_FAILED;
if (lv_is_writecache(lv_main)) {
- if (!_lvconvert_detach_writecache(cmd, lv_main, lv_fast))
+ if (!_lvconvert_detach_writecache(cmd, handle, lv_main, lv_fast))
return ECMD_FAILED;
if (cmd->command->command_enum == lvconvert_split_and_remove_cache_CMD) {
@@ -5008,11 +5012,33 @@ static int _lvconvert_split_cache_single(struct cmd_context *cmd,
int lvconvert_split_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
{
+ struct processing_handle *handle;
+ struct lvconvert_result lr = { 0 };
+ int ret;
+
cmd->handles_missing_pvs = 1;
cmd->partial_activation = 1;
- return process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL, READ_FOR_UPDATE,
- NULL, NULL, &_lvconvert_split_cache_single);
+ if (!(handle = init_processing_handle(cmd, NULL))) {
+ log_error("Failed to initialize processing handle.");
+ return ECMD_FAILED;
+ }
+
+ handle->custom_handle = &lr;
+
+ ret = process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL, READ_FOR_UPDATE,
+ handle, NULL, &_lvconvert_split_cache_single);
+
+ destroy_processing_handle(cmd, handle);
+
+ if (ret == ECMD_FAILED)
+ return ret;
+
+ if (lr.wait_cleaner_writecache)
+ if (!_lvconvert_detach_writecache_when_clean(cmd, &lr))
+ ret = ECMD_FAILED;
+
+ return ret;
}
static int _lvconvert_raid_types_single(struct cmd_context *cmd, struct logical_volume *lv,
@@ -5478,12 +5504,37 @@ int lvconvert_to_vdopool_param_cmd(struct cmd_context *cmd, int argc, char **arg
NULL, NULL, &_lvconvert_to_vdopool_single);
}
+/*
+ * Starts the detach process, and may complete it, or may defer the completion
+ * if cleaning is required, by returning a poll id. If deferred, the caller
+ * will notice the poll id and call lvconvert_detach_writecache_when_clean
+ * to wait for the cleaning and complete the detach. The command can be cancelled
+ * while waiting for cleaning and the same command be repeated to continue the
+ * process.
+ */
static int _lvconvert_detach_writecache(struct cmd_context *cmd,
+ struct processing_handle *handle,
struct logical_volume *lv,
struct logical_volume *lv_fast)
{
+ struct lvconvert_result *lr = (struct lvconvert_result *) handle->custom_handle;
+ struct writecache_settings settings;
+ struct convert_poll_id_list *idl;
+ uint32_t block_size_sectors;
+ int active_begin = 0;
+ int active_clean = 0;
+ int is_clean = 0;
int noflush = 0;
+ dm_list_init(&lr->poll_idls);
+
+ memset(&settings, 0, sizeof(settings));
+
+ if (!get_writecache_settings(cmd, &settings, &block_size_sectors)) {
+ log_error("Invalid writecache settings.");
+ return 0;
+ }
+
if (!archive(lv->vg))
return_0;
@@ -5508,15 +5559,99 @@ static int _lvconvert_detach_writecache(struct cmd_context *cmd,
}
/*
- * TODO: send a message to writecache in the kernel to start writing
- * back cache data to the origin. Then release the vg lock and monitor
- * the progress of that writeback. When it's complete we can reacquire
- * the vg lock, rescan the vg (ensure it hasn't changed), and do the
- * detach which should be quick since the writeback is complete. If
- * this command is canceled while monitoring writeback, it should just
- * be rerun. The LV will continue to have the writecache until this
- * command is run to completion.
+ * If the LV is inactive when we begin, then we want to
+ * deactivate the LV at the end.
+ */
+ active_begin = lv_is_active(lv);
+
+ if (!noflush) {
+ /*
+ * --cachesettings cleaner=0 means to skip the use of the cleaner
+ * and go directly to detach which will use a flush message.
+ * (This is currently the only cachesetting used during detach.)
+ */
+ if (settings.cleaner_set && !settings.cleaner) {
+ log_print_unless_silent("Detaching writecache skipping cleaner...");
+ goto detach;
+ }
+
+ if (!writecache_cleaner_supported(cmd)) {
+ log_print_unless_silent("Detaching writecache without cleaner...");
+ goto detach;
+ }
+
+ if (!active_begin && !activate_lv(cmd, lv)) {
+ log_error("Failed to activate LV to clean writecache.");
+ return 0;
+ }
+ active_clean = 1;
+
+ /*
+ * If the user ran this command previously (or set cleaner
+ * directly) the cache may already be empty and ready for
+ * detach.
+ */
+ if (lv_writecache_is_clean(cmd, lv, NULL)) {
+ log_print_unless_silent("Detaching writecache already clean.");
+ is_clean = 1;
+ goto detach;
+ }
+
+ /*
+ * If the user has not already done lvchange --cachesettings cleaner=1
+ * then do that here. If the LV is inactive, this activates it
+ * so that cache writeback can be done.
+ */
+ log_print_unless_silent("Detaching writecache setting cleaner.");
+
+ if (!lv_writecache_set_cleaner(lv)) {
+ log_error("Failed to set cleaner cachesetting to flush cache.");
+ log_error("See lvchange --cachesettings cleaner=1");
+
+ if (!active_begin && active_clean && !deactivate_lv(cmd, lv))
+ stack;
+ return 0;
+ }
+
+ /*
+ * The cache may have been nearly clean and will be empty with
+ * a short dely.
+ */
+ usleep(10000);
+ if (lv_writecache_is_clean(cmd, lv, NULL)) {
+ log_print_unless_silent("Detaching writecache finished cleaning.");
+ is_clean = 1;
+ goto detach;
+ }
+
+ if (!(idl = _convert_poll_id_list_create(cmd, lv))) {
+ log_error("Failed to monitor writecache cleaner progress.");
+ return 0;
+ }
+
+ /*
+ * Monitor the writecache status until the cache is unused.
+ * This is done at the end of the command where locks are not
+ * held since the writeback can take some time.
+ */
+ lr->wait_cleaner_writecache = 1;
+ lr->active_begin = active_begin;
+
+ dm_list_add(&lr->poll_idls, &idl->list);
+ return 1;
+ }
+
+ detach:
+
+ /*
+ * If the LV was inactive before cleaning and activated to do cleaning,
+ * then deactivate before the detach.
*/
+ if (!active_begin && active_clean && !deactivate_lv(cmd, lv))
+ stack;
+
+ if (is_clean)
+ noflush = 1;
if (!lv_detach_writecache_cachevol(lv, noflush))
return_0;
@@ -5528,6 +5663,128 @@ static int _lvconvert_detach_writecache(struct cmd_context *cmd,
return 1;
}
+/*
+ * _lvconvert_detach_writecache() set the cleaner option for the LV
+ * so writecache will begin writing back data from cache to origin.
+ * It then saved the LV name/id (lvconvert_result/poll_id), and
+ * exited process_each_lv (releasing the VG and VG lock). Then
+ * this is called to monitor the progress of the cache writeback.
+ * When the cache is clean, this does the detach (writecache is removed
+ * in metadata and LV in kernel is updated.)
+ */
+static int _lvconvert_detach_writecache_when_clean(struct cmd_context *cmd,
+ struct lvconvert_result *lr)
+{
+ struct convert_poll_id_list *idl;
+ struct poll_operation_id *id;
+ struct volume_group *vg;
+ struct logical_volume *lv;
+ uint32_t lockd_state, error_flags;
+ uint64_t dirty;
+ int ret;
+
+ idl = dm_list_item(dm_list_first(&lr->poll_idls), struct convert_poll_id_list);
+ id = idl->id;
+
+ /*
+ * TODO: we should be able to save info about the dm device for this LV
+ * and monitor the dm device status without doing vg lock/read around
+ * each check. The vg lock/read/write would then happen only once when
+ * status was finished and we want to finish the detach. If the dm
+ * device goes away while monitoring, it's no different and no worse
+ * than the LV going away here.
+ */
+
+ retry:
+ lockd_state = 0;
+ error_flags = 0;
+
+ if (!lockd_vg(cmd, id->vg_name, "ex", 0, &lockd_state)) {
+ log_error("Detaching writecache interrupted - locking VG failed.");
+ return 0;
+ }
+
+ vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state, &error_flags, NULL);
+
+ if (!vg) {
+ log_error("Detaching writecache interrupted - reading VG failed.");
+ ret = 0;
+ goto out_lockd;
+ }
+
+ if (error_flags) {
+ log_error("Detaching writecache interrupted - reading VG error %x.", error_flags);
+ ret = 0;
+ goto out_release;
+ }
+
+ lv = find_lv(vg, id->lv_name);
+
+ if (lv && id->uuid && strcmp(id->uuid, (char *)&lv->lvid))
+ lv = NULL;
+
+ if (!lv) {
+ log_error("Detaching writecache interrupted - LV not found.");
+ ret = 0;
+ goto out_release;
+ }
+
+ if (!lv_is_active(lv)) {
+ log_error("Detaching writecache interrupted - LV not active.");
+ ret = 0;
+ goto out_release;
+ }
+
+ if (!lv_writecache_is_clean(cmd, lv, &dirty)) {
+ unlock_and_release_vg(cmd, vg, vg->name);
+
+ if (!lockd_vg(cmd, id->vg_name, "un", 0, &lockd_state))
+ stack;
+
+ log_print_unless_silent("Detaching writecache cleaning %llu blocks", (unsigned long long)dirty);
+ log_print_unless_silent("This command can be cancelled and rerun to complete writecache detach.");
+ sleep(5);
+ goto retry;
+ }
+
+ if (!lr->active_begin) {
+ /*
+ * The LV was not active to begin so we should leave it inactive at the end.
+ * It will remain inactive during detach since it's clean and doesn't need
+ * a flush message.
+ */
+ if (!deactivate_lv(cmd, lv))
+ stack;
+ }
+
+ log_print("Detaching writecache completed cleaning.");
+
+ /*
+ * When the cleaner has finished, we can detach with noflush since
+ * the cleaner has done the flushing.
+ */
+
+ if (!lv_detach_writecache_cachevol(lv, 1)) {
+ log_error("Detaching writecache cachevol failed.");
+ ret = 0;
+ goto out_release;
+ }
+
+ ret = 1;
+ backup(vg);
+
+out_release:
+ unlock_and_release_vg(cmd, vg, vg->name);
+
+out_lockd:
+ if (!lockd_vg(cmd, id->vg_name, "un", 0, &lockd_state))
+ stack;
+
+ if (ret)
+ log_print_unless_silent("Logical volume %s write cache has been detached.", display_lvname(lv));
+ return ret;
+}
+
static int _writecache_zero(struct cmd_context *cmd, struct logical_volume *lv)
{
struct wipe_params wp = {
@@ -5838,6 +6095,8 @@ int lvconvert_writecache_attach_single(struct cmd_context *cmd,
is_active = lv_is_active(lv);
+ is_active = lv_is_active(lv);
+
memset(&settings, 0, sizeof(settings));
if (!get_writecache_settings(cmd, &settings, &block_size_sectors)) {
2 years, 4 months
master - pvcreate/pvremove: reimplement device checks
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=d1b7438c9fb7ab9b094...
Commit: d1b7438c9fb7ab9b0940ce433c0ece2fa17a6f03
Parent: 46e5908759a803173c2d8964d8ca832195993f3b
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Tue Jun 23 13:19:11 2020 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Thu Oct 1 10:09:09 2020 -0500
pvcreate/pvremove: reimplement device checks
Reorganize checking the device args for pvcreate/pvremove
to prepare for future changes. There should be no change
in behavior. Stop the inverted use of process_each_pv,
which pulled in a lot of unnecessary processing, and call
the check functions on each device directly.
---
tools/toollib.c | 542 ++++++++++++++++++++++++--------------------------------
1 file changed, 231 insertions(+), 311 deletions(-)
diff --git a/tools/toollib.c b/tools/toollib.c
index 0016648e3..1814399b9 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -4869,113 +4869,84 @@ static struct pvcreate_device *_pvcreate_list_find_name(struct dm_list *devices,
return NULL;
}
-/*
- * If this function decides that a arg_devices entry cannot be used, but the
- * command might be able to continue without it, then it moves that entry from
- * arg_devices to arg_fail.
- *
- * If this function decides that an arg_devices entry could be used (possibly
- * requiring a prompt), then it moves the entry from arg_devices to arg_process.
- *
- * Any arg_devices entries that are not moved to arg_fail or arg_process were
- * not found. The caller will decide if the command can continue if any
- * arg_devices entries were not found, or if any were moved to arg_fail.
- *
- * This check does not need to look at PVs in foreign, shared or clustered VGs.
- * If pvcreate/vgcreate/vgextend specifies a device in a
- * foreign/shared/clustered VG, that VG will not be processed by this function,
- * and the arg will be reported as not found.
- */
-static int _pvcreate_check_single(struct cmd_context *cmd,
- struct volume_group *vg,
- struct physical_volume *pv,
- struct processing_handle *handle)
+static int _pvcreate_check_used(struct cmd_context *cmd,
+ struct pvcreate_params *pp,
+ struct pvcreate_device *pd)
{
- struct pvcreate_params *pp = (struct pvcreate_params *) handle->custom_handle;
- struct pvcreate_device *pd;
struct pvcreate_prompt *prompt;
uint64_t size = 0;
uint64_t new_size = 0;
int need_size_prompt = 0;
int need_vg_prompt = 0;
- int found = 0;
+ struct lvmcache_info *info;
+ const char *vgname;
- if (!pv->dev)
- return 1;
+ log_debug("Checking %s for pvcreate %.32s.",
+ dev_name(pd->dev), pd->dev->pvid[0] ? pd->dev->pvid : "");
/*
- * Check if one of the command args in arg_devices
- * matches this device.
+ * Since the device is likely not a PV yet, it was probably not
+ * scanned by label_scan at the start of the command, so that
+ * needs to be done first to find if there's a PV label or metadata
+ * on it. If there's a PV label, it sets dev->pvid.
+ * If a VG is using the dev, it adds basic VG info for it to
+ * lvmcache.
*/
- dm_list_iterate_items(pd, &pp->arg_devices) {
- if (pd->dev != pv->dev)
- continue;
+ label_read(pd->dev);
- if (pv->dev->pvid[0])
- strncpy(pd->pvid, pv->dev->pvid, ID_LEN);
- found = 1;
- break;
+ if (!pd->dev->pvid[0]) {
+ log_debug("Check pvcreate arg %s no PVID found", dev_name(pd->dev));
+ pd->is_not_pv = 1;
+ return 1;
}
/*
- * Check if the uuid specified for the new PV is used by another PV.
+ * Don't allow using a device with duplicates.
*/
- if (!found && pv->dev && pp->uuid_str && id_equal(&pv->id, &pp->pva.id)) {
- log_error("UUID %s already in use on \"%s\".", pp->uuid_str, pv_dev_name(pv));
- pp->check_failed = 1;
+ if (lvmcache_pvid_in_unused_duplicates(pd->dev->pvid)) {
+ log_error("Cannot use device %s with duplicates.", dev_name(pd->dev));
+ dm_list_move(&pp->arg_fail, &pd->list);
return 0;
}
- if (!found)
- return 1;
-
- log_debug("Checking pvcreate arg %s which has existing PVID: %.32s.",
- pv_dev_name(pv), pv->dev->pvid[0] ? pv->dev->pvid : "<none>");
-
-
- /*
- * Don't allow using a device with duplicates.
- */
- if (lvmcache_pvid_in_unused_duplicates(pd->dev->pvid)) {
- log_error("Cannot use device %s with duplicates.", pd->name);
+ if (!(info = lvmcache_info_from_pvid(pd->dev->pvid, pd->dev, 0))) {
+ log_error("Failed to read lvm info for %s PVID %s.", dev_name(pd->dev), pd->dev->pvid);
dm_list_move(&pp->arg_fail, &pd->list);
- return 1;
+ return 0;
}
+ vgname = lvmcache_vgname_from_info(info);
+
/*
* What kind of device is this: an orphan PV, an uninitialized/unused
* device, a PV used in a VG.
*/
- if (vg && !is_orphan_vg(vg->name)) {
+ if (vgname && !is_orphan_vg(vgname)) {
/* Device is a PV used in a VG. */
- log_debug("Found pvcreate arg %s: pv is used in %s.", pd->name, vg->name);
+ log_debug("Check pvcreate arg %s found vg %s.", dev_name(pd->dev), vgname);
pd->is_vg_pv = 1;
- pd->vg_name = dm_pool_strdup(cmd->mem, vg->name);
- } else if (vg && is_orphan_vg(vg->name)) {
- if (is_used_pv(pv)) {
+ pd->vg_name = dm_pool_strdup(cmd->mem, vgname);
+ } else if (!vgname || (vgname && is_orphan_vg(vgname))) {
+ uint32_t ext_flags = lvmcache_ext_flags(info);
+ if (ext_flags & PV_EXT_USED) {
/* Device is used in an unknown VG. */
- log_debug("Found pvcreate arg %s: PV is used in unknown VG.", pd->name);
+ log_debug("Check pvcreate arg %s found EXT_USED flag.", dev_name(pd->dev));
pd->is_used_unknown_pv = 1;
} else {
/* Device is an orphan PV. */
- log_debug("Found pvcreate arg %s: PV is orphan in %s.", pd->name, vg->name);
+ log_debug("Check pvcreate arg %s is orphan.", dev_name(pd->dev));
pd->is_orphan_pv = 1;
}
-
pp->orphan_vg_name = FMT_TEXT_ORPHAN_VG_NAME;
- } else {
- log_debug("Found pvcreate arg %s: device is not a PV.", pd->name);
- /* Device is not a PV. */
- pd->is_not_pv = 1;
}
if (arg_is_set(cmd, setphysicalvolumesize_ARG)) {
new_size = arg_uint64_value(cmd, setphysicalvolumesize_ARG, UINT64_C(0));
- if (!dev_get_size(pv->dev, &size)) {
- log_error("Can't get device size of %s.", pv_dev_name(pv));
+ if (!dev_get_size(pd->dev, &size)) {
+ log_error("Can't get device size of %s.", dev_name(pd->dev));
dm_list_move(&pp->arg_fail, &pd->list);
- return 1;
+ return 0;
}
if (new_size != size)
@@ -4994,26 +4965,22 @@ static int _pvcreate_check_single(struct cmd_context *cmd,
else
need_vg_prompt = 1;
- if (!need_size_prompt && !need_vg_prompt) {
- pd->dev = pv->dev;
- dm_list_move(&pp->arg_process, &pd->list);
+ if (!need_size_prompt && !need_vg_prompt)
return 1;
- }
if (!(prompt = dm_pool_zalloc(cmd->mem, sizeof(*prompt)))) {
- log_error("prompt alloc failed.");
- pp->check_failed = 1;
- return 0;
+ dm_list_move(&pp->arg_fail, &pd->list);
+ return_0;
}
prompt->dev = pd->dev;
- prompt->pv_name = dm_pool_strdup(cmd->mem, pd->name);
+ prompt->pv_name = dm_pool_strdup(cmd->mem, dev_name(pd->dev));
prompt->size = size;
prompt->new_size = new_size;
if (pd->is_used_unknown_pv)
prompt->vg_name_unknown = 1;
else if (need_vg_prompt)
- prompt->vg_name = dm_pool_strdup(cmd->mem, vg->name);
+ prompt->vg_name = dm_pool_strdup(cmd->mem, vgname);
if (need_size_prompt)
prompt->type |= PROMPT_PVCREATE_DEV_SIZE;
@@ -5022,149 +4989,44 @@ static int _pvcreate_check_single(struct cmd_context *cmd,
prompt->type |= PROMPT_PVCREATE_PV_IN_VG;
dm_list_add(&pp->prompts, &prompt->list);
- pd->dev = pv->dev;
- dm_list_move(&pp->arg_process, &pd->list);
return 1;
}
-/*
- * This repeats the first check -- devices should be found, and should not have
- * changed since the first check. If they were changed/used while the orphans
- * lock was not held (during prompting), then they can't be used any more and
- * are moved to arg_fail. If they are not found by this loop, that also
- * disqualifies them from being used. Each arg_confirm entry that's found and
- * is ok, is moved to arg_process. Those not found will remain in arg_confirm.
- *
- * This check does not need to look in foreign/shared/clustered VGs. If a
- * device from arg_confirm was used in a foreign/shared/clustered VG during the
- * prompts, then it will not be found during this check.
- */
-
-static int _pv_confirm_single(struct cmd_context *cmd,
- struct volume_group *vg,
- struct physical_volume *pv,
- struct processing_handle *handle)
+static int _pvremove_check_used(struct cmd_context *cmd,
+ struct pvcreate_params *pp,
+ struct pvcreate_device *pd)
{
- struct pvcreate_params *pp = (struct pvcreate_params *) handle->custom_handle;
- struct pvcreate_device *pd;
- int found = 0;
-
- dm_list_iterate_items(pd, &pp->arg_confirm) {
- if (pd->dev != pv->dev)
- continue;
- found = 1;
- break;
- }
-
- if (!found)
- return 1;
-
- /*
- * What kind of device is this: an orphan PV, an uninitialized/unused
- * device, a PV used in a VG.
- */
- if (vg && !is_orphan_vg(vg->name)) {
- /* Device is a PV used in a VG. */
-
- if (pd->is_orphan_pv || pd->is_not_pv || pd->is_used_unknown_pv) {
- /* In check_single it was an orphan or unused. */
- goto fail;
- }
-
- if (pd->is_vg_pv && pd->vg_name && strcmp(pd->vg_name, vg->name)) {
- /* In check_single it was in a different VG. */
- goto fail;
- }
- } else if (is_orphan(pv)) {
- /* Device is an orphan PV. */
-
- if (pd->is_not_pv) {
- /* In check_single it was not a PV. */
- goto fail;
- }
-
- if (pd->is_vg_pv) {
- /* In check_single it was in a VG. */
- goto fail;
- }
-
- if (is_used_pv(pv) != pd->is_used_unknown_pv) {
- /* In check_single it was different. */
- goto fail;
- }
- } else {
- /* Device is not a PV. */
- if (pd->is_orphan_pv || pd->is_used_unknown_pv) {
- /* In check_single it was an orphan PV. */
- goto fail;
- }
-
- if (pd->is_vg_pv) {
- /* In check_single it was in a VG. */
- goto fail;
- }
- }
-
- /* Device is unchanged from check_single. */
- dm_list_move(&pp->arg_process, &pd->list);
-
- return 1;
-
-fail:
- log_error("Cannot use device %s: it changed during prompt.", pd->name);
- dm_list_move(&pp->arg_fail, &pd->list);
-
- return 1;
-}
-
-static int _pvremove_check_single(struct cmd_context *cmd,
- struct volume_group *vg,
- struct physical_volume *pv,
- struct processing_handle *handle)
-{
- struct pvcreate_params *pp = (struct pvcreate_params *) handle->custom_handle;
- struct pvcreate_device *pd;
struct pvcreate_prompt *prompt;
- int found = 0;
+ struct lvmcache_info *info;
+ const char *vgname = NULL;
- if (!pv->dev)
- return 1;
+ log_debug("Checking %s for pvremove %.32s.",
+ dev_name(pd->dev), pd->dev->pvid[0] ? pd->dev->pvid : "");
/*
- * Check if one of the command args in arg_devices
- * matches this device.
+ * Is there a pv here already?
+ * If not, this is an error unless you used -f.
*/
- dm_list_iterate_items(pd, &pp->arg_devices) {
- if (pd->dev != pv->dev)
- continue;
- if (pv->dev->pvid[0])
- strncpy(pd->pvid, pv->dev->pvid, ID_LEN);
- found = 1;
- break;
+ if (!pd->dev->pvid[0]) {
+ log_debug("Check pvremove arg %s no PVID found", dev_name(pd->dev));
+ if (pp->force)
+ return 1;
+ pd->is_not_pv = 1;
}
- if (!found)
- return 1;
-
- log_debug("Checking device %s for pvremove %.32s.",
- pv_dev_name(pv), pv->dev->pvid[0] ? pv->dev->pvid : "");
-
-
- /*
- * Is there a pv here already?
- * If not, this is an error unless you used -f.
- */
- if (!lvmcache_has_dev_info(pv->dev)) {
- if (pp->force) {
- dm_list_move(&pp->arg_process, &pd->list);
+ if (!(info = lvmcache_info_from_pvid(pd->dev->pvid, pd->dev, 0))) {
+ if (pp->force)
return 1;
- } else {
- pd->is_not_pv = 1;
- }
+ log_error("Failed to read lvm info for %s PVID %s.", dev_name(pd->dev), pd->dev->pvid);
+ dm_list_move(&pp->arg_fail, &pd->list);
+ return 0;
}
+ if (info)
+ vgname = lvmcache_vgname_from_info(info);
+
/*
* What kind of device is this: an orphan PV, an uninitialized/unused
* device, a PV used in a VG.
@@ -5172,49 +5034,40 @@ static int _pvremove_check_single(struct cmd_context *cmd,
if (pd->is_not_pv) {
/* Device is not a PV. */
- log_debug("Found pvremove arg %s: device is not a PV.", pd->name);
+ log_debug("Check pvremove arg %s device is not a PV.", dev_name(pd->dev));
- } else if (vg && !is_orphan_vg(vg->name)) {
+ } else if (vgname && !is_orphan_vg(vgname)) {
/* Device is a PV used in a VG. */
- log_debug("Found pvremove arg %s: pv is used in %s.", pd->name, vg->name);
+ log_debug("Check pvremove arg %s found vg %s.", dev_name(pd->dev), vgname);
pd->is_vg_pv = 1;
- pd->vg_name = dm_pool_strdup(cmd->mem, vg->name);
+ pd->vg_name = dm_pool_strdup(cmd->mem, vgname);
- } else if (vg && is_orphan_vg(vg->name)) {
- if (is_used_pv(pv)) {
+ } else if (info && (!vgname || (vgname && is_orphan_vg(vgname)))) {
+ uint32_t ext_flags = lvmcache_ext_flags(info);
+ if (ext_flags & PV_EXT_USED) {
/* Device is used in an unknown VG. */
- log_debug("Found pvremove arg %s: pv is used in unknown VG.", pd->name);
+ log_debug("Check pvremove arg %s found EXT_USED flag.", dev_name(pd->dev));
pd->is_used_unknown_pv = 1;
} else {
/* Device is an orphan PV. */
- log_debug("Found pvremove arg %s: pv is orphan in %s.", pd->name, vg->name);
+ log_debug("Check pvremove arg %s is orphan.", dev_name(pd->dev));
pd->is_orphan_pv = 1;
}
-
pp->orphan_vg_name = FMT_TEXT_ORPHAN_VG_NAME;
- } else {
- /* FIXME: is it possible to reach here? */
- log_debug("Found pvremove arg %s: device is not a PV.", pd->name);
- /* Device is not a PV. */
- pd->is_not_pv = 1;
}
if (pd->is_not_pv) {
- pd->dev = pv->dev;
- log_error("No PV found on device %s.", pd->name);
+ log_error("No PV found on device %s.", dev_name(pd->dev));
dm_list_move(&pp->arg_fail, &pd->list);
- return 1;
+ return 0;
}
/*
* pvremove is being run on this device, and it's not a PV,
* or is an orphan PV. Neither case requires a prompt.
*/
- if (pd->is_orphan_pv) {
- pd->dev = pv->dev;
- dm_list_move(&pp->arg_process, &pd->list);
+ if (pd->is_orphan_pv)
return 1;
- }
/*
* pvremove is being run on this device, but the device is in a VG.
@@ -5222,22 +5075,104 @@ static int _pvremove_check_single(struct cmd_context *cmd,
*/
if (!(prompt = dm_pool_zalloc(cmd->mem, sizeof(*prompt)))) {
- log_error("prompt alloc failed.");
- pp->check_failed = 1;
- return 0;
+ dm_list_move(&pp->arg_fail, &pd->list);
+ return_0;
}
prompt->dev = pd->dev;
- prompt->pv_name = dm_pool_strdup(cmd->mem, pd->name);
+ prompt->pv_name = dm_pool_strdup(cmd->mem, dev_name(pd->dev));
if (pd->is_used_unknown_pv)
prompt->vg_name_unknown = 1;
else
- prompt->vg_name = dm_pool_strdup(cmd->mem, vg->name);
+ prompt->vg_name = dm_pool_strdup(cmd->mem, vgname);
prompt->type |= PROMPT_PVREMOVE_PV_IN_VG;
dm_list_add(&pp->prompts, &prompt->list);
- pd->dev = pv->dev;
- dm_list_move(&pp->arg_process, &pd->list);
+ return 1;
+}
+
+static int _confirm_check_used(struct cmd_context *cmd,
+ struct pvcreate_params *pp,
+ struct pvcreate_device *pd)
+{
+ struct lvmcache_info *info = NULL;
+ const char *vgname = NULL;
+ int is_not_pv = 0;
+
+ log_debug("Checking %s to confirm %.32s.",
+ dev_name(pd->dev), pd->dev->pvid[0] ? pd->dev->pvid : "");
+
+ if (!pd->dev->pvid[0]) {
+ log_debug("Check confirm arg %s no PVID found", dev_name(pd->dev));
+ is_not_pv = 1;
+ }
+
+ if (!(info = lvmcache_info_from_pvid(pd->dev->pvid, pd->dev, 0))) {
+ log_debug("Check confirm arg %s no info.", dev_name(pd->dev));
+ is_not_pv = 1;
+ }
+
+ if (info)
+ vgname = lvmcache_vgname_from_info(info);
+
+
+ /*
+ * What kind of device is this: an orphan PV, an uninitialized/unused
+ * device, a PV used in a VG.
+ */
+ if (vgname && !is_orphan_vg(vgname)) {
+ /* Device is a PV used in a VG. */
+
+ if (pd->is_orphan_pv || pd->is_not_pv || pd->is_used_unknown_pv) {
+ /* In first check it was an orphan or unused. */
+ goto fail;
+ }
+
+ if (pd->is_vg_pv && pd->vg_name && strcmp(pd->vg_name, vgname)) {
+ /* In first check it was in a different VG. */
+ goto fail;
+ }
+ } else if (info && (!vgname || is_orphan_vg(vgname))) {
+ uint32_t ext_flags = lvmcache_ext_flags(info);
+
+ /* Device is an orphan PV. */
+
+ if (pd->is_not_pv) {
+ /* In first check it was not a PV. */
+ goto fail;
+ }
+
+ if (pd->is_vg_pv) {
+ /* In first check it was in a VG. */
+ goto fail;
+ }
+
+ if ((ext_flags & PV_EXT_USED) && !pd->is_used_unknown_pv) {
+ /* In first check it was different. */
+ goto fail;
+ }
+
+ if (!(ext_flags & PV_EXT_USED) && pd->is_used_unknown_pv) {
+ /* In first check it was different. */
+ goto fail;
+ }
+ } else if (is_not_pv) {
+ /* Device is not a PV. */
+ if (pd->is_orphan_pv || pd->is_used_unknown_pv) {
+ /* In first check it was an orphan PV. */
+ goto fail;
+ }
+
+ if (pd->is_vg_pv) {
+ /* In first check it was in a VG. */
+ goto fail;
+ }
+ }
+
+ return 1;
+fail:
+ log_error("Cannot use device %s: it changed during prompt.", dev_name(pd->dev));
+ dm_list_move(&pp->arg_fail, &pd->list);
return 1;
}
@@ -5247,10 +5182,6 @@ static int _pvremove_check_single(struct cmd_context *cmd,
* line args. This includes the pv_names field which specifies the devices to
* create PVs on.
*
- * This uses process_each_pv() and should be called from a high level in the
- * command -- the same level at which other instances of process_each are
- * called.
- *
* This function returns 0 (failed) if the caller requires all specified
* devices to be created, and any of those devices are not found, or any of
* them cannot be created.
@@ -5278,6 +5209,7 @@ int pvcreate_each_device(struct cmd_context *cmd,
unsigned int physical_block_size, logical_block_size;
unsigned int prev_pbs = 0, prev_lbs = 0;
int must_use_all = (cmd->cname->flags & MUST_USE_ALL_ARGS);
+ int unlocked_for_prompts = 0;
int found;
unsigned i;
@@ -5313,17 +5245,28 @@ int pvcreate_each_device(struct cmd_context *cmd,
/*
* Translate arg names into struct device's.
*/
- dm_list_iterate_items(pd, &pp->arg_devices)
+ dm_list_iterate_items_safe(pd, pd2, &pp->arg_devices) {
pd->dev = dev_cache_get(cmd, pd->name, cmd->filter);
+ if (!pd->dev) {
+ log_error("Device %s not found.", pd->name);
+ dm_list_del(&pd->list);
+ dm_list_add(&pp->arg_fail, &pd->list);
+ }
+ }
+
+ /*
+ * Can the command continue if some specified devices were not found?
+ */
+ if (must_use_all && !dm_list_empty(&pp->arg_fail)) {
+ log_error("Command requires all devices to be found.");
+ return_0;
+ }
/*
* Check for consistent block sizes.
*/
if (pp->check_consistent_block_size) {
dm_list_iterate_items(pd, &pp->arg_devices) {
- if (!pd->dev)
- continue;
-
logical_block_size = 0;
physical_block_size = 0;
@@ -5361,58 +5304,48 @@ int pvcreate_each_device(struct cmd_context *cmd,
}
}
- /*
- * Use process_each_pv to search all existing PVs and devices.
- *
- * This is a slightly different way to use process_each_pv, because the
- * command args (arg_devices) are not being processed directly by
- * process_each_pv (argc and argv are not passed). Instead,
- * process_each_pv is processing all existing PVs and devices, and the
- * single function is matching each of those against the command args
- * (arg_devices).
- *
- * If an arg_devices entry is found during process_each_pv, it's moved
- * to arg_process if it can be used, or arg_fail if it cannot be used.
- * If it's added to arg_process but needs a prompt or force option, then
- * a corresponding prompt entry is added to pp->prompts.
- */
- process_each_pv(cmd, 0, NULL, NULL, 1, PROCESS_SKIP_SCAN,
- handle, pp->is_remove ? _pvremove_check_single : _pvcreate_check_single);
+ /* check_used moves pd entries into the arg_fail list if pvcreate/pvremove is disallowed */
+ dm_list_iterate_items_safe(pd, pd2, &pp->arg_devices) {
+ if (pp->is_remove)
+ _pvremove_check_used(cmd, pp, pd);
+ else
+ _pvcreate_check_used(cmd, pp, pd);
+ }
/*
- * A fatal error was found while checking.
+ * If the user specified a uuid for the new PV, check
+ * if a PV on another dev is already using that uuid.
*/
- if (pp->check_failed)
- goto_bad;
+ if (!pp->is_remove && pp->uuid_str) {
+ struct device *dev;
+ if ((dev = lvmcache_device_from_pvid(cmd, &pp->pva.id, NULL))) {
+ dm_list_iterate_items_safe(pd, pd2, &pp->arg_devices) {
+ if (pd->dev != dev) {
+ log_error("UUID %s already in use on \"%s\".", pp->uuid_str, dev_name(dev));
+ dm_list_move(&pp->arg_fail, &pd->list);
+ }
+ }
+ }
+ }
/*
* Special case: pvremove -ff is allowed to clear a duplicate device in
- * the unchosen duplicates list. PVs in the unchosen duplicates list
- * won't be found by normal process_each searches -- they are not in
- * lvmcache and can't be processed normally. We save them here and
- * erase them below without going through the normal processing code.
+ * the unchosen duplicates list. We save them here and erase them below.
*/
if (pp->is_remove && (pp->force == DONT_PROMPT_OVERRIDE) &&
!dm_list_empty(&pp->arg_devices) && lvmcache_has_duplicate_devs()) {
dm_list_iterate_items_safe(pd, pd2, &pp->arg_devices) {
if (lvmcache_dev_is_unused_duplicate(pd->dev)) {
- log_debug("Found pvremove arg %s: device is a duplicate.", pd->name);
+ log_debug("Check pvremove arg %s device is a duplicate.", dev_name(pd->dev));
dm_list_move(&remove_duplicates, &pd->list);
}
}
}
/*
- * Check if all arg_devices were found by process_each_pv.
+ * Any devices not moved to arg_fail can be processed.
*/
- dm_list_iterate_items(pd, &pp->arg_devices)
- log_error("Device %s %s.", pd->name, dev_cache_filtered_reason(pd->name));
-
- /*
- * Can the command continue if some specified devices were not found?
- */
- if (!dm_list_empty(&pp->arg_devices) && must_use_all)
- goto_bad;
+ dm_list_splice(&pp->arg_process, &pp->arg_devices);
/*
* Can the command continue if some specified devices cannot be used?
@@ -5468,8 +5401,10 @@ int pvcreate_each_device(struct cmd_context *cmd,
lockf_global(cmd, "un");
+ unlocked_for_prompts = 1;
+
/*
- * Process prompts that require asking the user. The orphans lock is
+ * Process prompts that require asking the user. The global lock is
* not held, so there's no harm in waiting for a user to respond.
*/
dm_list_iterate_items_safe(prompt, prompt2, &pp->prompts) {
@@ -5500,7 +5435,7 @@ int pvcreate_each_device(struct cmd_context *cmd,
/*
* Reacquire the lock that was released above before waiting, then
- * check again that the devices can still be used. If the second loop
+ * check again that the devices can still be used. If the second check
* finds them changed, or can't find them any more, then they aren't
* used. Use a non-blocking request when reacquiring to avoid
* potential deadlock since this is not the normal locking sequence.
@@ -5511,41 +5446,6 @@ int pvcreate_each_device(struct cmd_context *cmd,
goto_out;
}
- lvmcache_label_scan(cmd);
-
- /*
- * The device args began on the arg_devices list, then the first check
- * loop moved those entries to arg_process as they were found. Devices
- * not found during the first loop are not being used, and remain on
- * arg_devices.
- *
- * Now, the arg_process entries are moved to arg_confirm, and the second
- * check loop moves them back to arg_process as they are found and are
- * unchanged. Like the first loop, the second loop moves an entry to
- * arg_fail if it cannot be used. After the second loop, any devices
- * remaining on arg_confirm were not found and are not used.
- */
- dm_list_splice(&pp->arg_confirm, &pp->arg_process);
-
- process_each_pv(cmd, 0, NULL, NULL, 1, PROCESS_SKIP_SCAN,
- handle, _pv_confirm_single);
-
- dm_list_iterate_items(pd, &pp->arg_confirm)
- log_error("Device %s %s.", pd->name, dev_cache_filtered_reason(pd->name));
-
- /* Some devices were not found during the second check. */
- if (!dm_list_empty(&pp->arg_confirm) && must_use_all)
- goto_bad;
-
- /* Some devices changed during the second check. */
- if (!dm_list_empty(&pp->arg_fail) && must_use_all)
- goto_bad;
-
- if (dm_list_empty(&pp->arg_process)) {
- log_debug("No devices to process.");
- goto bad;
- }
-
do_command:
dm_list_iterate_items(pd, &pp->arg_process) {
@@ -5561,6 +5461,25 @@ do_command:
goto bad;
}
+ /*
+ * If the global lock was unlocked to wait for prompts, then
+ * devs could have changed while unlocked, so confirm that
+ * the devs are unchanged since check_used.
+ * Changed pd entries are moved to arg_fail.
+ */
+ if (unlocked_for_prompts) {
+ dm_list_iterate_items_safe(pd, pd2, &pp->arg_process)
+ _confirm_check_used(cmd, pp, pd);
+
+ if (!dm_list_empty(&pp->arg_fail) && must_use_all)
+ goto_bad;
+ }
+
+ if (dm_list_empty(&pp->arg_process)) {
+ log_debug("No devices to process.");
+ goto bad;
+ }
+
/*
* Reorder arg_process entries to match the original order of args.
*/
@@ -5579,6 +5498,7 @@ do_command:
* Wipe signatures on devices being created.
*/
dm_list_iterate_items_safe(pd, pd2, &pp->arg_create) {
+ /* FIXME: won't all already be open excl from label_scan_devs_excl above? */
label_scan_open_excl(pd->dev);
log_verbose("Wiping signatures on new PV %s.", pd->name);
@@ -5658,6 +5578,7 @@ do_command:
pv_name = pd->name;
+ /* FIXME: won't all already be open excl from label_scan_devs_excl above? */
label_scan_open_excl(pd->dev);
log_debug("Creating a new PV on %s.", pv_name);
@@ -5671,7 +5592,6 @@ do_command:
log_verbose("Set up physical volume for \"%s\" with %" PRIu64
" available sectors.", pv_name, pv_size(pv));
-
if (!label_remove(pv->dev)) {
log_error("Failed to wipe existing label on %s.", pv_name);
dm_list_move(&pp->arg_fail, &pd->list);
2 years, 4 months
master - test: grep -q may fail and it does
by Marian Csontos
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=46e5908759a803173c2...
Commit: 46e5908759a803173c2d8964d8ca832195993f3b
Parent: 2272a32e6f9b99f98514e58de82f8a3baa8b46da
Author: Marian Csontos <mcsontos(a)redhat.com>
AuthorDate: Thu Oct 1 11:33:57 2020 +0200
Committer: Marian Csontos <mcsontos(a)redhat.com>
CommitterDate: Thu Oct 1 11:33:57 2020 +0200
test: grep -q may fail and it does
The script runs with pipefail, grep -q exits immediately sending SIGPIPE
to lvm segtype which fails whole pipe.
---
test/lib/aux.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/test/lib/aux.sh b/test/lib/aux.sh
index 77942009b..02989c5cb 100644
--- a/test/lib/aux.sh
+++ b/test/lib/aux.sh
@@ -1595,7 +1595,7 @@ have_thin() {
}
have_vdo() {
- lvm segtypes 2>/dev/null | grep vdo$ >/dev/null || {
+ lvm segtypes 2>/dev/null | grep 'vdo$' >/dev/null || {
echo "VDO is not built-in." >&2
return 1
}
@@ -1603,7 +1603,7 @@ have_vdo() {
}
have_writecache() {
- lvm segtypes 2>/dev/null | grep -q writecache$ || {
+ lvm segtypes 2>/dev/null | grep 'writecache$' >/dev/null || {
echo "writecache is not built-in." >&2
return 1
}
@@ -1611,7 +1611,7 @@ have_writecache() {
}
have_integrity() {
- lvm segtypes 2>/dev/null | grep -q integrity$ || {
+ lvm segtypes 2>/dev/null | grep 'integrity$' >/dev/null || {
echo "integrity is not built-in." >&2
return 1
}
2 years, 4 months