Repository navigation
kvm: support Host HA on Ceph RBD primary storage - #14320
Draft
weizhouapache wants to merge 4 commits into
Draft
weizhouapache wants to merge 4 commits into
weizhouapache wants to merge 4 commits into
Conversation
Extend the existing KVM Host-HA heartbeat/VM-activity-check framework (currently limited to NetworkFilesystem and SharedMountPoint pools) to also cover RBD primary storage, based on the approach from the old PR apache#5862, adapted to the current HA architecture and reusing the multi-monitor Ceph support already added in apache#6792. - Add StoragePoolType.RBD to LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT, which is the single switch that makes pool registration, KVMHAMonitor, and the CheckOnHostCommand/CheckVMActivityOnStoragePoolCommand wrappers treat RBD pools as HA-capable. - LibvirtStoragePool: build rbd/rados connection args (--mon-host, pool, and cephx --id/--key when set) from the pool's existing sourceHost/ sourceDir/authUsername/authSecret fields for the heartbeat and VM-activity checks, mirroring the conventions KVMPhysicalDisk already uses to talk to RBD. - Add kvmheartbeat_rbd.sh and kvmvmactivity_rbd.sh: RBD has no shared mount point to write a heartbeat file to, so the heartbeat timestamp is stored as a small RADOS object per host instead, and VM activity is detected via RBD watchers (rbd status) rather than file mtimes. - Minor: fix a stale "NFS storage pool" log message in KVMHAMonitor now that this path also runs for RBD. - Add LibvirtStoragePoolTest#testIsPoolSupportHA covering the new RBD case (no HA/heartbeat tests existed previously for any pool type).
14 tasks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14320 +/- ##
============================================
+ Coverage 19.91% 20.21% +0.30%
- Complexity 20198 20763 +565
============================================
Files 6373 6426 +53
Lines 577230 580218 +2988
Branches 70696 71043 +347
============================================
+ Hits 114936 117309 +2373
- Misses 449736 450196 +460
- Partials 12558 12713 +155
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19426 |
Member
Author
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-17079)
|
weizhouapache
requested review from
harikrishna-patnala
and
a lite review from Copilot
October 7, 2026 14:46
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical RBD monitor-port and stale-heartbeat handling issues remain, along with incomplete HA cleanup.
3 open findings
What changed in this PR
Extends KVM Host HA heartbeat and VM-activity checks to Ceph RBD primary storage.
Changes:
- Adds RBD HA support and connection handling.
- Adds RADOS heartbeat and RBD watcher activity scripts.
- Updates logging and adds HA eligibility tests.
| File | Summary |
|---|---|
scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh |
Adds RBD watcher-based VM activity detection. |
scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh |
Adds RADOS-backed heartbeat handling; stale delays over 255 seconds require bounded status handling. |
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java |
Tests RBD HA support eligibility. |
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java |
Selects RBD scripts and builds Ceph arguments; monitor ports must be propagated, and missing-script errors should be generic. |
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java |
Generalizes the storage-pool log message. |
engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java |
Enables RBD HA support; corresponding pool deletion paths must also clean up HA state. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- pass the pool's monitor port to the RBD heartbeat/activity scripts, for every monitor, instead of relying on the default port - kvmheartbeat_rbd.sh: keep the heartbeat age out of the function return status, which wraps above 255 and made a long-stale heartbeat look ALIVE - remove pools from the HA monitor on deletion for all storage pool types with HA support, without trying to umount an empty mount path - make the missing-script error message generic - add unit tests for the RBD monitor list (IPv4, IPv6, mixed, with/without port)
Member
Author
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
11 open findings
kvmheartbeat_rbd.shuses-hto derive the per-host RADOS object name… · New-hwas provided; ifHostIPis empty, the heartbeat object… · Newpool.getMountDestPath()directly into a shell command string can break if the path… · NewsourcePort <= 0, this returnssourceHostwithout normalization. IfsourceHostcontains… · NewauthUsername != nullis not sufficient to safely add cephx args:authUsernamecould be… · New-thelp text says 'time on ms', but the caller (LibvirtStoragePool) passes seconds… · New-thelp text says 'time on ms', but the caller (LibvirtStoragePool) passes seconds… · New-thelp text says 'time on ms', but the caller (LibvirtStoragePool) passes seconds… · New
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19467 |
Copilot stopped reviewing on behalf of
weizhouapache due to an error
October 8, 2026 10:46
…of the pool Follow-up to the review of the Host HA on Ceph RBD support: - kvmheartbeat_rbd.sh and kvmvmactivity_rbd.sh: exit with an error when the host IP, which names the RADOS objects, is missing (the self-fencing -c of the heartbeat script does not need it), or when a Ceph user is given without its key - LibvirtStoragePool: trim the Ceph monitors and skip empty entries, with or without a port; pass the Ceph user and key only if both are set and fail with a clear error if only one of them is - KVMHAMonitor: run umount without a shell when removing a pool - kvmvmactivity_rbd.sh: fix the -t help text and drop the unused field from the stored activity state - add unit tests for the monitor list (multiple IPv4 and IPv6 monitors, with and without a port, with spaces) and for the Ceph user and key handling
Member
Author
|
@blueorangutan package |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
6 open findings
:as an IPv6 literal and will wrap it in brackets, which… · NewgetRbdMonitors()currently branches on the presence of:(which can also appear in… · New--keyexposes it in the process list (both for this script’s args and… · Newintervalis used in a numeric comparison but is never validated when running with-r. If-r… · New${UUIDList//,/ }expansion relies on shell word-splitting and can mis-handle… · Newrados ... statoutput content to detect existence is brittle (output formats can vary, and… · New
11 resolved since last review
-hwas provided; ifHostIPis empty, the heartbeat object…kvmheartbeat_rbd.shuses-hto derive the per-host RADOS object name…-thelp text says 'time on ms', but the caller (LibvirtStoragePool) passes seconds…-thelp text says 'time on ms', but the caller (LibvirtStoragePool) passes seconds…-thelp text says 'time on ms', but the caller (LibvirtStoragePool) passes seconds…authUsername != nullis not sufficient to safely add cephx args:authUsernamecould be…sourcePort <= 0, this returnssourceHostwithout normalization. IfsourceHostcontains…pool.getMountDestPath()directly into a shell command string can break if the path…
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Copilot stopped reviewing on behalf of
weizhouapache due to an error
October 8, 2026 12:15
…st HA checks, and harden the parsing Follow-up to the review of the Host HA on Ceph RBD support: - LibvirtStoragePool: do not add a port to Ceph monitors which have one already, and only wrap bare IPv6 addresses in square brackets; mask the Ceph key in the logged command - kvmheartbeat_rbd.sh, kvmvmactivity_rbd.sh: give the Ceph key to rados and rbd in a temporary keyfile instead of --key - kvmheartbeat_rbd.sh: require -t to be a number when checking a heartbeat - kvmvmactivity_rbd.sh: read the list of volumes without word splitting or globbing, skipping empty entries, and check if the activity object exists by the exit status of rados stat - add unit tests for monitors with a port and for the masked key
Member
Author
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
7 open findings
-khelp text says the Ceph key is base64, but the script writes the provided string directly… · Newkvmheartbeat_rbd.sh:-kis described as base64 but is not decoded before being written… · Newradosfailures are currently silenced (2> /dev/null), and a read failure (e.g., Ceph down,… · Newpython3on KVM hosts for HA checks. Ifpython3is… · Newrados getfails (auth/connectivity),acTimebecomes empty and later branches may print… · NewsuspectTime:lastUpdateTime. If… · New
6 resolved since last review
rados ... statoutput content to detect existence is brittle (output formats can vary, and…${UUIDList//,/ }expansion relies on shell word-splitting and can mis-handle…intervalis used in a numeric comparison but is never validated when running with-r. If-r…--keyexposes it in the process list (both for this script’s args and…getRbdMonitors()currently branches on the presence of:(which can also appear in…:as an IPv6 literal and will wrap it in brackets, which…
🧠 Review effort: Lite
Comment on lines
+369
to
+384
| 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; | ||
| } |
Comment on lines
+27
to
+38
| 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 | ||
| } |
Comment on lines
+29
to
+40
| 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 | ||
| } |
Comment on lines
+121
to
+132
| # 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 |
Comment on lines
+152
to
+157
| 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) |
Comment on lines
+165
to
+170
| if rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" &> /dev/null | ||
| then | ||
| acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2> /dev/null) | ||
| else | ||
| acTime= | ||
| fi |
Comment on lines
+184
to
+188
| arrTime=(${acTime//:/ }) | ||
| lastSuspectTime=${arrTime[0]} | ||
| lastUpdateTime=${arrTime[1]} | ||
|
|
||
| suspectTimeDiff=$(expr $SuspectTime - $lastSuspectTime) |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19487 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Extend the existing KVM Host-HA heartbeat/VM-activity-check framework (currently limited to NetworkFilesystem and SharedMountPoint pools) to also cover RBD primary storage, based on the approach from the old PR #5862, adapted to the current HA architecture and reusing the multi-monitor Ceph support already added in #6792.
cwiki: https://cwiki.apache.org/confluence/spaces/CLOUDSTACK/pages/451979203/Host+HA+support+for+Ceph+RBD+primary+storage+KVM
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?