From 47f8bda051817649be23396419c4ffc21bb58ea1 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Wed, 22 May 2024 15:32:17 -0500 Subject: [PATCH] lvremove: remove device_id for PVs on LVs When PVs are created on LVs, remove the devices file entries for the PVs when the LVs are removed. In general, the devices file entries should be removed with lvmdevices --deldev when the LVs are removed (lvremove is the equivalent of detaching a device from the system when layering PVs on LVs.) This change is effectively an automatic lvmdevices --deldev command that is built into lvremove when the LV has a PV on it. --- lib/device/device_id.c | 67 +++++++++++++++++++++++++++++- lib/device/device_id.h | 2 + lib/metadata/metadata-exported.h | 4 ++ test/shell/devicesfile-scan-lvs.sh | 62 +++++++++++++++++++++++++++ tools/lvremove.c | 22 +++++++++- tools/toollib.c | 30 ++++++++++++- 6 files changed, 182 insertions(+), 5 deletions(-) create mode 100644 test/shell/devicesfile-scan-lvs.sh diff --git a/lib/device/device_id.c b/lib/device/device_id.c index 1ce7927ed..6af03df18 100644 --- a/lib/device/device_id.c +++ b/lib/device/device_id.c @@ -1861,7 +1861,7 @@ int device_ids_use_devname(struct cmd_context *cmd) return 0; } -static int _device_ids_use_lvmlv(struct cmd_context *cmd) +int device_ids_use_lvmlv(struct cmd_context *cmd) { struct dev_use *du; @@ -2243,6 +2243,69 @@ void device_id_pvremove(struct cmd_context *cmd, struct device *dev) } } + +/* + * Remove LVMLV_UUID entries from system.devices for LVs that were removed. + * lvremove vg/lv where a PV exists on vg/lv does an automatic + * lvmdevices --deldev /dev/vg/lv + */ +void device_id_lvremove(struct cmd_context *cmd, struct dm_list *removed_uuids) +{ + struct dev_use *du; + struct dm_str_list *sl; + int found = 0; + + if (!device_ids_use_lvmlv(cmd)) + return; + + dm_list_iterate_items(sl, removed_uuids) { + if (!(du = get_du_for_device_id(cmd, DEV_ID_TYPE_LVMLV_UUID, sl->str))) + continue; + found++; + } + + if (!found) + return; + + if (!lock_devices_file(cmd, LOCK_EX)) + return; + + /* + * Clear cmd->use_devices which may no longer be an accurate + * representation of system.devices, since another command may have + * changed system.devices after this command read and unlocked it. + */ + free_dus(&cmd->use_devices); + + /* + * Reread system.devices, recreating cmd->use_devices. + */ + if (!device_ids_read(cmd)) { + log_debug("Failed to read devices file"); + goto out; + } + + found = 0; + + dm_list_iterate_items(sl, removed_uuids) { + if (!(du = get_du_for_device_id(cmd, DEV_ID_TYPE_LVMLV_UUID, sl->str))) + continue; + + log_debug("Removing devices file entry for device_id %s", sl->str); + dm_list_del(&du->list); + free_du(du); + found++; + } + + if (!found) + goto out; + + if (!device_ids_write(cmd)) + log_debug("Failed to write devices file"); +out: + unlock_devices_file(cmd); +} + void device_id_update_vg_uuid(struct cmd_context *cmd, struct volume_group *vg, struct id *old_vg_id) { struct dev_use *du; @@ -2260,7 +2323,7 @@ void device_id_update_vg_uuid(struct cmd_context *cmd, struct volume_group *vg, return; /* Check if any devices file entries are stacked on LVs. */ - if (!_device_ids_use_lvmlv(cmd)) + if (!device_ids_use_lvmlv(cmd)) return; memcpy(old_vgid, old_vg_id, ID_LEN); diff --git a/lib/device/device_id.h b/lib/device/device_id.h index 2603ba0f0..29fe9bd63 100644 --- a/lib/device/device_id.h +++ b/lib/device/device_id.h @@ -25,11 +25,13 @@ uint16_t idtype_from_str(const char *str); const char *dev_idtype_for_metadata(struct cmd_context *cmd, struct device *dev); const char *dev_idname_for_metadata(struct cmd_context *cmd, struct device *dev); int device_ids_use_devname(struct cmd_context *cmd); +int device_ids_use_lvmlv(struct cmd_context *cmd); int device_ids_read(struct cmd_context *cmd); int device_ids_write(struct cmd_context *cmd); int device_id_add(struct cmd_context *cmd, struct device *dev, const char *pvid, const char *idtype_arg, const char *id_arg, int use_idtype_only); void device_id_pvremove(struct cmd_context *cmd, struct device *dev); +void device_id_lvremove(struct cmd_context *cmd, struct dm_list *removed_uuids); void device_ids_match(struct cmd_context *cmd); int device_ids_match_dev(struct cmd_context *cmd, struct device *dev); void device_ids_match_device_list(struct cmd_context *cmd); diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index f894aca26..5ca8f7cfb 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -1467,6 +1467,10 @@ struct vgcreate_params { const char *lock_args; }; +struct lvremove_params { + struct dm_list removed_uuids; /* entries are str_list */ +}; + int validate_major_minor(const struct cmd_context *cmd, const struct format_type *fmt, int32_t major, int32_t minor); diff --git a/test/shell/devicesfile-scan-lvs.sh b/test/shell/devicesfile-scan-lvs.sh new file mode 100644 index 000000000..17de35b68 --- /dev/null +++ b/test/shell/devicesfile-scan-lvs.sh @@ -0,0 +1,62 @@ + + +# Copyright (C) 2020 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +test_description='devices file' + +SKIP_WITH_LVMPOLLD=1 + +. lib/inittest + +aux prepare_devs 1 + +# The tests run with system dir of "/etc" but lvm when running +# normally has cmd->system_dir set to "/etc/lvm". +DFDIR="$LVM_SYSTEM_DIR/devices" +mkdir -p "$DFDIR" || true +DF="$DFDIR/system.devices" + +aux lvmconf 'devices/use_devicesfile = 1' +aux lvmconf 'devices/scan_lvs = 1' + +rm -f "$DF" +touch "$DF" + +vgcreate $vg1 "$dev1" +lvcreate -n $lv1 -L 8M $vg1 +lvcreate -n $lv2 -L 8M $vg1 +pvcreate $DM_DEV_DIR/$vg1/$lv1 +pvcreate $DM_DEV_DIR/$vg1/$lv2 +vgcreate $vg2 $DM_DEV_DIR/$vg1/$lv1 $DM_DEV_DIR/$vg1/$lv2 + +pvs $DM_DEV_DIR/$vg1/$lv1 +pvs $DM_DEV_DIR/$vg1/$lv2 + +grep "$dev1" "$DF" +grep $DM_DEV_DIR/$vg1/$lv1 "$DF" | tee out1 +grep "IDTYPE=lvmlv_uuid" out1 +grep "IDNAME=LVM-" out1 +grep $DM_DEV_DIR/$vg1/$lv2 "$DF" | tee out2 +grep "IDTYPE=lvmlv_uuid" out2 +grep "IDNAME=LVM-" out2 + +lvremove -y $vg1/$lv1 +not grep $DM_DEV_DIR/$vg1/$lv1 "$DF" + +lvremove -y $vg1/$lv2 +not grep $DM_DEV_DIR/$vg1/$lv2 "$DF" + +grep "$dev1" "$DF" +not grep "IDTYPE=lvmlv_uuid" "$DF" +not grep "IDNAME=LVM-" "$DF" + +vgremove -y $vg1 + diff --git a/tools/lvremove.c b/tools/lvremove.c index 4653817e1..724a22ea3 100644 --- a/tools/lvremove.c +++ b/tools/lvremove.c @@ -17,6 +17,10 @@ int lvremove(struct cmd_context *cmd, int argc, char **argv) { + struct processing_handle *handle = NULL; + struct lvremove_params lp = { 0 }; + int ret; + if (!argc && !arg_is_set(cmd, select_ARG)) { log_error("Please enter one or more logical volume paths " "or use --select for selection."); @@ -26,6 +30,22 @@ int lvremove(struct cmd_context *cmd, int argc, char **argv) cmd->handles_missing_pvs = 1; cmd->include_historical_lvs = 1; - return process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, NULL, + if (!(handle = init_processing_handle(cmd, NULL))) { + log_error("Failed to initialize processing handle."); + return ECMD_FAILED; + } + + dm_list_init(&lp.removed_uuids); + + handle->custom_handle = &lp; + + ret = process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, handle, NULL, &lvremove_single); + + if (cmd->scan_lvs && cmd->enable_devices_file) + device_id_lvremove(cmd, &lp.removed_uuids); + + destroy_processing_handle(cmd, handle); + + return ret; } diff --git a/tools/toollib.c b/tools/toollib.c index 080ee5429..6fdd0cfae 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -18,6 +18,7 @@ #include "lib/label/hints.h" #include "lib/device/device_id.h" #include "lib/device/online.h" +#include "libdm/misc/dm-ioctl.h" #include #include @@ -4782,9 +4783,31 @@ out: return ret_max; } -int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv, - struct processing_handle *handle __attribute__((unused))) +static void _lvremove_save_uuid(struct cmd_context *cmd, struct logical_volume *lv, + struct lvremove_params *lp) { + char dm_uuid[DM_UUID_LEN] = { 0 }; + + /* + * Create the dm/uuid string that would be displayed + * in sysfs for this LV. + * + * DM_UUID_LEN is 129 + * ID_LEN is 32 + */ + memcpy(dm_uuid, "LVM-", 4); + memcpy(dm_uuid+4, &lv->vg->id, ID_LEN); + memcpy(dm_uuid+4+ID_LEN, &lv->lvid.id[1], ID_LEN); + + if (!str_list_add(cmd->mem, &lp->removed_uuids, dm_pool_strdup(cmd->mem, dm_uuid))) + stack; +} + +int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv, + struct processing_handle *handle) +{ + struct lvremove_params *lp = (struct lvremove_params *) handle->custom_handle; + /* * Single force is equivalent to single --yes * Even multiple --yes are equivalent to single --force @@ -4796,6 +4819,9 @@ int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv, if (!lv_remove_with_dependencies(cmd, lv, force, 0)) return_ECMD_FAILED; + if (cmd->scan_lvs && cmd->enable_devices_file) + _lvremove_save_uuid(cmd, lv, lp); + return ECMD_PROCESSED; }