man/udevadm.xml

@@ -791,7 +791,7 @@
       </title>
 
       <para><command>udevadm lock</command> takes an (advisory) exclusive lock(s) on a block device (or
-      multiple thereof), as per <ulink url="https://systemd.io/BLOCK_DEVICE_LOCKING">Locking Block Device
+      multiple therof), as per <ulink url="https://systemd.io/BLOCK_DEVICE_LOCKING">Locking Block Device
       Access</ulink> and invokes a program with the lock(s) taken. When the invoked program exits the lock(s)
       are automatically released.</para>
 

shell-completion/bash/udevadm

@@ -65,10 +65,9 @@ _udevadm() {
         [TEST]='-a --action -N --resolve-names'
         [TEST_BUILTIN]='-a --action'
         [WAIT]='-t --timeout --initialized=no --removed --settle'
-        [LOCK]='-t --timeout -d --device -b --backing -p --print'
     )
 
-    local verbs=(info trigger settle control monitor test-builtin test wait lock)
+    local verbs=(info trigger settle control monitor test-builtin test wait)
     local builtins=(blkid btrfs hwdb input_id keyboard kmod net_id net_setup_link path_id usb_id uaccess)
 
     for ((i=0; i < COMP_CWORD; i++)); do
@@ -266,24 +265,6 @@ _udevadm() {
             fi
             ;;
 
-        'lock')
-            if __contains_word "$prev" ${OPTS[LOCK]}; then
-                case $prev in
-                    *)
-                        comps=''
-                        ;;
-                esac
-                COMPREPLY=( $(compgen -W '$comps' -- "$cur") )
-                return 0
-            fi
-
-            if [[ $cur = -* ]]; then
-                comps="${OPTS[COMMON]} ${OPTS[LOCK]}"
-            else
-                comps=''
-            fi
-            ;;
-
         *)
             comps=${VERBS[*]}
             ;;

shell-completion/zsh/_udevadm

@@ -115,16 +115,6 @@ _udevadm_wait(){
         '*::devpath:_files -P /dev/ -W /dev'
 }
 
