Copilot commented on code in PR #14320:
URL: https://github.com/apache/cloudstack/pull/14320#discussion_r4222547781
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java:
##########
@@ -330,29 +332,94 @@ public boolean isPoolSupportHA() {
public String getHearthBeatPath() {
if (StoragePoolType.NetworkFilesystem.equals(type)) {
- String kvmScriptsDir =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR);
- String scriptPath = Script.findScript(kvmScriptsDir,
"kvmheartbeat.sh");
- if (scriptPath == null) {
- throw new CloudRuntimeException("Unable to find heartbeat
script 'kvmheartbeat.sh' in directory: " + kvmScriptsDir);
- }
- return scriptPath;
+ return findKvmHaScript("kvmheartbeat.sh");
} else if (StoragePoolType.SharedMountPoint.equals(type)) {
- String kvmScriptsDir =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR);
- String scriptPath = Script.findScript(kvmScriptsDir,
"kvmsmpheartbeat.sh");
- if (scriptPath == null) {
- throw new CloudRuntimeException("Unable to find heartbeat
script 'kvmsmpheartbeat.sh' in directory: " + kvmScriptsDir);
- }
- return scriptPath;
+ return findKvmHaScript("kvmsmpheartbeat.sh");
+ } else if (StoragePoolType.RBD.equals(type)) {
+ return findKvmHaScript("kvmheartbeat_rbd.sh");
}
return null;
}
+ private String findKvmHaScript(String scriptName) {
+ String kvmScriptsDir =
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR);
+ String scriptPath = Script.findScript(kvmScriptsDir, scriptName);
+ if (scriptPath == null) {
+ throw new CloudRuntimeException(String.format("Unable to find
script '%s' in directory: %s", scriptName, kvmScriptsDir));
+ }
+ return scriptPath;
+ }
+
+ /**
+ * Returns the Ceph monitors as expected by "--mon-host": the
comma-separated monitors of the pool, trimmed
+ * and without empty entries. If the pool has a monitor port, it is added
to each monitor which has none yet.
+ */
+ protected String getRbdMonitors() {
+ List<String> monitors = new ArrayList<>();
+ for (String monitor : sourceHost.split(",")) {
+ monitor = monitor.trim();
+ if (monitor.isEmpty()) {
+ continue;
+ }
+ monitors.add(sourcePort > 0 ? addPortToRbdMonitor(monitor) :
monitor);
+ }
+ return String.join(",", monitors);
+ }
+
+ private String addPortToRbdMonitor(String monitor) {
+ if (monitor.startsWith("[")) {
+ // IPv6 address in square brackets, which has a port if followed
by ":<port>"
+ return monitor.contains("]:") ? monitor : monitor + ":" +
sourcePort;
+ }
+ int colons = StringUtils.countMatches(monitor, ":");
+ if (colons == 0) {
+ return monitor + ":" + sourcePort;
+ }
+ if (colons == 1) {
+ // IPv4 address or host name, with a port
+ return monitor;
+ }
+ // IPv6 address without square brackets, so without a port
+ return "[" + monitor + "]:" + sourcePort;
+ }
Review Comment:
If a user provides an IPv6 monitor with an explicit port but without
brackets (e.g. `fd00::1:3300`), this method will treat it as 'IPv6 without a
port' and produce an invalid result like `[fd00::1:3300]:<sourcePort>`. A more
robust approach is to detect a trailing `:<digits>` port segment for IPv6 and
treat it as 'already has port' (wrapping in brackets if needed). Adding a unit
test for this input would prevent regressions.
##########
scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh:
##########
@@ -0,0 +1,204 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Ceph RBD flavor of kvmvmactivity.sh.
+#
+# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime
+# of the volume files on the shared mount point. RBD volumes aren't files on
+# a mount point, so instead activity is detected via RBD watchers: as long as
+# qemu has an RBD image open (i.e. a VM using that volume is running
+# somewhere), the image will have a live watcher. The most recent
+# suspect-time/watcher-state is persisted as a RADOS object (per host)
+# in place of the "ac-<host>" file used by the NFS/SMP scripts.
+
+help() {
+ printf "Usage: $0
+ -s ceph monitor host(s), comma separated
+ -o ceph/rbd pool name
+ -n cephx auth user (optional)
+ -k cephx auth key, base64 (optional, required if -n is set)
+ -h host
+ -u volume (rbd image) uuid list
+ -t current time in seconds (accepted for compatibility
with kvmvmactivity.sh, not used)
+ -d suspect time\n"
+ exit 1
+}
Review Comment:
Same as `kvmheartbeat_rbd.sh`: `-k` is described as base64 but is not
decoded before being written to a keyfile. Align behavior and documentation by
either decoding base64 or updating the usage string to reflect the expected
format (raw Ceph key string).
##########
scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh:
##########
@@ -0,0 +1,204 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Ceph RBD flavor of kvmvmactivity.sh.
+#
+# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime
+# of the volume files on the shared mount point. RBD volumes aren't files on
+# a mount point, so instead activity is detected via RBD watchers: as long as
+# qemu has an RBD image open (i.e. a VM using that volume is running
+# somewhere), the image will have a live watcher. The most recent
+# suspect-time/watcher-state is persisted as a RADOS object (per host)
+# in place of the "ac-<host>" file used by the NFS/SMP scripts.
+
+help() {
+ printf "Usage: $0
+ -s ceph monitor host(s), comma separated
+ -o ceph/rbd pool name
+ -n cephx auth user (optional)
+ -k cephx auth key, base64 (optional, required if -n is set)
+ -h host
+ -u volume (rbd image) uuid list
+ -t current time in seconds (accepted for compatibility
with kvmvmactivity.sh, not used)
+ -d suspect time\n"
+ exit 1
+}
+
+#set -x
+
+MonHosts=
+PoolName=
+CephUser=
+CephKey=
+HostIP=
+UUIDList=
+SuspectTime=
+
+while getopts 's:o:n:k:h:u:t:d:' OPTION
+do
+ case $OPTION in
+ s)
+ MonHosts="$OPTARG"
+ ;;
+ o)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ CephUser="$OPTARG"
+ ;;
+ k)
+ CephKey="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ u)
+ UUIDList="$OPTARG"
+ ;;
+ t)
+ # not used, see help
+ ;;
+ d)
+ SuspectTime="$OPTARG"
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$MonHosts" ] || [ -z "$PoolName" ]
+then
+ exit 2
+fi
+
+if [ -z "$SuspectTime" ]
+then
+ exit 2
+fi
+
+# the host IP names the heartbeat and activity objects
+if [ -z "$HostIP" ]
+then
+ exit 2
+fi
+
+if [ -n "$CephUser" ] && [ -z "$CephKey" ]
+then
+ exit 2
+fi
+
+RadosOpts=(--mon-host "$MonHosts")
+RbdOpts=(--mon-host "$MonHosts")
+if [ -n "$CephUser" ]
+then
+ # the key is given to rados and rbd in a file, to keep it out of the
process list
+ KeyFile=$(mktemp)
+ trap 'rm -f "$KeyFile"' EXIT
+ printf '%s' "$CephKey" > "$KeyFile"
+ RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+ RbdOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+fi
+
+hbObject="KVMHA-hb-$HostIP"
+acObject="KVMHA-ac-$HostIP"
+
+# First check: heartbeat object, same as kvmheartbeat_rbd.sh
+now=$(date +%s)
+hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null)
+if [[ "$hb" =~ ^[0-9]+$ ]]
+then
+ diff=$(expr $now - $hb)
+ if [ $diff -lt 61 ]
+ then
+ echo "=====> ALIVE <====="
+ exit 0
+ fi
+fi
+
+if [ -z "$UUIDList" ]
+then
+ echo "=====> Considering host as DEAD due to empty UUIDList <======"
+ exit 0
+fi
+
+# Second check: RBD watcher based disk activity check.
+# If any of the host's volumes still has a live watcher, something (most
+# likely qemu on the host being checked) is actively using it right now.
+latestUpdateTime=0
+IFS=',' read -ra images <<< "$UUIDList"
+for image in "${images[@]}"
+do
+ image=${image//[[:space:]]/}
+ if [ -z "$image" ]
+ then
+ continue
+ fi
+ watcherCount=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" --format json
2> /dev/null | \
+ python3 -c 'import json,sys
+try:
+ print(len(json.load(sys.stdin).get("watchers", [])))
+except Exception:
+ print(0)' 2> /dev/null)
Review Comment:
This introduces a hard runtime dependency on `python3` on KVM hosts for HA
checks. If `python3` is missing, the script will silently treat errors as '0
watchers', which can incorrectly mark hosts as DEAD. Consider either (a)
explicitly checking for `python3` at startup and emitting a clear failure
reason, or (b) using a parser-less approach (or an already-guaranteed
dependency) and propagating parsing failures distinctly from 'no watchers'.
##########
scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh:
##########
@@ -0,0 +1,204 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Ceph RBD flavor of kvmvmactivity.sh.
+#
+# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime
+# of the volume files on the shared mount point. RBD volumes aren't files on
+# a mount point, so instead activity is detected via RBD watchers: as long as
+# qemu has an RBD image open (i.e. a VM using that volume is running
+# somewhere), the image will have a live watcher. The most recent
+# suspect-time/watcher-state is persisted as a RADOS object (per host)
+# in place of the "ac-<host>" file used by the NFS/SMP scripts.
+
+help() {
+ printf "Usage: $0
+ -s ceph monitor host(s), comma separated
+ -o ceph/rbd pool name
+ -n cephx auth user (optional)
+ -k cephx auth key, base64 (optional, required if -n is set)
+ -h host
+ -u volume (rbd image) uuid list
+ -t current time in seconds (accepted for compatibility
with kvmvmactivity.sh, not used)
+ -d suspect time\n"
+ exit 1
+}
+
+#set -x
+
+MonHosts=
+PoolName=
+CephUser=
+CephKey=
+HostIP=
+UUIDList=
+SuspectTime=
+
+while getopts 's:o:n:k:h:u:t:d:' OPTION
+do
+ case $OPTION in
+ s)
+ MonHosts="$OPTARG"
+ ;;
+ o)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ CephUser="$OPTARG"
+ ;;
+ k)
+ CephKey="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ u)
+ UUIDList="$OPTARG"
+ ;;
+ t)
+ # not used, see help
+ ;;
+ d)
+ SuspectTime="$OPTARG"
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$MonHosts" ] || [ -z "$PoolName" ]
+then
+ exit 2
+fi
+
+if [ -z "$SuspectTime" ]
+then
+ exit 2
+fi
+
+# the host IP names the heartbeat and activity objects
+if [ -z "$HostIP" ]
+then
+ exit 2
+fi
+
+if [ -n "$CephUser" ] && [ -z "$CephKey" ]
+then
+ exit 2
+fi
+
+RadosOpts=(--mon-host "$MonHosts")
+RbdOpts=(--mon-host "$MonHosts")
+if [ -n "$CephUser" ]
+then
+ # the key is given to rados and rbd in a file, to keep it out of the
process list
+ KeyFile=$(mktemp)
+ trap 'rm -f "$KeyFile"' EXIT
+ printf '%s' "$CephKey" > "$KeyFile"
+ RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+ RbdOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+fi
+
+hbObject="KVMHA-hb-$HostIP"
+acObject="KVMHA-ac-$HostIP"
+
+# First check: heartbeat object, same as kvmheartbeat_rbd.sh
+now=$(date +%s)
+hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null)
+if [[ "$hb" =~ ^[0-9]+$ ]]
+then
+ diff=$(expr $now - $hb)
+ if [ $diff -lt 61 ]
+ then
+ echo "=====> ALIVE <====="
+ exit 0
+ fi
+fi
+
+if [ -z "$UUIDList" ]
+then
+ echo "=====> Considering host as DEAD due to empty UUIDList <======"
+ exit 0
+fi
+
+# Second check: RBD watcher based disk activity check.
+# If any of the host's volumes still has a live watcher, something (most
+# likely qemu on the host being checked) is actively using it right now.
+latestUpdateTime=0
+IFS=',' read -ra images <<< "$UUIDList"
+for image in "${images[@]}"
+do
+ image=${image//[[:space:]]/}
+ if [ -z "$image" ]
+ then
+ continue
+ fi
+ watcherCount=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" --format json
2> /dev/null | \
+ python3 -c 'import json,sys
+try:
+ print(len(json.load(sys.stdin).get("watchers", [])))
+except Exception:
+ print(0)' 2> /dev/null)
+ if [ -n "$watcherCount" ] && [ "$watcherCount" -gt 0 ] 2> /dev/null
+ then
+ latestUpdateTime=$now
+ break
+ fi
+done
+
+if rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" &> /dev/null
+then
+ acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2>
/dev/null)
+else
+ acTime=
+fi
+
+tmpFile=$(mktemp)
+echo "$SuspectTime:$latestUpdateTime" > "$tmpFile"
+rados -p "$PoolName" "${RadosOpts[@]}" put "$acObject" "$tmpFile" &> /dev/null
+rm -f "$tmpFile"
+
+if [ -z "$acTime" ]; then
+ if [[ $latestUpdateTime -gt $SuspectTime ]]; then
+ echo "=====> ALIVE <====="
+ else
+ echo "=====> Considering host as DEAD due to RADOS object [$acObject]
did not exist and condition [latestUpdateTime -gt SuspectTime] has not been
satisfied. <======"
+ fi
+else
+ arrTime=(${acTime//:/ })
+ lastSuspectTime=${arrTime[0]}
+ lastUpdateTime=${arrTime[1]}
+
+ suspectTimeDiff=$(expr $SuspectTime - $lastSuspectTime)
Review Comment:
This assumes the persisted RADOS object content is always exactly
`suspectTime:lastUpdateTime`. If the object is corrupted, empty, or missing the
delimiter, `lastSuspectTime/lastUpdateTime` can be unset and `expr` will error
(potentially causing incorrect DEAD/ALIVE decisions). Add a format validation
step (e.g., ensure it matches `^[0-9]+:[0-9]+$`) and treat invalid content as
'no prior state' (or rewrite it) before doing arithmetic.
##########
scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh:
##########
@@ -0,0 +1,204 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Ceph RBD flavor of kvmvmactivity.sh.
+#
+# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime
+# of the volume files on the shared mount point. RBD volumes aren't files on
+# a mount point, so instead activity is detected via RBD watchers: as long as
+# qemu has an RBD image open (i.e. a VM using that volume is running
+# somewhere), the image will have a live watcher. The most recent
+# suspect-time/watcher-state is persisted as a RADOS object (per host)
+# in place of the "ac-<host>" file used by the NFS/SMP scripts.
+
+help() {
+ printf "Usage: $0
+ -s ceph monitor host(s), comma separated
+ -o ceph/rbd pool name
+ -n cephx auth user (optional)
+ -k cephx auth key, base64 (optional, required if -n is set)
+ -h host
+ -u volume (rbd image) uuid list
+ -t current time in seconds (accepted for compatibility
with kvmvmactivity.sh, not used)
+ -d suspect time\n"
+ exit 1
+}
+
+#set -x
+
+MonHosts=
+PoolName=
+CephUser=
+CephKey=
+HostIP=
+UUIDList=
+SuspectTime=
+
+while getopts 's:o:n:k:h:u:t:d:' OPTION
+do
+ case $OPTION in
+ s)
+ MonHosts="$OPTARG"
+ ;;
+ o)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ CephUser="$OPTARG"
+ ;;
+ k)
+ CephKey="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ u)
+ UUIDList="$OPTARG"
+ ;;
+ t)
+ # not used, see help
+ ;;
+ d)
+ SuspectTime="$OPTARG"
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$MonHosts" ] || [ -z "$PoolName" ]
+then
+ exit 2
+fi
+
+if [ -z "$SuspectTime" ]
+then
+ exit 2
+fi
+
+# the host IP names the heartbeat and activity objects
+if [ -z "$HostIP" ]
+then
+ exit 2
+fi
+
+if [ -n "$CephUser" ] && [ -z "$CephKey" ]
+then
+ exit 2
+fi
+
+RadosOpts=(--mon-host "$MonHosts")
+RbdOpts=(--mon-host "$MonHosts")
+if [ -n "$CephUser" ]
+then
+ # the key is given to rados and rbd in a file, to keep it out of the
process list
+ KeyFile=$(mktemp)
+ trap 'rm -f "$KeyFile"' EXIT
+ printf '%s' "$CephKey" > "$KeyFile"
+ RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+ RbdOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+fi
+
+hbObject="KVMHA-hb-$HostIP"
+acObject="KVMHA-ac-$HostIP"
+
+# First check: heartbeat object, same as kvmheartbeat_rbd.sh
+now=$(date +%s)
+hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null)
+if [[ "$hb" =~ ^[0-9]+$ ]]
+then
+ diff=$(expr $now - $hb)
+ if [ $diff -lt 61 ]
+ then
+ echo "=====> ALIVE <====="
+ exit 0
+ fi
+fi
+
+if [ -z "$UUIDList" ]
+then
+ echo "=====> Considering host as DEAD due to empty UUIDList <======"
+ exit 0
+fi
+
+# Second check: RBD watcher based disk activity check.
+# If any of the host's volumes still has a live watcher, something (most
+# likely qemu on the host being checked) is actively using it right now.
+latestUpdateTime=0
+IFS=',' read -ra images <<< "$UUIDList"
+for image in "${images[@]}"
+do
+ image=${image//[[:space:]]/}
+ if [ -z "$image" ]
+ then
+ continue
+ fi
+ watcherCount=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" --format json
2> /dev/null | \
+ python3 -c 'import json,sys
+try:
+ print(len(json.load(sys.stdin).get("watchers", [])))
+except Exception:
+ print(0)' 2> /dev/null)
+ if [ -n "$watcherCount" ] && [ "$watcherCount" -gt 0 ] 2> /dev/null
+ then
+ latestUpdateTime=$now
+ break
+ fi
+done
+
+if rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" &> /dev/null
+then
+ acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2>
/dev/null)
+else
+ acTime=
+fi
Review Comment:
When `rados get` fails (auth/connectivity), `acTime` becomes empty and later
branches may print messages implying the object 'did not exist'. To keep the
failure reason accurate, capture and use the `rados get` return code (or avoid
redirecting stderr) so you can distinguish 'missing object' from 'read error'.
##########
scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh:
##########
@@ -0,0 +1,167 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Ceph RBD flavor of kvmheartbeat.sh/kvmsmpheartbeat.sh.
+#
+# There is no shared POSIX mount point to write a heartbeat file to when the
+# primary storage pool is Ceph RBD, so the heartbeat timestamp is instead
+# stored as a small RADOS object (one object per host) in the same RBD pool.
+# Any host with a working path to the Ceph cluster can write/read this object,
+# which gives the same semantics as the NFS/SharedMountPoint heartbeat file.
+
+help() {
+ printf "Usage: $0
+ -s ceph monitor host(s), comma separated
+ -o ceph/rbd pool name
+ -n cephx auth user (optional)
+ -k cephx auth key, base64 (optional, required if -n is set)
+ -h host
+ -r write/read hb log
+ -c cleanup
+ -t interval between read hb log\n"
+ exit 1
+}
Review Comment:
The `-k` help text says the Ceph key is base64, but the script writes the
provided string directly to `--keyfile` without decoding. Either (a) decode
base64 before writing, or (b) update the usage text (and any related docs) to
state the key is passed as-is (Ceph key string). This is important because
following the current help output can cause auth failures.
##########
scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh:
##########
@@ -0,0 +1,204 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Ceph RBD flavor of kvmvmactivity.sh.
+#
+# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime
+# of the volume files on the shared mount point. RBD volumes aren't files on
+# a mount point, so instead activity is detected via RBD watchers: as long as
+# qemu has an RBD image open (i.e. a VM using that volume is running
+# somewhere), the image will have a live watcher. The most recent
+# suspect-time/watcher-state is persisted as a RADOS object (per host)
+# in place of the "ac-<host>" file used by the NFS/SMP scripts.
+
+help() {
+ printf "Usage: $0
+ -s ceph monitor host(s), comma separated
+ -o ceph/rbd pool name
+ -n cephx auth user (optional)
+ -k cephx auth key, base64 (optional, required if -n is set)
+ -h host
+ -u volume (rbd image) uuid list
+ -t current time in seconds (accepted for compatibility
with kvmvmactivity.sh, not used)
+ -d suspect time\n"
+ exit 1
+}
+
+#set -x
+
+MonHosts=
+PoolName=
+CephUser=
+CephKey=
+HostIP=
+UUIDList=
+SuspectTime=
+
+while getopts 's:o:n:k:h:u:t:d:' OPTION
+do
+ case $OPTION in
+ s)
+ MonHosts="$OPTARG"
+ ;;
+ o)
+ PoolName="$OPTARG"
+ ;;
+ n)
+ CephUser="$OPTARG"
+ ;;
+ k)
+ CephKey="$OPTARG"
+ ;;
+ h)
+ HostIP="$OPTARG"
+ ;;
+ u)
+ UUIDList="$OPTARG"
+ ;;
+ t)
+ # not used, see help
+ ;;
+ d)
+ SuspectTime="$OPTARG"
+ ;;
+ *)
+ help
+ ;;
+ esac
+done
+
+if [ -z "$MonHosts" ] || [ -z "$PoolName" ]
+then
+ exit 2
+fi
+
+if [ -z "$SuspectTime" ]
+then
+ exit 2
+fi
+
+# the host IP names the heartbeat and activity objects
+if [ -z "$HostIP" ]
+then
+ exit 2
+fi
+
+if [ -n "$CephUser" ] && [ -z "$CephKey" ]
+then
+ exit 2
+fi
+
+RadosOpts=(--mon-host "$MonHosts")
+RbdOpts=(--mon-host "$MonHosts")
+if [ -n "$CephUser" ]
+then
+ # the key is given to rados and rbd in a file, to keep it out of the
process list
+ KeyFile=$(mktemp)
+ trap 'rm -f "$KeyFile"' EXIT
+ printf '%s' "$CephKey" > "$KeyFile"
+ RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+ RbdOpts+=(--id "$CephUser" --keyfile "$KeyFile")
+fi
+
+hbObject="KVMHA-hb-$HostIP"
+acObject="KVMHA-ac-$HostIP"
+
+# First check: heartbeat object, same as kvmheartbeat_rbd.sh
+now=$(date +%s)
+hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null)
+if [[ "$hb" =~ ^[0-9]+$ ]]
+then
+ diff=$(expr $now - $hb)
+ if [ $diff -lt 61 ]
+ then
+ echo "=====> ALIVE <====="
+ exit 0
+ fi
+fi
Review Comment:
All `rados` failures are currently silenced (`2> /dev/null`), and a read
failure (e.g., Ceph down, auth failure) is treated the same as 'object
missing'. This can lead to misleading 'DEAD' conclusions and messages
downstream. Consider checking the `rados get` exit status explicitly and
emitting a distinct reason (e.g., 'could not read heartbeat object') similar to
the heartbeat script’s behavior.
--
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]
