lvm2-commits July 2021

lvm2-commits@lists.fedorahosted.org

3 participants
36 discussions

main - vdo: add vdoimport support
by Zdenek Kabelac 09 Jul '21

09 Jul '21

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=ed48cb26a31e8e5a12d82… Commit: ed48cb26a31e8e5a12d82d96d8bec6bfc4d5b218 Parent: 3a92d633a5a23377aa0d5d4ed5a01db8ca14cf69 Author: Zdenek Kabelac <zkabelac(a)redhat.com> AuthorDate: Fri Jul 9 14:44:07 2021 +0200 Committer: Zdenek Kabelac <zkabelac(a)redhat.com> CommitterDate: Fri Jul 9 14:57:59 2021 +0200 vdo: add vdoimport support Add tool 'vdoimport' to support easy conversion of an existing VDO manager managed VDO volumes into lvm2 managed VDO LV. When physical converted volume is already a logical volume, conversion happens with the VG itself, just with validation for extent_size, so the virtually sized logical VDO volume size can be expressed in extents. Example of basic simple usage: vdoimport --name vg/vdolv /dev/mapper/vdophysicalvolume --- WHATS_NEW | 1 + configure | 25 +++ configure.ac | 15 ++ include/configure.h.in | 3 + man/Makefile.in | 7 +- scripts/Makefile.in | 4 + scripts/vdoimport.sh | 376 ++++++++++++++++++++++++++++++++++++++++++++++ spec/packages.inc | 1 + test/Makefile.in | 1 + test/shell/vdo-convert.sh | 110 ++++++++++++++ 10 files changed, 542 insertions(+), 1 deletion(-) diff --git a/WHATS_NEW b/WHATS_NEW index 8f6dad27a..810411914 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.03.13 - =============================== + Add vdoimport tool to support conversion of VDO volumes. Support configurable allocation/vdo_pool_header_size. Fix handling of lvconvert --type vdo-pool --virtualsize. Simplified handling of archive() and backup() internal calls. diff --git a/configure b/configure index 3a9888528..c2d2cff5b 100755 --- a/configure +++ b/configure @@ -643,6 +643,8 @@ WRITE_INSTALL WRITECACHE VDO_LIB VDO_INCLUDE +VDOIMPORT_PATH +VDOIMPORT VDO VALGRIND_POOL USRSBINDIR @@ -969,6 +971,7 @@ enable_dbus_service enable_pkgconfig enable_write_install enable_fsadm +enable_vdoimport enable_blkdeactivate enable_dmeventd enable_selinux @@ -1707,6 +1710,7 @@ Optional Features: --enable-pkgconfig install pkgconfig support --enable-write_install install user writable files --disable-fsadm disable fsadm + --disable-vdoimport disable vdoimport --disable-blkdeactivate disable blkdeactivate --enable-dmeventd enable the device-mapper event daemon --disable-selinux disable selinux support @@ -3139,6 +3143,7 @@ case "$host_os" in DM_IOCTLS=yes SELINUX=yes FSADM=yes + VDOIMPORT=yes BLKDEACTIVATE=yes ;; darwin*) @@ -3152,6 +3157,7 @@ case "$host_os" in DM_IOCTLS=no SELINUX=no FSADM=no + VDOIMPORT=no BLKDEACTIVATE=no ;; *) @@ -12544,6 +12550,18 @@ fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $FSADM" >&5 $as_echo "$FSADM" >&6; } + +################################################################################ +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to install vdoimport" >&5 +$as_echo_n "checking whether to install vdoimport... " >&6; } +# Check whether --enable-vdoimport was given. +if test "${enable_vdoimport+set}" = set; then : + enableval=$enable_vdoimport; VDOIMPORT=$enableval +fi + +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $VDOIMPORT" >&5 +$as_echo "$VDOIMPORT" >&6; } + ################################################################################ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to install blkdeactivate" >&5 $as_echo_n "checking whether to install blkdeactivate... " >&6; } @@ -14030,6 +14048,13 @@ cat >>confdefs.h <<_ACEOF _ACEOF +VDOIMPORT_PATH="$SBINDIR/vdoimport" + +cat >>confdefs.h <<_ACEOF +#define VDOIMPORT_PATH "$VDOIMPORT_PATH" +_ACEOF + + ################################################################################ if test "$BUILD_DMEVENTD" = yes; then diff --git a/configure.ac b/configure.ac index 40acc49c2..a20633e21 100644 --- a/configure.ac +++ b/configure.ac @@ -46,6 +46,7 @@ case "$host_os" in DM_IOCTLS=yes SELINUX=yes FSADM=yes + VDOIMPORT=yes BLKDEACTIVATE=yes ;; darwin*) @@ -59,6 +60,7 @@ case "$host_os" in DM_IOCTLS=no SELINUX=no FSADM=no + VDOIMPORT=no BLKDEACTIVATE=no ;; *) @@ -1311,6 +1313,14 @@ AC_ARG_ENABLE(fsadm, AC_HELP_STRING([--disable-fsadm], [disable fsadm]), FSADM=$enableval) AC_MSG_RESULT($FSADM) + +################################################################################ +dnl -- Enable vdoimport +AC_MSG_CHECKING(whether to install vdoimport) +AC_ARG_ENABLE(vdoimport, AC_HELP_STRING([--disable-vdoimport], [disable vdoimport]), + VDOIMPORT=$enableval) +AC_MSG_RESULT($VDOIMPORT) + ################################################################################ dnl -- Enable blkdeactivate AC_MSG_CHECKING(whether to install blkdeactivate) @@ -1666,6 +1676,9 @@ USRSBINDIR="$(eval echo $(eval echo $usrsbindir))" FSADM_PATH="$SBINDIR/fsadm" AC_DEFINE_UNQUOTED(FSADM_PATH, ["$FSADM_PATH"], [Path to fsadm binary.]) +VDOIMPORT_PATH="$SBINDIR/vdoimport" +AC_DEFINE_UNQUOTED(VDOIMPORT_PATH, ["$VDOIMPORT_PATH"], [Path to vdoimport binary.]) + ################################################################################ dnl -- dmeventd pidfile and executable path if test "$BUILD_DMEVENTD" = yes; then @@ -1902,6 +1915,8 @@ AC_SUBST(SILENT_RULES) AC_SUBST(USRSBINDIR) AC_SUBST(VALGRIND_POOL) AC_SUBST(VDO) +AC_SUBST(VDOIMPORT) +AC_SUBST(VDOIMPORT_PATH) AC_SUBST(VDO_FORMAT_CMD) AC_SUBST(VDO_INCLUDE) AC_SUBST(VDO_LIB) diff --git a/include/configure.h.in b/include/configure.h.in index 08f696167..55d18bd4f 100644 --- a/include/configure.h.in +++ b/include/configure.h.in @@ -687,6 +687,9 @@ /* Enable a valgrind aware build of pool */ #undef VALGRIND_POOL +/* Path to vdoimport binary. */ +#undef VDOIMPORT_PATH + /* The path to 'vdoformat', if available. */ #undef VDO_FORMAT_CMD diff --git a/man/Makefile.in b/man/Makefile.in index 29afc776b..d60a92cde 100644 --- a/man/Makefile.in +++ b/man/Makefile.in @@ -23,6 +23,7 @@ else endif FSADMMAN = fsadm.8 +VDOIMPORTMAN = vdoimport.8 BLKDEACTIVATEMAN = blkdeactivate.8 DMEVENTDMAN = dmeventd.8 DMFILEMAPDMAN = dmfilemapd.8 @@ -50,7 +51,7 @@ MAN8SYSTEMD_GENERATORS=lvm2-activation-generator.8 ifeq (,$(findstring $(MAKECMDGOALS), distclean all_man install_all_man)) MAN7 += lvmcache.7 lvmthin.7 lvmvdo.7 - MAN8+=$(FSADMMAN) $(LVMPOLLDMAN) $(LVMLOCKDMAN) $(LVMDBUSDMAN) + MAN8+=$(FSADMMAN) $(LVMPOLLDMAN) $(LVMLOCKDMAN) $(LVMDBUSDMAN) $(VDOIMPORTMAN) MAN8DM+=$(BLKDEACTIVATEMAN) $(DMEVENTDMAN) $(DMFILEMAPDMAN) MAN8CLUSTER+=$(CMIRRORDMAN) else @@ -58,6 +59,10 @@ else MAN8+=$(FSADMMAN) endif + ifeq ("@VDOIMPORT@", "yes") + MAN8+=$(VDOIMPORTMAN) + endif + ifeq ("@BUILD_LVMDBUSD@", "yes") MAN8+=$(LVMDBUSDMAN) endif diff --git a/scripts/Makefile.in b/scripts/Makefile.in index e8f674282..1fe88ca90 100644 --- a/scripts/Makefile.in +++ b/scripts/Makefile.in @@ -31,6 +31,10 @@ ifeq ("@FSADM@", "yes") LVM_SCRIPTS += fsadm.sh endif +ifeq ("@VDOIMPORT@", "yes") + LVM_SCRIPTS += vdoimport.sh +endif + ifeq ("@BLKDEACTIVATE@", "yes") DM_SCRIPTS += blkdeactivate.sh endif diff --git a/scripts/vdoimport.sh b/scripts/vdoimport.sh new file mode 100755 index 000000000..ef9659103 --- /dev/null +++ b/scripts/vdoimport.sh @@ -0,0 +1,376 @@ +#!/bin/bash +# +# Copyright (C) 2021 Red Hat, Inc. All rights reserved. +# +# This file is part of LVM2. +# +# 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 +# +# Author: Zdenek Kabelac <zkabelac at redhat.com> +# +# Script for converting VDO volumes to lvm2 VDO LVs +# +# Needed utilities: +# lvm, dmsetup, +# vdo, vdo2lvm, +# grep, awk, sed, blockdev, readlink, mkdir +# +# Conversion is using 'vdo convert' support from VDO manager to move +# existing VDO header by 2M which makes space to place in PV header +# and VG metadata area, and then create VDOPOOL LV and VDO LV in such VG. +# + +set -euE -o pipefail + +TOOL=vdoimport + +_SAVEPATH=$PATH +PATH="/sbin:/usr/sbin:/bin:/usr/sbin:$PATH" + +# user may override lvm location by setting LVM_BINARY +LVM=${LVM_BINARY:-lvm} +VDO=${VDO_BINARY:-vdo} +VDOCONF=${VDOCONF:-} +BLOCKDEV="blockdev" +READLINK="readlink" +READLINK_E="-e" +MKDIR="mkdir" + +TEMPDIR="${TMPDIR:-/tmp}/${TOOL}_${RANDOM}$$" +DM_DEV_DIR="${DM_DEV_DIR:-/dev}" + +DRY=0 +VERB="" +FORCE="" +YES="" + +# default name for converted VG and its VDO LV +NAME="vdovg/vdolvol" + +# help message +tool_usage() { + echo "${TOOL}: Utility to convert VDO volume to VDO LV." + echo + echo " ${TOOL} [options] <vdo_device_path>" + echo + echo " Options:" + echo " -f | --force Bypass sanity checks" + echo " -h | --help Show this help message" + echo " -n | --name Specifies VG/LV name for converted VDO volume" + echo " -v | --verbose Be verbose" + echo " -y | --yes Answer \"yes\" at any prompts" + echo " --dry-run Print commands without running them" + + exit +} + +verbose() { + test -z "$VERB" || echo "$TOOL:" "$@" +} + +# Support multi-line error messages +error() { + for i in "$@" ; do + echo "$TOOL: $i" >&2 + done + cleanup 1 +} + +dry() { + if [ "$DRY" -ne 0 ]; then + verbose "Dry execution" "$@" + return 0 + fi + verbose "Executing" "$@" + "$@" +} + +cleanup() { + trap '' 2 + + rm -rf "$TEMPDIR" + # error exit status for break + exit "${1:-1}" +} + +get_enabled_value_() { + case "$1" in + enabled) echo "1" ;; + *) echo "0" ;; + esac +} + +get_kb_size_with_unit_() { + case "$1" in + *[kK]) echo $(( ${1%[kK]} )) ;; + *[mM]) echo $(( ${1%[mM]} * 1024 )) ;; + *[gG]) echo $(( ${1%[gG]} * 1024 * 1024 )) ;; + *[tT]) echo $(( ${1%[tT]} * 1024 * 1024 * 1024 )) ;; + *[pP]) echo $(( ${1%[pP]} * 1024 * 1024 * 1024 * 1024 )) ;; + esac +} + +get_mb_size_with_unit_() { + case "$1" in + *[mM]) echo $(( ${1%[mM]} )) ;; + *[gG]) echo $(( ${1%[gG]} * 1024 )) ;; + *[tT]) echo $(( ${1%[tT]} * 1024 * 1024 )) ;; + *[pP]) echo $(( ${1%[pP]} * 1024 * 1024 * 1024 )) ;; + esac +} + +# Figure out largest possible extent size usable for VG +# $1 physical size +# $2 logical size +get_largest_extent_size_() { + local max=4 + local i + local d + + for i in 8 16 32 64 128 256 512 1024 2048 4096 ; do + d=$(( $1 / i )) + test $(( d * i )) -eq "$1" || break + d=$(( $2 / i )) + test $(( d * i )) -eq "$2" || break + max=$i + done + echo "$max" +} + +# detect LV on the given device +# dereference device name if it is symbolic link +detect_lv_() { + local DEVICE=$1 + local MAJOR + local MINOR + local SYSVOLUME + local MAJORMINOR + + DEVICE=${1/#"${DM_DEV_DIR}/"/} + DEVICE=$("$READLINK" $READLINK_E "$DM_DEV_DIR/$DEVICE") + test -n "$DEVICE" || error "Cannot get readlink \"$1\"." + RDEVICE=$DEVICE + case "$RDEVICE" in + # hardcoded /dev since udev does not create these entries elsewhere + /dev/dm-[0-9]*) + read -r <"/sys/block/${RDEVICE#/dev/}/dm/name" SYSVOLUME 2>&1 && DEVICE="$DM_DEV_DIR/mapper/$SYSVOLUME" + read -r <"/sys/block/${RDEVICE#/dev/}/dev" MAJORMINOR 2>&1 || error "Cannot get major:minor for \"$DEVICE\"." + MAJOR=${MAJORMINOR%%:*} + MINOR=${MAJORMINOR##*:} + ;; + *) + STAT=$(stat --format "MAJOR=\$((0x%t)) MINOR=\$((0x%T))" "$RDEVICE") + test -n "$STAT" || error "Cannot get major:minor for \"$DEVICE\"." + eval "$STAT" + ;; + esac + + eval "$(dmsetup info -c -j "$MAJOR" -m "$MINOR" -o uuid,name --noheadings --nameprefixes --separator ' ')" +} + +# parse yaml config files into 'prefix_yaml_part_names=("value")' strings +parse_yaml_() { + local yaml_file=$1 + local prefix=$2 + local s + local w + local fs + + s='[[:space:]]*' + w='[a-zA-Z0-9_.-]*' + fs="$(echo @|tr @ '\034')" + + ( + sed -ne '/^--/s|--||g; s|\"|\\\"|g; s/[[:space:]]*$//g;' \ + -e 's/\$/\\\$/g' \ + -e "/#.*[\"\']/!s| #.*||g; /^#/s|#.*||g;" \ + -e "s|^$$s$$$w$$s:$s\"$.*$\"$s\$|\1$fs\2$fs\3|p" \ + -e "s|^$$s$$$w$${s}[:-]$s$.*$$s\$|\1$fs\2$fs\3|p" | + + awk -F"$fs" '{ + indent = length($1)/2; + if (length($2) == 0) { conj[indent]="+";} else {conj[indent]="";} + vname[indent] = $2; + for (i in vname) {if (i > indent) {delete vname[i]}} + if (length($3) > 0) { + vn=""; for (i=0; i<indent; i++) {vn=(vn)(vname[i])("_")} + printf("%s%s%s%s=(\"%s\")\n", "'"$prefix"'",vn, $2, conj[indent-1], $3); + } + }' | + + sed -e 's/_=/+=/g' | + + awk 'BEGIN { + FS="="; + OFS="=" + } + /(-|\.).*=/ { + gsub("-|\\.", "_", $1) + } + { print }' + ) < "$yaml_file" +} + +# convert existing VDO volume into lvm2 volume +convert2lvm_() { + local DEVICE=$1 + local VGNAME=${NAME%/*} + local LVNAME=${NAME#*/} + local VDONAME + local TRVDONAME + local EXTENTSZ + local IS_LV=1 + + DM_UUID="" + detect_lv_ "$DEVICE" + case "$DM_UUID" in + LVM-*) eval "$(dmsetup splitname --nameprefixes --noheadings --separator ' ' "$DM_NAME")" + if [ -z "$VGNAME" ] || [ "$VGNAME" = "$LVNAME" ] ; then + VGNAME=$DM_VG_NAME + elif test "$VGNAME" != "$DM_VG_NAME" ; then + error "Volume group name \"$VGNAME\" does not match name \"$DM_VG_NAME\" for device \"$DEVICE\"." + fi + ;; + *) IS_LV=0 + # Check $VGNANE does not already exists + "$LVM" vgs "$VGNAME" && error "Cannot use already existing volume group name \"$VGNAME\"." + ;; + esac + + verbose "Checked whether device $1 is already LV ($IS_LV)." + + "$MKDIR" -p -m 0000 "$TEMPDIR" || error "Failed to create $TEMPDIR." + + verbose "Getting YAML VDO configuration." + "$VDO" printConfigFile $VDOCONF >"$TEMPDIR/vdoconf.yml" + + VDONAME=$(awk -v DNAME="$DEVICE" '/.*VDOService$/ {VNAME=substr($1, 0, length($1) - 1)} /[[:space:]]*device:/ { if ($2 ~ DNAME) {print VNAME}}' "$TEMPDIR/vdoconf.yml") + TRVDONAME=$(echo "$VDONAME" | tr '-' '_') + + # When VDO volume is 'active', check it's not mounted/being used + eval "$(dmsetup info -c -o open "$VDONAME" --noheadings --nameprefixes || true)" + test "${DM_OPEN:-0}" -eq 0 || error "Cannot converted VDO volume \"$VDONAME\" which is in use!" + + #parse_yaml_ "$TEMPDIR/vdoconf.yml" _ + eval "$(parse_yaml_ "$TEMPDIR/vdoconf.yml" _ | grep "$TRVDONAME" | sed -e "s/_config_vdos_$TRVDONAME/vdo/g")" + + vdo_logicalSize=$(get_kb_size_with_unit_ "$vdo_logicalSize") + vdo_physicalSize=$(get_kb_size_with_unit_ "$vdo_physicalSize") + + verbose "Going to convert physical sized VDO device $vdo_physicalSize KiB." + verbose "With logical volume of size $vdo_logicalSize KiB." + + PARAMS=$(cat <<EOF +allocation { + vdo_use_compression = $(get_enabled_value_ "$vdo_compression") + vdo_use_deduplication = $(get_enabled_value_ "$vdo_deduplication") + vdo_use_metadata_hints=1 + vdo_minimum_io_size = $vdo_logicalBlockSize + vdo_block_map_cache_size_mb = $(get_mb_size_with_unit_ "$vdo_blockMapCacheSize") + vdo_block_map_period = $vdo_blockMapPeriod + vdo_check_point_frequency = $vdo_indexCfreq + vdo_use_sparse_index = $(get_enabled_value_ "$vdo_indexSparse") + vdo_index_memory_size_mb = $(awk "BEGIN {print $vdo_indexMemory * 1024}") + vdo_slab_size_mb = $(get_mb_size_with_unit_ "$vdo_blockMapCacheSize") + vdo_ack_threads = $vdo_ackThreads + vdo_bio_threads = $vdo_bioThreads + vdo_bio_rotation = $vdo_bioRotationInterval + vdo_cpu_threads = $vdo_cpuThreads + vdo_hash_zone_threads = $vdo_hashZoneThreads + vdo_logical_threads = $vdo_logicalThreads + vdo_physical_threads = $vdo_physicalThreads + vdo_write_policy = $vdo_writePolicy + vdo_max_discard = $(( $(get_kb_size_with_unit_ "$vdo_maxDiscardSize") * 1024 )) + vdo_pool_header_size = 0 +} +EOF +) + verbose "VDO conversion paramaters: $PARAMS" + + verbose "Stopping VDO volume." + dry "$VDO" stop $VDOCONF --name "$VDONAME" + + if [ "$IS_LV" = "0" ]; then + verbose "Moving VDO header by 2MiB." + dry "$VDO" convert $VDOCONF --force --name "$VDONAME" + + dry "$LVM" pvcreate $YES --dataalignment 2M "$DEVICE" || { + error "Creation of PV on \"$DEVICE\" failed, while VDO header has been already moved!" + } + + # Obtain free space in this new PV + # after 'vdo convert/vdo2lvm' call there is +2M free space at the front of the device + case "$DRY" in + 0) pvfree=$("$LVM" pvs -o devsize --units b --nosuffix --noheadings "$DEVICE") ;; + *) pvfree=$("$BLOCKDEV" --getsize64 "$DEVICE") ;; + esac + + pvfree=$(( pvfree / 1024 - 2048 )) # to KiB + else + pvfree=$("$LVM" lvs -o size --units b --nosuffix --noheadings "$VGNAME/$LVNAME") + pvfree=$(( pvfree / 1024 )) # to KiB + fi + + # select largest possible extent size that can exactly express both sizes + EXTENTSZ=$(get_largest_extent_size_ "$pvfree" "$vdo_logicalSize") + + if [ "$IS_LV" = "0" ]; then + verbose "Creating VG \"${NAME%/*}\" with extent size $EXTENTSZ KiB." + dry "$LVM" vgcreate $YES $VERB -s "${EXTENTSZ}k" "$VGNAME" "$DEVICE" || { + error "Creation of VG \"$VGNAME\" failed, while VDO header has been already moved!" + } + + verbose "Creating VDO pool data LV from all extents in volume group $VGNAME." + dry "$LVM" lvcreate -Zn -Wn $YES $VERB -l100%VG -n "${LVNAME}_vpool" "$VGNAME" + else + # validate existing VG extent_size can express virtual VDO size + vg_extent_size=$("$LVM" vgs -o vg_extent_size --units b --nosuffix --noheadings "$VGNAME" || true) + vg_extent_size=$(( vg_extent_size / 1024 )) + + test "$vg_extent_size" -le "$EXTENTSZ" || { + error "Please vgchange extent_size to at most $EXTENTSZ KiB or extend and align virtual size on $vg_extent_size KiB." + } + verbose "Renaming existing LV to be used as _vdata volume for VDO pool LV." + dry "$LVM" lvrename $YES $VERB "$VGNAME/$LVNAME" "$VGNAME/${LVNAME}_vpool" || { + error "Rename of LV \"$VGNAME/$LVNAME\" failed, while VDO header has been already moved!" + } + fi + + verbose "Converting to VDO pool." + dry "$LVM" lvconvert $YES $VERB $FORCE --config "$PARAMS" -Zn -V "${vdo_logicalSize}k" -n "$LVNAME" --type vdo-pool "$VGNAME/${LVNAME}_vpool" + + rm -fr "$TEMPDIR" +} + +############################# +# start point of this script +# - parsing parameters +############################# +trap "cleanup 2" 2 + +test "$#" -eq 0 && tool_usage + +while [ "$#" -ne 0 ] +do + case "$1" in + "") ;; + "-f"|"--force" ) FORCE="-f" ;; + "-h"|"--help" ) tool_usage ;; + "-n"|"--name" ) shift; NAME=$1 ;; + "-v"|"--verbose") VERB="-v" ;; + "-y"|"--yes" ) YES="-y" ;; + "--dry-run" ) DRY="1" ;; + "-*") error "Wrong argument \"$1\". (see: $TOOL --help)" ;; + *) DEVICENAME=$1 ;; # device name does not start with '-' + esac + shift +done + +# do conversion +convert2lvm_ "$DEVICENAME" diff --git a/spec/packages.inc b/spec/packages.inc index 71aaeea7a..bf693a4ce 100644 --- a/spec/packages.inc +++ b/spec/packages.inc @@ -70,6 +70,7 @@ fi %{_sbindir}/vgdisplay %{_sbindir}/vgexport %{_sbindir}/vgextend +%{_sbindir}/vdoimport %{_sbindir}/vgimport %{_sbindir}/vgimportclone %{_sbindir}/vgimportdevices diff --git a/test/Makefile.in b/test/Makefile.in index 5f35c30b8..093054087 100644 --- a/test/Makefile.in +++ b/test/Makefile.in @@ -368,6 +368,7 @@ LIB = $(addprefix lib/, $(LIB_SECURETEST) $(LIB_DMSECURETEST) $(LIB_SHARED) $(LI $(Q) $(LN_S) -f $(abs_top_srcdir)/conf/lvmdbusd.profile lib/ $(Q) $(LN_S) -f $(abs_top_srcdir)/conf/thin-performance.profile lib/ $(Q) $(LN_S) -f $(abs_top_srcdir)/scripts/fsadm.sh lib/fsadm + $(Q) $(LN_S) -f $(abs_top_srcdir)/scripts/vdoimport.sh lib/vdoimport @test "$(srcdir)" = . || \ for i in $(LIB_LVMLOCKD_CONF) $(LIB_MKE2FS_CONF); do \ test -n "$(Q)" || echo "$(LN_S) -f $(abs_top_srcdir)/test/lib/$$i lib/"; \ diff --git a/test/shell/vdo-convert.sh b/test/shell/vdo-convert.sh new file mode 100644 index 000000000..538147b8d --- /dev/null +++ b/test/shell/vdo-convert.sh @@ -0,0 +1,110 @@ +#!/usr/bin/env bash + +# Copyright (C) 2021 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 conversion of VDO volumes made by vdo manager into VDO LV. + + +SKIP_WITH_LVMPOLLD=1 + +. lib/inittest + +# Use local for this test vdo configuratoin +VDOCONF="-f vdotestconf.yml" +#VDOCONF="" +export VDOCONF +VDONAME="${PREFIX}-TESTVDO" + +# VDO automatically starts dmeventd +aux prepare_dmeventd + +# +# Main +# +which vdo || skip +which mkfs.ext4 || skip +export MKE2FS_CONFIG="$TESTDIR/lib/mke2fs.conf" + +aux have_vdo 6 2 0 || skip + +aux prepare_devs 2 10000 + +aux extend_filter_LVMTEST + + +# +# Check conversion of VDO volume made on some LV +# +# In this case we do not need to move any VDO headers. +# +vgcreate $vg "$dev1" + +lvcreate -L5G -n $lv1 $vg + +vdo create $VDOCONF --name "$VDONAME" --device="$DM_DEV_DIR/$vg/$lv1" --vdoLogicalSize=10G + +mkfs -E nodiscard "$DM_DEV_DIR/mapper/$VDONAME" + +# Different VG name fails +not vdoimport -y -v --name $vg1/$lv1 "$DM_DEV_DIR/$vg/$lv1" + +# Try just dry run and observe logging +vdoimport --dry-run -y -v --name $lv1 "$DM_DEV_DIR/$vg/$lv1" + +vdoimport -y --name $lv1 "$DM_DEV_DIR/$vg/$lv1" + +# ATM needed - since we do not call 'vdo convert' in this case +vdo remove $VDOCONF --force --name "$VDONAME" || true + +vgremove -f $vg + +aux wipefs_a "$dev1" + +# prepare 'unused' $vg2 +vgcreate $vg2 "$dev2" + +# +# Check conversion of VDO volume on non-LV device +# +vdo create $VDOCONF --name "$VDONAME" --device="$dev1" --vdoLogicalSize=31G + +mkfs -E nodiscard "$DM_DEV_DIR/mapper/$VDONAME" + +# Fail with an already existing volume group $vg2 +not vdoimport --dry-run -y -v --name $vg2/$lv1 "$dev1" |& tee err +grep "already existing volume group" err + +# User can also convert already stopped VDO volume +vdo stop $VDOCONF --name "$VDONAME" + +vdoimport -y -v --name $vg/$lv1 "$dev1" + +fsck -n "$DM_DEV_DIR/$vg/$lv1" + +vgremove -f $vg + + +# +# Try once again with different vgname/lvname and sizes +# +aux teardown_devs +aux prepare_devs 1 23456 + +vdo create $VDOCONF --name "$VDONAME" --device="$dev1" --vdoLogicalSize=23G + +mkfs -E nodiscard "$DM_DEV_DIR/mapper/$VDONAME" + +vdoimport -y -v --name $vg1/$lv2 "$dev1" + +fsck -n "$DM_DEV_DIR/$vg1/$lv2" + +vgremove -f $vg1 +

