danivogel90 commented on code in PR #14331:
URL: https://github.com/apache/cloudstack/pull/14331#discussion_r4206284194
##########
scripts/vm/hypervisor/kvm/kvmsmpheartbeat.sh:
##########
@@ -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"
Review Comment:
Good catch, thanks! It is unreachable now, and in practice it was before as
well: in 4.22.1/4.23 the `mountpoint -q` check exits first whenever
`mountpoint` is installed (util-linux, so always).
I dropped the branch and the now unused `deleteVMs()` function in acd5ba4
rather than moving the kill in front of the first exit, because:
- The NFS script (`kvmheartbeat.sh`) only kills VMs *after* it has remounted
a lost share. Their disks pointed to the old, dead mount. A SharedMountPoint
can't be remounted by the script, so that reasoning doesn't apply here.
- On hyperconverged setups with a clustered FS, a failed check can be
transient, e.g. while the FS recovers a failed node. During our HA test,
heartbeat writes on the *surviving* hosts timed out for ~40 s. Running `kill
-9` on all VMs of the pool at that moment would turn a short storage hiccup
into an outage of healthy VMs.
- Fencing and VM restart decisions are already made by the HA logic in the
agent and the management server.
So the behaviour stays the same as what actually ran in 4.22.1/4.23, minus
the dead code. Re-tested: write/read on a subdirectory of a GPFS mount, tmpfs
mount point, path on the root FS and non-existent path (both refused), stale
heartbeat → DEAD.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
