From d62ce351c632e86e34f6afcfab484b1177f9ba22 Mon Sep 17 00:00:00 2001 From: Daniel Vogel Date: Wed, 7 Oct 2026 11:40:12 +0200 Subject: [PATCH 1/3] kvm: fix SharedMountPoint heartbeat when pool path is a subdirectory of a mount kvmsmpheartbeat.sh required the pool path itself to be a mount point (mountpoint -q). A SharedMountPoint pool is "a file system path local to each server" and is commonly a subdirectory of a mounted clustered or parallel filesystem (e.g. /data01/vol01 on IBM Storage Scale mounted at /data01). For such pools every heartbeat write failed. - add is_on_mounted_fs(): the path must reside on a mounted filesystem other than / (findmnt -T, fallback df -P). Use it for both the initial check and the /proc/mounts check in front of deleteVMs, so relaxing the first check cannot lead to deleteVMs (kill -9) being run on every write. - check_hbLog: 'expr' exits with 1 when the result is 0, so a heartbeat read in the same second it was written reported the host as DEAD. Validate the timestamp and use shell arithmetic instead. - remove the temporary heartbeat file when the script is interrupted (e.g. killed on timeout). Fixes #14326 --- scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh | 42 ++++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh b/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh index b102a1a866bb..015e41798872 100755 --- a/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh +++ b/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh @@ -80,12 +80,23 @@ if [ ! -d "$MountPoint" ]; then exit 1 fi -# If the 'mountpoint' utility is available, ensure this is an actual mount -if command -v mountpoint >/dev/null 2>&1; then - if ! mountpoint -q "$MountPoint"; then - echo "Mount point is not a mounted filesystem: $MountPoint" >&2 - exit 1 +# Returns 0 if the given path resides on a mounted filesystem other than the +# root filesystem. A SharedMountPoint path does not need to be a mount point +# itself, it may be a subdirectory of a mounted (e.g. clustered) filesystem. +is_on_mounted_fs() { + local target + if command -v findmnt >/dev/null 2>&1; then + target=$(findmnt -n -o TARGET -T "$1" 2>/dev/null) + else + target=$(df -P "$1" 2>/dev/null | awk 'NR==2 {print $6}') fi + [ -n "$target" ] && [ "$target" != "/" ] +} + +# Ensure the path is on a mounted filesystem (not the local root filesystem) +if ! is_on_mounted_fs "$MountPoint"; then + echo "Mount point is not on a mounted filesystem: $MountPoint" >&2 + exit 1 fi # Ensure the mount point is writable @@ -112,8 +123,8 @@ deleteVMs() { done } -#checking is there the mount point present under $MountPoint? -if grep -q "^[^ ]\+ $MountPoint " /proc/mounts +#checking is the filesystem of $MountPoint mounted? +if is_on_mounted_fs "$MountPoint" then # mount exists; nothing to do here; keep for compatibility with original flow : @@ -146,6 +157,9 @@ write_hbLog() { timestamp=$(date +%s) # Write atomically to avoid partial writes (write to tmp then mv) tmpfile="${hbFile}.$$" + # remove the temporary file if the script is interrupted (e.g. on timeout) + trap 'rm -f "$tmpfile"' EXIT + trap 'exit 1' INT TERM printf "%s\n" "$timestamp" > "$tmpfile" 2>/dev/null if [ $? -ne 0 ]; then printf "Failed to write heartbeat to $tmpfile" >&2 @@ -168,12 +182,14 @@ check_hbLog() { hb_diff=999998 return 1 fi - diff=`expr $now - $hb 2>/dev/null` - if [ $? -ne 0 ] - then - hb_diff=999997 - return 1 - fi + # note: 'expr' exits with 1 when the result is 0, so use shell arithmetic + case "$hb" in + ''|*[!0-9]*) + hb_diff=999997 + return 1 + ;; + esac + diff=$((now - hb)) if [ -z "$interval" ]; then # if no interval provided, consider 0 as success if [ $diff -gt 0 ]; then From acd5ba45c8f1736f953cb4dca99e9055cd70b88f Mon Sep 17 00:00:00 2001 From: Daniel Vogel Date: Wed, 7 Oct 2026 13:24:53 +0200 Subject: [PATCH 2/3] kvm: drop unreachable deleteVMs branch from kvmsmpheartbeat.sh The initial check already exits when the path is not on a mounted filesystem, so the deleteVMs branch could not be reached. Remove the branch together with the now unused deleteVMs function. This keeps the behaviour of 4.22.1/4.23: there the earlier 'mountpoint -q' check exited first as well, so deleteVMs never ran in practice. Unlike kvmheartbeat.sh (NFS), which kills VMs only after it remounted a lost NFS share, a SharedMountPoint cannot be remounted by the script. Killing VMs on a failed local check is left to the existing HA/fencing logic. --- scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh | 32 -------------------- 1 file changed, 32 deletions(-) diff --git a/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh b/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh index 015e41798872..5a8e4e8d564c 100755 --- a/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh +++ b/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh @@ -104,38 +104,6 @@ if [ ! -w "$MountPoint" ]; then echo "Mount point is not writable: $MountPoint" >&2 exit 1 fi -#delete VMs on this mountpoint (best-effort) -deleteVMs() { - local mountPoint=$1 - # ensure it ends with a single trailing slash - mountPoint="${mountPoint%/}/" - - vmPids=$(ps aux | grep qemu | grep "$mountPoint" | awk '{print $2}' 2> /dev/null) - - if [ -z "$vmPids" ] - then - return - fi - - for pid in $vmPids - do - kill -9 $pid &> /dev/null - done -} - -#checking is the filesystem of $MountPoint mounted? -if is_on_mounted_fs "$MountPoint" -then - # mount exists; nothing to do here; keep for compatibility with original flow - : -else - # mount point not present - # if not in read-check mode, consider deleting VMs similar to original behavior - if [ "$rflag" == "0" ] - then - deleteVMs $MountPoint - fi -fi hbFolder="$MountPoint/KVMHA" hbFile="$hbFolder/hb-$HostIP" From d089836ddf33e4733818ad6e9ff1c35bc6c894da Mon Sep 17 00:00:00 2001 From: Daniel Vogel Date: Thu, 8 Oct 2026 10:01:14 +0200 Subject: [PATCH 3/3] kvm: harden kvmsmpheartbeat.sh mount check, temp cleanup and hb parsing Address the review comments on the heartbeat script: - is_on_mounted_fs(): accept the path if it is a mount point itself (as with the former 'mountpoint -q' check). Otherwise accept a subdirectory only if its filesystem is neither / nor a local disk filesystem (ext*, xfs, btrfs, tmpfs, ...). An exact pool mount nested below a separate local filesystem (e.g. /var) that is lost no longer passes: its empty directory belongs to the local parent filesystem, so the heartbeat is not written locally. - write_hbLog: Script.execute() ends a timed out run with destroyForcibly(), so the EXIT/TERM trap cannot remove the temporary file. Remove leftover hb-.* files older than one minute on the next run. - check_hbLog: limit the timestamp to 12 digits and evaluate it as base 10, so a corrupt value with a leading zero (e.g. "08") reports DEAD instead of failing with "value too great for base". Co-Authored-By: Claude Opus 5.5 --- scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh | 36 ++++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh b/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh index 5a8e4e8d564c..0855b95dc7c8 100755 --- a/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh +++ b/scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh @@ -80,17 +80,31 @@ if [ ! -d "$MountPoint" ]; then exit 1 fi -# Returns 0 if the given path resides on a mounted filesystem other than the -# root filesystem. A SharedMountPoint path does not need to be a mount point -# itself, it may be a subdirectory of a mounted (e.g. clustered) filesystem. +# Returns 0 if the given path is usable for the heartbeat: +# - the path is a mount point itself (same behaviour as before), or +# - the path is a subdirectory of a mounted (e.g. clustered) filesystem that +# is neither the root filesystem nor a local disk filesystem. +# The second rule keeps a lost exact mount from passing: its empty directory +# then belongs to the parent filesystem (e.g. a local /var), which is refused. is_on_mounted_fs() { - local target + local target fstype + if command -v mountpoint >/dev/null 2>&1 && mountpoint -q "$1"; then + return 0 + fi if command -v findmnt >/dev/null 2>&1; then target=$(findmnt -n -o TARGET -T "$1" 2>/dev/null) + fstype=$(findmnt -n -o FSTYPE -T "$1" 2>/dev/null) else target=$(df -P "$1" 2>/dev/null | awk 'NR==2 {print $6}') + fstype=$(df -PT "$1" 2>/dev/null | awk 'NR==2 {print $2}') fi - [ -n "$target" ] && [ "$target" != "/" ] + [ -n "$target" ] && [ "$target" != "/" ] || return 1 + case "$fstype" in + ext2|ext3|ext4|xfs|btrfs|zfs|f2fs|vfat|exfat|ntfs|ntfs3|tmpfs|ramfs|overlay|squashfs) + return 1 + ;; + esac + return 0 } # Ensure the path is on a mounted filesystem (not the local root filesystem) @@ -122,6 +136,10 @@ write_hbLog() { fi fi + # A run killed on timeout (SIGKILL) cannot run its trap; remove its + # leftover temporary file on the next run. + find "$hbFolder" -maxdepth 1 -name "hb-$HostIP.*" -mmin +1 -delete 2>/dev/null + timestamp=$(date +%s) # Write atomically to avoid partial writes (write to tmp then mv) tmpfile="${hbFile}.$$" @@ -151,13 +169,19 @@ check_hbLog() { return 1 fi # note: 'expr' exits with 1 when the result is 0, so use shell arithmetic + # only accept a plain decimal timestamp of sane length case "$hb" in ''|*[!0-9]*) hb_diff=999997 return 1 ;; esac - diff=$((now - hb)) + if [ ${#hb} -gt 12 ]; then + hb_diff=999997 + return 1 + fi + # base 10, otherwise a leading 0 (e.g. "08") is parsed as octal + diff=$((now - 10#$hb)) if [ -z "$interval" ]; then # if no interval provided, consider 0 as success if [ $diff -gt 0 ]; then