1 0

main - configure: updates
by Zdenek Kabelac 09 Jul '21

09 Jul '21

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=3a92d633a5a23377aa0d5… Commit: 3a92d633a5a23377aa0d5d4ed5a01db8ca14cf69 Parent: d5a06f9a7df5a43b2e2311db62ff8d3011217d74 Author: Zdenek Kabelac <zkabelac(a)redhat.com> AuthorDate: Fri Jul 9 14:42:26 2021 +0200 Committer: Zdenek Kabelac <zkabelac(a)redhat.com> CommitterDate: Fri Jul 9 14:57:59 2021 +0200 configure: updates --- configure | 26 ++++++++++++++------------ include/configure.h.in | 6 +++--- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/configure b/configure index e2299ee91..3a9888528 100755 --- a/configure +++ b/configure @@ -747,7 +747,6 @@ BUILD_DMFILEMAPD BUILD_LOCKDDLM_CONTROL BUILD_LOCKDDLM BUILD_LOCKDSANLOCK -BUILD_LOCKDIDM BUILD_LVMLOCKD BUILD_LVMPOLLD BUILD_LVMDBUSD @@ -773,18 +772,18 @@ UDEV_LIBS UDEV_CFLAGS SYSTEMD_LIBS SYSTEMD_CFLAGS -BLKID_LIBS -BLKID_CFLAGS NOTIFY_DBUS_LIBS NOTIFY_DBUS_CFLAGS +BLKID_LIBS +BLKID_CFLAGS +LOCKD_IDM_LIBS +LOCKD_IDM_CFLAGS LOCKD_DLM_CONTROL_LIBS LOCKD_DLM_CONTROL_CFLAGS LOCKD_DLM_LIBS LOCKD_DLM_CFLAGS LOCKD_SANLOCK_LIBS LOCKD_SANLOCK_CFLAGS -LOCKD_IDM_LIBS -LOCKD_IDM_CFLAGS VALGRIND_LIBS VALGRIND_CFLAGS GENPNG @@ -1025,10 +1024,10 @@ LOCKD_DLM_CONTROL_CFLAGS LOCKD_DLM_CONTROL_LIBS LOCKD_IDM_CFLAGS LOCKD_IDM_LIBS -NOTIFY_DBUS_CFLAGS -NOTIFY_DBUS_LIBS BLKID_CFLAGS BLKID_LIBS +NOTIFY_DBUS_CFLAGS +NOTIFY_DBUS_LIBS SYSTEMD_CFLAGS SYSTEMD_LIBS UDEV_CFLAGS @@ -1843,13 +1842,13 @@ Some influential environment variables: C compiler flags for LOCKD_IDM, overriding pkg-config LOCKD_IDM_LIBS linker flags for LOCKD_IDM, overriding pkg-config + BLKID_CFLAGS + C compiler flags for BLKID, overriding pkg-config + BLKID_LIBS linker flags for BLKID, overriding pkg-config NOTIFY_DBUS_CFLAGS C compiler flags for NOTIFY_DBUS, overriding pkg-config NOTIFY_DBUS_LIBS linker flags for NOTIFY_DBUS, overriding pkg-config - BLKID_CFLAGS - C compiler flags for BLKID, overriding pkg-config - BLKID_LIBS linker flags for BLKID, overriding pkg-config SYSTEMD_CFLAGS C compiler flags for SYSTEMD, overriding pkg-config SYSTEMD_LIBS @@ -11260,7 +11259,7 @@ fi if test $pkg_failed = yes; then - { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 $as_echo "no" >&6; } if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then @@ -11278,7 +11277,7 @@ fi $bailout elif test $pkg_failed = untried; then - { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 $as_echo "no" >&6; } $bailout else @@ -11286,6 +11285,7 @@ else LOCKD_IDM_LIBS=$pkg_cv_LOCKD_IDM_LIBS { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 $as_echo "yes" >&6; } + HAVE_LOCKD_IDM=yes fi pkg_failed=no @@ -14350,6 +14350,8 @@ _ACEOF + + diff --git a/include/configure.h.in b/include/configure.h.in index bcb282660..08f696167 100644 --- a/include/configure.h.in +++ b/include/configure.h.in @@ -558,12 +558,12 @@ /* Define to 1 to include code that uses lvmlockd dlm option. */ #undef LOCKDDLM_SUPPORT -/* Define to 1 to include code that uses lvmlockd sanlock option. */ -#undef LOCKDSANLOCK_SUPPORT - /* Define to 1 to include code that uses lvmlockd IDM option. */ #undef LOCKDIDM_SUPPORT +/* Define to 1 to include code that uses lvmlockd sanlock option. */ +#undef LOCKDSANLOCK_SUPPORT + /* Define to 1 if `lstat' dereferences a symlink specified with a trailing slash. */ #undef LSTAT_FOLLOWS_SLASHED_SYMLINK

