From 125712bea0a368a6cad1d358d503ae3e7b309927 Mon Sep 17 00:00:00 2001 From: Peter Rajnoha Date: Thu, 22 Sep 2011 17:33:50 +0000 Subject: [PATCH] Replace open_count check with holders/mounted_fs check on lvremove path. Before, we used to display "Can't remove open logical volume" which was generic. There 3 possibilities of how a device could be opened: - used by another device - having a filesystem on that device which is mounted - opened directly by an application With the help of sysfs info, we can distinguish the first two situations. The third one will be subject to "remove retry" logic - if it's opened quickly (e.g. a parallel scan from within a udev rule run), this will finish quickly and we can remove it once it has finished. If it's a legitimate application that keeps the device opened, we'll do our best to remove the device, but we will fail finally after a few retries. --- WHATS_NEW | 1 + lib/activate/activate.c | 33 ++++++++++++++++++++++++++++----- lib/activate/activate.h | 3 +++ lib/commands/toolcontext.c | 1 + lib/metadata/lv_manip.c | 7 ++----- libdm/libdm-deptree.c | 30 ++++++++++++++++++++++++++++-- 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index a1f598030..4da72a841 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.89 - ================================== + Replace open_count check with holders/mounted_fs check on lvremove path. Disallow the creation of mirrors (mirror or raid1 segtype) with only one leg. Cleanup restart clvmd code (no memory allocation, debug print passed args). Add all exclusive locks to clvmd restart option args. diff --git a/lib/activate/activate.c b/lib/activate/activate.c index ae769dde3..21f597bcc 100644 --- a/lib/activate/activate.c +++ b/lib/activate/activate.c @@ -526,6 +526,31 @@ int lv_info_by_lvid(struct cmd_context *cmd, const char *lvid_s, return r; } +int lv_check_not_in_use(struct cmd_context *cmd __attribute__((unused)), + struct logical_volume *lv, struct lvinfo *info) +{ + if (!info->exists) + return 1; + + /* If sysfs is not used, use open_count information only. */ + if (!*dm_sysfs_dir()) + return !info->open_count; + + if (dm_device_has_holders(info->major, info->minor)) { + log_error("Logical volume %s/%s is used by another device.", + lv->vg->name, lv->name); + return 0; + } + + if (dm_device_has_mounted_fs(info->major, info->minor)) { + log_error("Logical volume %s/%s contains a filesystem in use.", + lv->vg->name, lv->name); + return 0; + } + + return 1; +} + /* * Returns 1 if percent set, else 0 on failure. */ @@ -1437,11 +1462,9 @@ int lv_deactivate(struct cmd_context *cmd, const char *lvid_s) } if (lv_is_visible(lv)) { - if (info.open_count) { - log_error("LV %s/%s in use: not deactivating", - lv->vg->name, lv->name); - goto out; - } + if (!lv_check_not_in_use(cmd, lv, &info)) + goto_out; + if (lv_is_origin(lv) && _lv_has_open_snapshots(lv)) goto_out; } diff --git a/lib/activate/activate.h b/lib/activate/activate.h index 7030633c7..5b90420f8 100644 --- a/lib/activate/activate.h +++ b/lib/activate/activate.h @@ -80,6 +80,9 @@ int lv_info(struct cmd_context *cmd, const struct logical_volume *lv, int lv_info_by_lvid(struct cmd_context *cmd, const char *lvid_s, unsigned origin_only, struct lvinfo *info, int with_open_count, int with_read_ahead); +int lv_check_not_in_use(struct cmd_context *cmd, struct logical_volume *lv, + struct lvinfo *info); + /* * Returns 1 if activate_lv has been set: 1 = activate; 0 = don't. */ diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index 859da2bdf..336689604 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -264,6 +264,7 @@ static int _process_config(struct cmd_context *cmd) /* FIXME Use global value of sysfs_dir everywhere instead cmd->sysfs_dir. */ _get_sysfs_dir(cmd); set_sysfs_dir_path(cmd->sysfs_dir); + dm_set_sysfs_dir(cmd->sysfs_dir); /* activation? */ cmd->default_settings.activation = find_config_tree_int(cmd, diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index 61e84c1de..06d809beb 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -3099,11 +3099,8 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv, /* FIXME Ensure not referred to by another existing LVs */ if (lv_info(cmd, lv, 0, &info, 1, 0)) { - if (info.open_count) { - log_error("Can't remove open logical volume \"%s\"", - lv->name); - return 0; - } + if (!lv_check_not_in_use(cmd, lv, &info)) + return_0; if (lv_is_active(lv) && (force == PROMPT) && lv_is_visible(lv) && diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c index 0db8de2e5..45025047a 100644 --- a/libdm/libdm-deptree.c +++ b/libdm/libdm-deptree.c @@ -940,6 +940,30 @@ static int _info_by_dev(uint32_t major, uint32_t minor, int with_open_count, return r; } +static int _check_device_not_in_use(struct dm_info *info) +{ + if (!info->exists) + return 1; + + /* If sysfs is not used, use open_count information only. */ + if (!*dm_sysfs_dir()) + return !info->open_count; + + if (dm_device_has_holders(info->major, info->minor)) { + log_error("Device %" PRIu32 ":%" PRIu32 " is used " + "by another device.", info->major, info->minor); + return 0; + } + + if (dm_device_has_mounted_fs(info->major, info->minor)) { + log_error("Device %" PRIu32 ":%" PRIu32 " contains " + "a filesystem in use.", info->major, info->minor); + return 0; + } + + return 1; +} + /* Check if all parent nodes of given node have open_count == 0 */ static int _node_has_closed_parents(struct dm_tree_node *node, const char *uuid_prefix, @@ -1184,9 +1208,11 @@ static int _dm_tree_deactivate_children(struct dm_tree_node *dnode, !info.exists) continue; + if (!_check_device_not_in_use(&info)) + continue; + /* Also checking open_count in parent nodes of presuspend_node */ - if (info.open_count || - (child->presuspend_node && + if ((child->presuspend_node && !_node_has_closed_parents(child->presuspend_node, uuid_prefix, uuid_prefix_len))) { /* Only report error from (likely non-internal) dependency at top level */ -- 2.43.5