-(( $+functions[_udevadm_lock] )) ||
-_udevadm_lock(){
-    _arguments \
-        '--timeout=[Maximum number of seconds to wait for the devices being locked.]' \
-        '--device=[Block device to lock.]' \
-        '--backing=[File whose backing block device to lock.]' \
-        '--print[Only show which block device the lock would be taken on.]' \
-        '--help[Print help text.]'
-}
-
 (( $+functions[_udevadm_mounts] )) ||
 _udevadm_mounts(){
     local dev_tmp dpath_tmp mp_tmp mline
@@ -154,8 +144,6 @@ _udevadm_commands(){
         'monitor:listen to kernel and udev events'
         'test:test an event run'
         'test-builtin:test a built-in command'
-        'wait:wait for devices or device symlinks being created'
-        'lock:lock a block device and run a comand'
     )
 
     if ((CURRENT == 1)); then

src/udev/udevadm-lock.c

@@ -41,7 +41,7 @@ static int help(void) {
 
         printf("%s [OPTIONS...] COMMAND\n"
                "%s [OPTIONS...] --print\n"
-               "\n%sLock a block device and run a command.%s\n\n"
+               "\n%sLock a block device and run a comand.%s\n\n"
                "  -h --help            Print this message\n"
                "  -V --version         Print version of the program\n"
                "  -d --device=DEVICE   Block device to lock\n"
@@ -247,7 +247,7 @@ static int lock_device(
 
                         /* flock() doesn't support a time-out. Let's fake one then. The traditional way to do
                          * this is via alarm()/setitimer()/timer_create(), but that's racy, given that the
-                         * SIGALRM might already fire between the alarm() and the flock() in which case the
+                         * SIGALRM might aleady fire between the alarm() and the flock() in which case the
                          * flock() is never cancelled and we lock up (this is a short time window, but with
                          * short timeouts on a loaded machine we might run into it, who knows?). Let's
                          * instead do the lock out-of-process: fork off a child that does the locking, and

test/units/testsuite-64.sh

@@ -41,6 +41,132 @@ helper_check_device_symlinks() {(
     done < <(find "${paths[@]}" -type l)
 )}
 
+# Wrapper around `helper_wait_for_lvm_activate()` and `helper_wait_for_pvscan()`
+# functions to cover differences between pre and post lvm 2.03.14, which introduced
+# a new way of vgroup autoactivation
+# See: https://sourceware.org/git/?p=lvm2.git;a=commit;h=67722b312390cdab29c076c912e14bd739c5c0f6
+# Arguments:
+#   $1 - device path (for helper_wait_for_pvscan())
+#   $2 - volume group name (for helper_wait_for_lvm_activate())
+#   $3 - number of retries (default: 10)
+helper_wait_for_vgroup() {
+    local dev="${1:?}"
+    local vgroup="${2:?}"
+    local ntries="${3:-10}"
+
+    if ! systemctl -q list-unit-files lvm2-pvscan@.service >/dev/null; then
+        helper_wait_for_lvm_activate "$vgroup" "$ntries"
+    else
+        helper_wait_for_pvscan "$dev" "$ntries"
+    fi
+}
+
+# Wait for the lvm-activate-$vgroup.service of a specific $vgroup to finish
+# Arguments:
+#   $1 - volume group name
+#   $2 - number of retries (default: 10)
+helper_wait_for_lvm_activate() {
+    local vgroup="${1:?}"
+    local ntries="${2:-10}"
+    local i lvm_activate_svc
+
+    lvm_activate_svc="lvm-activate-$vgroup.service"
+    for ((i = 0; i < ntries; i++)); do
+        if systemctl -q is-active "$lvm_activate_svc"; then
+            # Since the service is started via `systemd-run --no-block`, we need
+            # to wait until it finishes, otherwise we might continue while
+            # `vgchange` is still running
+            if [[ "$(systemctl show -P SubState "$lvm_activate_svc")" == exited ]]; then
+                return 0
+            fi
+        else
+            # Since lvm 2.03.15 the lvm-activate transient unit no longer remains
+            # after finishing, so we have to treat non-existent units as a success
+            # as well
+            # See: https://sourceware.org/git/?p=lvm2.git;a=commit;h=fbd8b0cf43dc67f51f86f060dce748f446985855
+            if [[ "$(systemctl show -P LoadState "$lvm_activate_svc")" == not-found ]]; then
+                return 0
+            fi
+        fi
+
+        sleep .5
+    done
+
+    return 1
+}
+
+# Wait for the lvm2-pvscan@.service of a specific device to finish
+# Arguments:
+#   $1 - device path
+#   $2 - number of retries (default: 10)
+helper_wait_for_pvscan() {
+    local dev="${1:?}"
+    local ntries="${2:-10}"
+    local MAJOR MINOR i pvscan_svc real_dev
+
+    # Sanity check we got a valid block device (or a symlink to it)
+    real_dev="$(readlink -f "$dev")"
+    if [[ ! -b "$real_dev" ]]; then
+        echo >&2 "ERROR: '$dev ($real_dev) is not a valid block device'"
+        return 1
+    fi
+
+    # Get major and minor numbers from the udev database
+    # (udevadm returns MAJOR= and MINOR= expressions, so let's pull them into
+    # the current environment via `source` for easier parsing)
+    #
+    # shellcheck source=/dev/null
+    source <(udevadm info -q property "$real_dev" | grep -E "(MAJOR|MINOR)=")
+    # Sanity check if we got correct major and minor numbers
+    test -e "/sys/dev/block/$MAJOR:$MINOR/"
+
+    # Wait n_tries*0.5 seconds until the respective lvm2-pvscan service becomes
+    # active (i.e. it got executed and finished)
+    pvscan_svc="lvm2-pvscan@$MAJOR:$MINOR.service"
+    for ((i = 0; i < ntries; i++)); do
+        ! systemctl -q is-active "$pvscan_svc" || return 0
+        sleep .5
+    done
+
+    return 1
+}
+
+# Generate an `flock` command line for a device list
+#
+# This is useful mainly for mkfs.btrfs, which doesn't hold the lock on each
+# device for the entire duration of mkfs.btrfs, causing weird races between udev
+# and mkfs.btrfs. This function creates an array of chained flock calls to take
+# the lock of all involved devices, which can be then used in combination with
+# mkfs.btrfs to mitigate the issue.
+#
+# For example, calling:
+#   helper_generate_flock_cmdline my_array /dev/loop1 /dev/loop2 /dev/loop3
+#
+# will result in "${my_array[@]}" containing:
+#   flock -x /dev/loop1 flock -x /dev/loop2 flock -x /dev/loop3
+#
+# Note: the array will be CLEARED before the first assignment
+#
+# Arguments:
+#   $1    - NAME of an array in which the commands/argument will be stored
+#   $2-$n - path to devices
+helper_generate_flock_cmdline() {
+    # Create a name reference to the array passed as the first argument
+    # (requires bash 4.3+)
+    local -n cmd_array="${1:?}"
+    shift
+
+    if [[ $# -eq 0 ]]; then
+        echo >&2 "Missing argument(s): device path(s)"
+        return 1
+    fi
+
+    cmd_array=()
+    for dev in "$@"; do
+        cmd_array+=("flock" "-x" "$dev")
+    done
+}
+
 testcase_megasas2_basic() {
     lsblk -S
     [[ "$(lsblk --scsi --noheadings | wc -l)" -ge 128 ]]
@@ -109,7 +235,9 @@ EOF
         "/dev/disk/by-label/failover_vol"
         "/dev/disk/by-uuid/deadbeef-dead-dead-beef-111111111111"
     )
-    udevadm wait --settle --timeout=30 "${part_links[@]}"
+    for link in "${part_links[@]}"; do
+        test -e "$link"
+    done
 
     # Choose a random symlink to the failover data partition each time, for
     # a better coverage
@@ -140,7 +268,9 @@ EOF
         echo -n "$expected" >"$mpoint/test"
 
         # Make sure all symlinks are still valid
-        udevadm wait --settle --timeout=30 "${part_links[@]}"
+        for link in "${part_links[@]}"; do
+            test -e "$link"
+        done
     done
 
     multipath -l "$path"
@@ -181,7 +311,7 @@ EOF
         sfdisk -q -X gpt "$blockdev" <"$partscript"
 
         if ((i % 10 == 0)); then
-            udevadm wait --settle --timeout=30 "$blockdev"
+            udevadm settle
             helper_check_device_symlinks
         fi
     done
@@ -209,19 +339,27 @@ testcase_lvm_basic() {
     lvm lvcreate -y -L 4M "$vgroup" -n mypart1
     lvm lvcreate -y -L 8M "$vgroup" -n mypart2
     lvm lvs
-    udevadm wait --settle --timeout=30 "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2"
+    udevadm settle
+    test -e "/dev/$vgroup/mypart1"
+    test -e "/dev/$vgroup/mypart2"
     mkfs.ext4 -L mylvpart1 "/dev/$vgroup/mypart1"
-    udevadm wait --settle --timeout=30 "/dev/disk/by-label/mylvpart1"
+    udevadm settle
+    test -e "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
 
     # Disable the VG and check symlinks...
     lvm vgchange -an "$vgroup"
-    udevadm wait --settle --timeout=30 --removed "/dev/$vgroup" "/dev/disk/by-label/mylvpart1"
+    udevadm settle
+    test ! -e "/dev/$vgroup"
+    test ! -e "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk"
 
     # reenable the VG and check the symlinks again if all LVs are properly activated
     lvm vgchange -ay "$vgroup"
-    udevadm wait --settle --timeout=30 "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2" "/dev/disk/by-label/mylvpart1"
+    udevadm settle
+    test -e "/dev/$vgroup/mypart1"
+    test -e "/dev/$vgroup/mypart2"
+    test -e "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
 
     # Same as above, but now with more "stress"
@@ -230,15 +368,19 @@ testcase_lvm_basic() {
         lvm vgchange -ay "$vgroup"
 
         if ((i % 5 == 0)); then
-            udevadm wait --settle --timeout=30 "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2" "/dev/disk/by-label/mylvpart1"
+            udevadm settle
+            test -e "/dev/$vgroup/mypart1"
+            test -e "/dev/$vgroup/mypart2"
+            test -e "/dev/disk/by-label/mylvpart1"
             helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
         fi
     done
 
     # Remove the first LV
     lvm lvremove -y "$vgroup/mypart1"
-    udevadm wait --settle --timeout=30 --removed "/dev/$vgroup/mypart1"
+    udevadm settle
-    udevadm wait --timeout=0 "/dev/$vgroup/mypart2"
+    test ! -e "/dev/$vgroup/mypart1"
+    test -e "/dev/$vgroup/mypart2"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
 
     # Create & remove LVs in a loop, i.e. with more "stress"
@@ -254,8 +396,9 @@ testcase_lvm_basic() {
         # 3) On every 4th iteration settle udev and check if all partitions are
         #    indeed gone, and if all symlinks are still valid
         if ((i % 4 == 0)); then
+            udevadm settle
             for part in {0..15}; do
-                udevadm wait --settle --timeout=30 --removed "/dev/$vgroup/looppart$part"
+                test ! -e "/dev/$vgroup/looppart$part"
             done
             helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
         fi
@@ -264,6 +407,7 @@ testcase_lvm_basic() {
 
 testcase_btrfs_basic() {
     local dev_stub i label mpoint uuid
+    local flock_cmd=()
     local devices=(
         /dev/disk/by-id/ata-foobar_deadbeefbtrfs{0..3}
     )
@@ -273,9 +417,12 @@ testcase_btrfs_basic() {
     echo "Single device: default settings"
     uuid="deadbeef-dead-dead-beef-000000000000"
     label="btrfs_root"
-    udevadm lock --device="${devices[0]}" mkfs.btrfs -L "$label" -U "$uuid" "${devices[0]}"
+    helper_generate_flock_cmdline flock_cmd "${devices[0]}"
-    udevadm wait --settle --timeout=30 "${devices[0]}" "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
+    "${flock_cmd[@]}" mkfs.btrfs -L "$label" -U "$uuid" "${devices[0]}"
+    udevadm settle
     btrfs filesystem show
+    test -e "/dev/disk/by-uuid/$uuid"
+    test -e "/dev/disk/by-label/$label"
     helper_check_device_symlinks
 
     echo "Multiple devices: using partitions, data: single, metadata: raid1"
@@ -289,25 +436,26 @@ name="diskpart2", size=85M
 name="diskpart3", size=85M
 name="diskpart4", size=85M
 EOF
-    udevadm wait --settle --timeout=30 /dev/disk/by-partlabel/diskpart{1..4}
+    udevadm settle
-    udevadm lock --device="${devices[0]}" mkfs.btrfs -d single -m raid1 -L "$label" -U "$uuid" /dev/disk/by-partlabel/diskpart{1..4}
+    # We need to flock only the device itself, not its partitions
-    udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
+    helper_generate_flock_cmdline flock_cmd "${devices[0]}"
+    "${flock_cmd[@]}" mkfs.btrfs -d single -m raid1 -L "$label" -U "$uuid" /dev/disk/by-partlabel/diskpart{1..4}
+    udevadm settle
     btrfs filesystem show
+    test -e "/dev/disk/by-uuid/$uuid"
+    test -e "/dev/disk/by-label/$label"
     helper_check_device_symlinks
     wipefs -a -f "${devices[0]}"
-    udevadm wait --settle --timeout=30 --removed /dev/disk/by-partlabel/diskpart{1..4}
 
     echo "Multiple devices: using disks, data: raid10, metadata: raid10, mixed mode"
     uuid="deadbeef-dead-dead-beef-000000000002"
     label="btrfs_mdisk"
-    udevadm lock \
+    helper_generate_flock_cmdline flock_cmd "${devices[@]}"
-            --device=/dev/disk/by-id/ata-foobar_deadbeefbtrfs0 \
+    "${flock_cmd[@]}" mkfs.btrfs -M -d raid10 -m raid10 -L "$label" -U "$uuid" "${devices[@]}"
-            --device=/dev/disk/by-id/ata-foobar_deadbeefbtrfs1 \
+    udevadm settle
-            --device=/dev/disk/by-id/ata-foobar_deadbeefbtrfs2 \
-            --device=/dev/disk/by-id/ata-foobar_deadbeefbtrfs3 \
-            mkfs.btrfs -M -d raid10 -m raid10 -L "$label" -U "$uuid" "${devices[@]}"
-    udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
+    test -e "/dev/disk/by-uuid/$uuid"
+    test -e "/dev/disk/by-label/$label"
     helper_check_device_symlinks
 
     echo "Multiple devices: using LUKS encrypted disks, data: raid1, metadata: raid1, mixed mode"
@@ -327,7 +475,9 @@ EOF
         cryptsetup luksFormat -q \
             --use-urandom --pbkdf pbkdf2 --pbkdf-force-iterations 1000 \
             --uuid "deadbeef-dead-dead-beef-11111111111$i" --label "encdisk$i" "${devices[$i]}" /etc/btrfs_keyfile
-        udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/deadbeef-dead-dead-beef-11111111111$i" "/dev/disk/by-label/encdisk$i"
+        udevadm settle
+        test -e "/dev/disk/by-uuid/deadbeef-dead-dead-beef-11111111111$i"
+        test -e "/dev/disk/by-label/encdisk$i"
         # Add the device into /etc/crypttab, reload systemd, and then activate
         # the device so we can create a filesystem on it later
         echo "encbtrfs$i UUID=deadbeef-dead-dead-beef-11111111111$i /etc/btrfs_keyfile luks,noearly" >>/etc/crypttab
@@ -338,14 +488,12 @@ EOF
     # Check if we have all necessary DM devices
     ls -l /dev/mapper/encbtrfs{0..3}
     # Create a multi-device btrfs filesystem on the LUKS devices
-    udevadm lock \
+    helper_generate_flock_cmdline flock_cmd /dev/mapper/encbtrfs{0..3}
-            --device=/dev/mapper/encbtrfs0 \
+    "${flock_cmd[@]}" mkfs.btrfs -M -d raid1 -m raid1 -L "$label" -U "$uuid" /dev/mapper/encbtrfs{0..3}
-            --device=/dev/mapper/encbtrfs1 \
+    udevadm settle
-            --device=/dev/mapper/encbtrfs2 \
-            --device=/dev/mapper/encbtrfs3 \
-            mkfs.btrfs -M -d raid1 -m raid1 -L "$label" -U "$uuid" /dev/mapper/encbtrfs{0..3}
-    udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
+    test -e "/dev/disk/by-uuid/$uuid"
+    test -e "/dev/disk/by-label/$label"
     helper_check_device_symlinks
     # Mount it and write some data to it we can compare later
     mount -t btrfs /dev/mapper/encbtrfs0 "$mpoint"
@@ -353,7 +501,7 @@ EOF
     # "Deconstruct" the btrfs device and check if we're in a sane state (symlink-wise)
     umount "$mpoint"
     systemctl stop systemd-cryptsetup@encbtrfs{0..3}
-    udevadm wait --settle --timeout=30 --removed "/dev/disk/by-uuid/$uuid"
+    test ! -e "/dev/disk/by-uuid/$uuid"
     helper_check_device_symlinks
     # Add the mount point to /etc/fstab and check if the device can be put together
     # automagically. The source device is the DM name of the first LUKS device
@@ -368,8 +516,9 @@ EOF
     # Start the corresponding mount unit and check if the btrfs device was reconstructed
     # correctly
     systemctl start "${mpoint##*/}.mount"
-    udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
+    test -e "/dev/disk/by-uuid/$uuid"
+    test -e "/dev/disk/by-label/$label"
     helper_check_device_symlinks
     grep "hello there" "$mpoint/test"
     # Cleanup
@@ -432,7 +581,7 @@ testcase_iscsi_lvm() {
     expected_symlinks=()
     # Use the first device as it's configured with larger capacity
     mkfs.ext4 -L iscsi_store "${devices[0]}"
-    udevadm wait --settle --timeout=30 "${devices[0]}"
+    udevadm settle
     mount "${devices[0]}" "$mpoint"
     for i in {1..4}; do
         dd if=/dev/zero of="$mpoint/lun$i.img" bs=1M count=32
@@ -464,9 +613,12 @@ testcase_iscsi_lvm() {
     lvm lvcreate -y -L 4M "$vgroup" -n mypart1
     lvm lvcreate -y -L 8M "$vgroup" -n mypart2
     lvm lvs
-    udevadm wait --settle --timeout=30 "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2"
+    udevadm settle
+    test -e "/dev/$vgroup/mypart1"
+    test -e "/dev/$vgroup/mypart2"
     mkfs.ext4 -L mylvpart1 "/dev/$vgroup/mypart1"
-    udevadm wait --settle --timeout=30 "/dev/disk/by-label/mylvpart1"
+    udevadm settle
+    test -e "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
     # Disconnect the iSCSI devices and check all the symlinks
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --logout
@@ -479,7 +631,13 @@ testcase_iscsi_lvm() {
     # Reconnect the iSCSI devices and check if everything get detected correctly
     iscsiadm --mode discoverydb --type sendtargets --portal "$target_ip" --discover
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --login
-    udevadm wait --settle --timeout=30 "${expected_symlinks[@]}" "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2" "/dev/disk/by-label/mylvpart1"
+    udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
+    for link in "${expected_symlinks[@]}"; do
+        helper_wait_for_vgroup "$link" "$vgroup"
+    done
+    test -e "/dev/$vgroup/mypart1"
+    test -e "/dev/$vgroup/mypart2"
+    test -e "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
     # Cleanup
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --logout
@@ -504,7 +662,9 @@ testcase_long_sysfs_path() {
     stat /sys/block/vda
     readlink -f /sys/block/vda/dev
 
-    udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
+    for link in "${expected_symlinks[@]}"; do
+        test -e "$link"
+    done
 
     # Try to mount the data partition manually (using its label)
     mpoint="$(mktemp -d /logsysfsXXX)"