1 0

main - pvscan: skip indexing devices used by LVs
by David Teigland 06 Jul '21

06 Jul '21

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=d5a06f9a7df5a43b2e231… Commit: d5a06f9a7df5a43b2e2311db62ff8d3011217d74 Parent: b876dbfc245b8fe0a4c433b39d214652455c3f9e Author: David Teigland <teigland(a)redhat.com> AuthorDate: Thu Jul 1 17:25:43 2021 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Tue Jul 6 10:18:07 2021 -0500 pvscan: skip indexing devices used by LVs dev_cache_index_devs() is taking a large amount of time when there are many PVs. The index keeps track of devices that are currently in use by active LVs. This info is used to print warnings for users in some limited cases. The checks/warnings that are enabled by the index are not needed by pvscan --cache, so disable it in this case. This may be expanded to other cases in future commits. dev_cache_index_devs should also be improved in another commit to avoid the extreme delays with many devices. --- lib/commands/toolcontext.c | 1 + lib/commands/toolcontext.h | 1 + lib/device/dev-cache.c | 9 +++++---- lib/device/dev-cache.h | 2 +- lib/metadata/metadata.c | 3 ++- tools/lvmdevices.c | 2 +- tools/pvscan.c | 2 ++ 7 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index e2be89d0f..b295a20ef 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -1608,6 +1608,7 @@ struct cmd_context *create_toolcontext(unsigned is_clvmd, cmd->handles_missing_pvs = 0; cmd->handles_unknown_segments = 0; cmd->hosttags = 0; + cmd->check_devs_used = 1; dm_list_init(&cmd->arg_value_groups); dm_list_init(&cmd->formats); dm_list_init(&cmd->segtypes); diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h index a47b7d760..34808ce46 100644 --- a/lib/commands/toolcontext.h +++ b/lib/commands/toolcontext.h @@ -192,6 +192,7 @@ struct cmd_context { unsigned filter_nodata_only:1; /* only use filters that do not require data from the dev */ unsigned run_by_dmeventd:1; /* command is being run by dmeventd */ unsigned sysinit:1; /* --sysinit is used */ + unsigned check_devs_used:1; /* check devs used by LVs */ /* * Devices and filtering. diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c index d7fa93fd6..bb0d0f211 100644 --- a/lib/device/dev-cache.c +++ b/lib/device/dev-cache.c @@ -1139,7 +1139,7 @@ static int _insert(const char *path, const struct stat *info, return 1; } -void dev_cache_scan(void) +void dev_cache_scan(struct cmd_context *cmd) { log_debug_devs("Creating list of system devices."); @@ -1147,7 +1147,8 @@ void dev_cache_scan(void) _insert_dirs(&_cache.dirs); - (void) dev_cache_index_devs(); + if (cmd->check_devs_used) + (void) dev_cache_index_devs(); } int dev_cache_has_scanned(void) @@ -1583,7 +1584,7 @@ struct device *dev_cache_get_by_devt(struct cmd_context *cmd, dev_t dev, struct log_debug_devs("Device num not found in dev_cache repeat dev_cache_scan for %d:%d", (int)MAJOR(dev), (int)MINOR(dev)); - dev_cache_scan(); + dev_cache_scan(cmd); d = (struct device *) btree_lookup(_cache.devices, (uint32_t) dev); if (!d) @@ -1953,7 +1954,7 @@ int setup_devices(struct cmd_context *cmd) * This will not open or read any devices, but may look at sysfs properties. * This list of devs comes from looking /dev entries, or from asking libudev. */ - dev_cache_scan(); + dev_cache_scan(cmd); /* * Match entries from cmd->use_devices with device structs in dev-cache. diff --git a/lib/device/dev-cache.h b/lib/device/dev-cache.h index 9b7e39d33..c3f5eddda 100644 --- a/lib/device/dev-cache.h +++ b/lib/device/dev-cache.h @@ -48,7 +48,7 @@ int dev_cache_exit(void); */ int dev_cache_check_for_open_devices(void); -void dev_cache_scan(void); +void dev_cache_scan(struct cmd_context *cmd); int dev_cache_has_scanned(void); int dev_cache_add_dir(const char *path); diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index d5b28a58f..0cbf67869 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -5132,7 +5132,8 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const if (!check_pv_dev_sizes(vg)) log_warn("WARNING: One or more devices used as PVs in VG %s have changed sizes.", vg->name); - _check_devs_used_correspond_with_vg(vg); + if (cmd->check_devs_used) + _check_devs_used_correspond_with_vg(vg); if (!_access_vg_lock_type(cmd, vg, lockd_state, &failure)) { /* Either FAILED_LOCK_TYPE or FAILED_LOCK_MODE were set. */ diff --git a/tools/lvmdevices.c b/tools/lvmdevices.c index 3448bdd14..5117277a0 100644 --- a/tools/lvmdevices.c +++ b/tools/lvmdevices.c @@ -171,7 +171,7 @@ int lvmdevices(struct cmd_context *cmd, int argc, char **argv) log_error("Failed to read the devices file."); return ECMD_FAILED; } - dev_cache_scan(); + dev_cache_scan(cmd); device_ids_match(cmd); if (arg_is_set(cmd, check_ARG) || arg_is_set(cmd, update_ARG)) { diff --git a/tools/pvscan.c b/tools/pvscan.c index f8d27372b..464501ad5 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -1627,6 +1627,8 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv) dm_list_init(&complete_vgnames); + cmd->check_devs_used = 0; + if (do_activate && !find_config_tree_bool(cmd, global_event_activation_CFG, NULL)) { log_verbose("Ignoring pvscan --cache -aay because event_activation is disabled.");

1 0

main - scan: move metadata vgname check
by David Teigland 06 Jul '21

06 Jul '21

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=b876dbfc245b8fe0a4c43… Commit: b876dbfc245b8fe0a4c433b39d214652455c3f9e Parent: e035e323508383fdcd9df0ef036d276a9c9909ab Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Jun 29 12:40:03 2021 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Tue Jul 6 10:10:23 2021 -0500 scan: move metadata vgname check There have been two separate checks for metadata validity: first that the metadata text begins with a valid VG name, and second the checksum of the metadata text. These happen in different places, which means there have been two separate error paths for invalid metadata. This also causes large metadata to be read in multiple parts, the first part is read just to check the vgname, and then remaining parts are read later when the full metadata is needed. This patch moves the vg name verification so it's done just before the checksum verification, which results in a single error path for invalid metadata, and causes the entire metadata to be read together rather that in parts from different parts of the code. --- lib/config/config.c | 25 +++++++++++- lib/format_text/format-text.c | 94 +++++-------------------------------------- 2 files changed, 33 insertions(+), 86 deletions(-) diff --git a/lib/config/config.c b/lib/config/config.c index 485063a3d..25a87c983 100644 --- a/lib/config/config.c +++ b/lib/config/config.c @@ -501,6 +501,9 @@ int config_file_read_fd(struct dm_config_tree *cft, struct device *dev, dev_io_r checksum_fn_t checksum_fn, uint32_t checksum, int checksum_only, int no_dup_node_check) { + char namebuf[NAME_LEN + 1] __attribute__((aligned(8))); + int namelen = 0; + int bad_name = 0; char *fb, *fe; int r = 0; int sz, use_plain_read = 1; @@ -548,6 +551,23 @@ int config_file_read_fd(struct dm_config_tree *cft, struct device *dev, dev_io_r fb = buf; + if (!(dev->flags & DEV_REGULAR)) { + memcpy(namebuf, buf, NAME_LEN); + + while (namebuf[namelen] && !isspace(namebuf[namelen]) && namebuf[namelen] != '{' && namelen < (NAME_LEN - 1)) + namelen++; + namebuf[namelen] = '\0'; + + /* + * Check that the text metadata begins with a valid name. + */ + if (!validate_name(namebuf)) { + log_warn("WARNING: Metadata location on %s at offset %llu begins with invalid name.", + dev_name(dev), (unsigned long long)offset); + bad_name = 1; + } + } + /* * The checksum passed in is the checksum from the mda_header * preceding this metadata. They should always match. @@ -557,10 +577,13 @@ int config_file_read_fd(struct dm_config_tree *cft, struct device *dev, dev_io_r if (checksum_fn && checksum != (checksum_fn(checksum_fn(INITIAL_CRC, (const uint8_t *)fb, size), (const uint8_t *)(fb + size), size2))) { - log_error("%s: Checksum error at offset %" PRIu64, dev_name(dev), (uint64_t) offset); + log_warn("WARNING: Checksum error on %s at offset %llu.", dev_name(dev), (unsigned long long)offset); goto out; } + if (bad_name) + goto out; + if (!checksum_only) { fe = fb + size + size2; if (no_dup_node_check) { diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index dd550a6eb..26e55cec4 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -296,24 +296,11 @@ static struct raw_locn *_read_metadata_location_vg(struct cmd_context *cmd, const char *vgname, int *precommitted) { - size_t len; - char vgnamebuf[NAME_LEN + 2] __attribute__((aligned(8))); struct raw_locn *rlocn, *rlocn_precommitted; - struct lvmcache_info *info; - struct lvmcache_vgsummary vgsummary_orphan = { - .vgname = FMT_TEXT_ORPHAN_VG_NAME, - }; - int rlocn_was_ignored; - - dm_list_init(&vgsummary_orphan.pvsummaries); - - memcpy(&vgsummary_orphan.vgid, FMT_TEXT_ORPHAN_VG_NAME, sizeof(FMT_TEXT_ORPHAN_VG_NAME)); rlocn = mdah->raw_locns; /* Slot 0 */ rlocn_precommitted = rlocn + 1; /* Slot 1 */ - rlocn_was_ignored = rlocn_is_ignored(rlocn); - /* Should we use precommitted metadata? */ if (*precommitted && rlocn_precommitted->size && (rlocn_precommitted->offset != rlocn->offset)) { @@ -338,42 +325,7 @@ static struct raw_locn *_read_metadata_location_vg(struct cmd_context *cmd, if (!rlocn->offset && !rlocn->size) return NULL; - /* - * Don't try to check existing metadata - * if given vgname is an empty string. - */ - if (!vgname || !*vgname) - return rlocn; - - /* - * If live rlocn has ignored flag, data will be out-of-date so skip further checks. - */ - if (rlocn_was_ignored) - return rlocn; - - /* - * Verify that the VG metadata pointed to by the rlocn - * begins with a valid vgname. - */ - memset(vgnamebuf, 0, sizeof(vgnamebuf)); - - if (!dev_read_bytes(dev_area->dev, dev_area->start + rlocn->offset, NAME_LEN, vgnamebuf)) - goto fail; - - if (!strncmp(vgnamebuf, vgname, len = strlen(vgname)) && - (isspace(vgnamebuf[len]) || vgnamebuf[len] == '{')) - return rlocn; - fail: - log_error("Metadata on %s at %llu has wrong VG name \"%s\" expected %s.", - dev_name(dev_area->dev), - (unsigned long long)(dev_area->start + rlocn->offset), - vgnamebuf, vgname); - - if ((info = lvmcache_info_from_pvid(dev_area->dev->pvid, dev_area->dev, 0)) && - !lvmcache_update_vgname_and_id(cmd, info, &vgsummary_orphan)) - stack; - - return NULL; + return rlocn; } /* @@ -487,9 +439,13 @@ static struct volume_group *_vg_read_raw_area(struct cmd_context *cmd, rlocn->checksum, &when, &desc); - if (!vg) { - /* FIXME: detect and handle errors, and distinguish from the optimization - that skips parsing the metadata which also returns NULL. */ + if (!vg && !*use_previous_vg) { + log_warn("WARNING: failed to read metadata text on %s at %llu size %llu for VG %s.", + dev_name(area->dev), + (unsigned long long)(area->start + rlocn->offset), + (unsigned long long)rlocn->size, + vgname); + return NULL; } log_debug_metadata("Found metadata on %s at %llu size %llu for VG %s", @@ -498,7 +454,7 @@ static struct volume_group *_vg_read_raw_area(struct cmd_context *cmd, (unsigned long long)rlocn->size, vgname); - if (vg && precommitted) + if (precommitted) vg->status |= PRECOMMITTED; out: @@ -1533,8 +1489,6 @@ int read_metadata_location_summary(const struct format_type *fmt, { struct raw_locn *rlocn; uint32_t wrap = 0; - unsigned int len = 0; - char namebuf[NAME_LEN + 1] __attribute__((aligned(8))); uint64_t max_size; if (!mdah) { @@ -1563,36 +1517,6 @@ int read_metadata_location_summary(const struct format_type *fmt, return 0; } - /* - * This code reading the start of the metadata area and verifying that - * it looks like a vgname can be removed. The checksum verifies it. - */ - log_debug_metadata("Reading metadata_vgname summary from %s at %llu", - dev_name(dev_area->dev), - (unsigned long long)(dev_area->start + rlocn->offset)); - - memset(namebuf, 0, sizeof(namebuf)); - - if (!dev_read_bytes(dev_area->dev, dev_area->start + rlocn->offset, NAME_LEN, namebuf)) - stack; - - while (namebuf[len] && !isspace(namebuf[len]) && namebuf[len] != '{' && - len < (NAME_LEN - 1)) - len++; - - namebuf[len] = '\0'; - - /* - * Check that the text metadata in the circular buffer begins with a - * valid vg name. - */ - if (!validate_name(namebuf)) { - log_warn("WARNING: Metadata location on %s at %llu begins with invalid VG name.", - dev_name(dev_area->dev), - (unsigned long long)(dev_area->start + rlocn->offset)); - return 0; - } - /* * This function is used to read the vg summary during label scan. * Save the text start location and checksum during scan. After the VG

1 0

main - scan: retry reading metadata on error
by David Teigland 06 Jul '21

06 Jul '21

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=e035e323508383fdcd9df… Commit: e035e323508383fdcd9df0ef036d276a9c9909ab Parent: d89942d157e49168d069a65f77a6ecb2c155b3fb Author: David Teigland <teigland(a)redhat.com> AuthorDate: Mon Jun 28 18:10:47 2021 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Tue Jul 6 10:10:23 2021 -0500 scan: retry reading metadata on error If label_scan encounters bad vg metadata, invalidate bcache data for the device and reread the mda_header and metadata text back to back. With concurrent commands modifying large metadata, it's possible that the entire metadata area can be rewritten in the time between a command reading the mda_header and reading the metadata text that the header points to. Since the label_scan is just assembling an initial overview of devices, it doesn't use locking to serialize with other commands that may be modifying the vg metadata at the same time. --- lib/format_text/format-text.c | 8 ++++++++ lib/format_text/text_label.c | 35 +++++++++++++++++++++++++++++++++++ lib/label/label.c | 5 +++++ lib/label/label.h | 1 + 4 files changed, 49 insertions(+) diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index 64ad4677c..dd550a6eb 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -1563,6 +1563,14 @@ int read_metadata_location_summary(const struct format_type *fmt, return 0; } + /* + * This code reading the start of the metadata area and verifying that + * it looks like a vgname can be removed. The checksum verifies it. + */ + log_debug_metadata("Reading metadata_vgname summary from %s at %llu", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset)); + memset(namebuf, 0, sizeof(namebuf)); if (!dev_read_bytes(dev_area->dev, dev_area->start + rlocn->offset, NAME_LEN, namebuf)) diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c index be5195039..7bc7a1ed3 100644 --- a/lib/format_text/text_label.c +++ b/lib/format_text/text_label.c @@ -327,6 +327,9 @@ static int _read_mda_header_and_metadata(const struct format_type *fmt, { struct mda_context *mdac = (struct mda_context *) mda->metadata_locn; struct mda_header *mdah; + int retries = 0; + + retry: if (!(mdah = raw_read_mda_header(fmt, &mdac->area, (mda->mda_num == 1), 0, bad_fields))) { log_warn("WARNING: bad metadata header on %s at %llu.", @@ -354,6 +357,38 @@ static int _read_mda_header_and_metadata(const struct format_type *fmt, if (vgsummary->zero_offset) return 1; + /* + * This code is used by label_scan to get a summary of the + * VG metadata that will be properly read later by vg_read. + * The initial read of this device during label_scan + * populates bcache with the first 128K of data from the + * device. That block of data contains the mda_header + * (at 4k) but will often not include the metadata text, + * which is often located further into the metadata area + * (beyond the 128K block saved in bcache.) + * So read_metadata_location_summary will usually get the + * mda_header from bcache which was read initially, and + * then it will often need to do a new disk read to get + * the actual metadata text that the mda_header points to. + * Since there is no locking around label_scan, it's + * possible (but very rare) that the entire metadata area + * can be rewritten by other commands between the time that + * this command read the mda_header and the time that it + * reads the metadata text. This means the expected metadata + * text isn't found, and an error is returned here. + * To handle this, invalidate all data in bcache for this + * device and reread the mda_header and metadata text back to + * back, so inconsistency is less likely (without locking + * there's no guarantee, e.g. if the command is blocked + * somehow between the two reads.) + */ + if (!retries) { + log_print("Retrying metadata scan."); + retries++; + dev_invalidate(mdac->area.dev); + goto retry; + } + log_warn("WARNING: bad metadata text on %s in mda%d", dev_name(mdac->area.dev), mda->mda_num); *bad_fields |= BAD_MDA_TEXT; diff --git a/lib/label/label.c b/lib/label/label.c index 50107edc8..ac84b6293 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -1704,6 +1704,11 @@ bool dev_invalidate_bytes(struct device *dev, uint64_t start, size_t len) return bcache_invalidate_bytes(scan_bcache, dev->bcache_di, start, len); } +void dev_invalidate(struct device *dev) +{ + bcache_invalidate_di(scan_bcache, dev->bcache_di); +} + bool dev_write_zeros(struct device *dev, uint64_t start, size_t len) { return dev_set_bytes(dev, start, len, 0); diff --git a/lib/label/label.h b/lib/label/label.h index fcdc309ac..32ceebc34 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -130,6 +130,7 @@ bool dev_write_bytes(struct device *dev, uint64_t start, size_t len, void *data) bool dev_write_zeros(struct device *dev, uint64_t start, size_t len); bool dev_set_bytes(struct device *dev, uint64_t start, size_t len, uint8_t val); bool dev_invalidate_bytes(struct device *dev, uint64_t start, size_t len); +void dev_invalidate(struct device *dev); void dev_set_last_byte(struct device *dev, uint64_t offset); void dev_unset_last_byte(struct device *dev);

1 0

main - scan: don't hold bcache block during scan
by David Teigland 06 Jul '21

06 Jul '21

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=d89942d157e49168d069a… Commit: d89942d157e49168d069a65f77a6ecb2c155b3fb Parent: a47e20a0929bc0d399851c398e7e3d06e97daf65 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Mon Jun 28 17:09:09 2021 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Tue Jul 6 10:10:23 2021 -0500 scan: don't hold bcache block during scan This allows data from the bcache block to be invalidated and reread if needed. --- lib/label/label.c | 71 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/lib/label/label.c b/lib/label/label.c index cfb9ebc80..50107edc8 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -264,9 +264,8 @@ static bool _in_bcache(struct device *dev) } static struct labeller *_find_lvm_header(struct device *dev, - char *scan_buf, - uint32_t scan_buf_sectors, - char *label_buf, + char *headers_buf, + int headers_buf_size, uint64_t *label_sector, uint64_t block_sector, uint64_t start_sector) @@ -277,24 +276,13 @@ static struct labeller *_find_lvm_header(struct device *dev, uint64_t sector; int found = 0; - /* - * Find which sector in scan_buf starts with a valid label, - * and copy it into label_buf. - */ - for (sector = start_sector; sector < start_sector + LABEL_SCAN_SECTORS; sector += LABEL_SIZE >> SECTOR_SHIFT) { - /* - * The scan_buf passed in is a bcache block, which is - * BCACHE_BLOCK_SIZE_IN_SECTORS large. So if start_sector is - * one of the last couple sectors in that buffer, we need to - * break early. - */ - if (sector >= scan_buf_sectors) + if ((sector * 512) >= headers_buf_size) break; - lh = (struct label_header *) (scan_buf + (sector << SECTOR_SHIFT)); + lh = (struct label_header *) (headers_buf + (sector << SECTOR_SHIFT)); if (!memcmp(lh->id, LABEL_ID, sizeof(lh->id))) { if (found) { @@ -332,7 +320,6 @@ static struct labeller *_find_lvm_header(struct device *dev, labeller_ret = li->l; found = 1; - memcpy(label_buf, lh, LABEL_SIZE); if (label_sector) *label_sector = block_sector + sector; break; @@ -354,13 +341,13 @@ static struct labeller *_find_lvm_header(struct device *dev, * are performed in the processing functions to get that data. */ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, - struct device *dev, struct block *bb, + struct device *dev, char *headers_buf, int headers_buf_size, uint64_t block_sector, uint64_t start_sector, int *is_lvm_device) { - char label_buf[LABEL_SIZE] __attribute__((aligned(8))); + char *label_buf; struct labeller *labeller; - uint64_t sector = 0; + uint64_t label_sector = 0; int is_duplicate = 0; int ret = 0; @@ -396,13 +383,9 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, } /* - * Finds the data sector containing the label and copies into label_buf. - * label_buf: struct label_header + struct pv_header + struct pv_header_extension - * - * FIXME: we don't need to copy one sector from bb->data into label_buf, - * we can just point label_buf at one sector in ld->buf. + * Finds the data sector containing the label. */ - if (!(labeller = _find_lvm_header(dev, bb->data, BCACHE_BLOCK_SIZE_IN_SECTORS, label_buf, &sector, block_sector, start_sector))) { + if (!(labeller = _find_lvm_header(dev, headers_buf, headers_buf_size, &label_sector, block_sector, start_sector))) { /* * Non-PVs exit here @@ -427,6 +410,7 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, dev->flags |= DEV_SCAN_FOUND_LABEL; *is_lvm_device = 1; + label_buf = headers_buf + (label_sector * 512); /* * This is the point where the scanning code dives into the rest of @@ -436,7 +420,7 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, * info/vginfo structs. That lvmcache info is used later when the * command wants to read the VG to do something to it. */ - ret = labeller->ops->read(cmd, labeller, dev, label_buf, sector, &is_duplicate); + ret = labeller->ops->read(cmd, labeller, dev, label_buf, label_sector, &is_duplicate); if (!ret) { if (is_duplicate) { @@ -670,9 +654,12 @@ static void _invalidate_di(struct bcache *cache, int di) * its info is removed from lvmcache. */ +#define HEADERS_BUF_SIZE 4096 + static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs, int want_other_devs, int *failed) { + char headers_buf[HEADERS_BUF_SIZE]; struct dm_list wait_devs; struct dm_list done_devs; struct dm_list reopen_devs; @@ -738,14 +725,35 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, scan_read_errors++; scan_failed_count++; lvmcache_del_dev(devl->dev); + if (bb) + bcache_put(bb); } else { - log_debug_devs("Processing data from device %s %d:%d di %d block %p", + /* copy the first 4k from bb that will contain label_header */ + + memcpy(headers_buf, bb->data, HEADERS_BUF_SIZE); + + /* + * "put" the bcache block before process_block because + * processing metadata may need to invalidate and reread + * metadata that's covered by bb. invalidate/reread is + * not allowed while bb is held. The functions for + * filtering and scanning metadata for this device use + * dev_read_bytes(), which will generally grab the + * bcache block/data that we're putting here. Since + * we're doing put, it's possible but not likely that + * bcache could drop the block before dev_read_bytes() + * uses it again, in which case bcache will reread it + * from disk for dev_read_bytes(). + */ + bcache_put(bb); + + log_debug_devs("Processing data from device %s %d:%d di %d", dev_name(devl->dev), (int)MAJOR(devl->dev->dev), (int)MINOR(devl->dev->dev), - devl->dev->bcache_di, (void *)bb); + devl->dev->bcache_di); - ret = _process_block(cmd, f, devl->dev, bb, 0, 0, &is_lvm_device); + ret = _process_block(cmd, f, devl->dev, headers_buf, sizeof(headers_buf), 0, 0, &is_lvm_device); if (!ret && is_lvm_device) { log_debug_devs("Scan failed to process %s", dev_name(devl->dev)); @@ -754,9 +762,6 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, } } - if (bb) - bcache_put(bb); - /* * Keep the bcache block of lvm devices we have processed so * that the vg_read phase can reuse it. If bcache failed to

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits July 2021