Skip to content

kvm: apply rbd_default_data_pool when creating volumes from templates on RBD - #13361

Open
bhouse-nexthop wants to merge 1 commit into
apache:4.22from
bhouse-nexthop:fix/kvm-rbd-ec-datapool-template-clone
Open

kvm: apply rbd_default_data_pool when creating volumes from templates on RBD#13361
bhouse-nexthop wants to merge 1 commit into
apache:4.22from
bhouse-nexthop:fix/kvm-rbd-ec-datapool-template-clone

Conversation

@bhouse-nexthop

@bhouse-nexthopbhouse-nexthop commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Description

RBD erasure-coded (EC) pool support (#9808) taught the KVM agent to honor the rbd_default_data_pool storage-pool detail, but only in two places:

  • KVMPhysicalDisk.RBDStringBuilder() — the qemu-img URI builder
  • createPhysicalDisk() — blank volume creation (routes RBD creates to the QemuImg path when a data pool is set)

It was not applied to createDiskFromTemplateOnRBD(), which creates ROOT volumes from a template using rados-java (rbd.clone() / rbd.create()) directly. Those calls build a Rados connection that sets mon_host, key and client_mount_timeout but never rbd_default_data_pool, so the resulting image is created without a data pool.

Net effect: every volume cloned from a template onto an EC-backed primary storage has all of its data objects written to the replicated metadata pool instead of the erasure-coded data pool. This silently defeats EC and consumes ~3x raw space for those volumes. Blank data disks on the same pool are correct, which makes the inconsistency easy to miss.

This was observed in production: of the VMs on an EC primary storage, the template-derived ROOT volumes had no data_pool (rbd info shows no data_pool: line and lacks the data-pool feature), while blank DATADISKs and the template base images themselves were correct.

Fix

In createDiskFromTemplateOnRBD, read the destination pool's rbd_default_data_pool detail once and, when present, confSet it on the Rados connection beforeconnect() — in both the same-cluster clone/copy branch and the cross-cluster copy branch. librbd then uses it as the default data pool when the new image is created, so template-derived volumes get data_pool set, exactly like blank volumes already do. This mirrors how RBDStringBuilder injects the same key for the qemu-img path.

StringdataPool = (destDetails == null) ? null : destDetails.get(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL);
...
if (dataPool != null) {
r.confSet(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL, dataPool);
}
r.connect();

No behavior change for non-EC pools (dataPool == null → no-op).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Root cause reproduced live: deploying a VM from a template onto an EC primary storage produced a ROOT clone with no data_pool (parent base snapshot in the metadata pool); the data-pool feature was absent.
  • Confirmed the working paths (template seed via qemu-img, blank DATADISK create) already set the data pool, isolating the gap to the rados-java clone/create path.
  • Code-path traced to Add support to RBD erasure code pools #9808, which did not touch createDiskFromTemplateOnRBD.

… on RBD
RBD erasure-coded pool support (apache#9808) added handling of the
rbd_default_data_pool storage-pool detail to RBDStringBuilder (qemu-img
path) and to createPhysicalDisk (blank volumes), but not to
createDiskFromTemplateOnRBD. As a result, ROOT volumes created from a
template via rados-java rbd.clone()/rbd.create() are created without a
data pool: all of their data objects land in the (replicated) metadata
pool instead of the erasure-coded data pool, defeating the point of EC
and consuming ~3x raw space.
Set rbd_default_data_pool on the Rados connection (before connect) in
both the same-cluster clone/copy branch and the cross-cluster copy
branch of createDiskFromTemplateOnRBD, using the destination pool's
detail. librbd then honors it as the default data pool when the new
image is created, so template-derived volumes get data_pool set, the
same way blank volumes already do.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@codecov

codecovBot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.67%. Comparing base (21b2025) to head (aaa2a61).
⚠️ Report is 70 commits behind head on 4.22.

Files with missing linesPatch %Lines
.../hypervisor/kvm/storage/LibvirtStorageAdaptor.java0.00%8 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## 4.22 #13361 +/- ##
============================================
- Coverage 17.67% 17.67% -0.01% + Complexity 15792 15789 -3 
============================================
Files 5922 5922 Lines 533165 533173 +8 Branches 65208 65211 +3 ============================================
- Hits 94242 94224 -18 - Misses 428276 428305 +29 + Partials 10647 10644 -3 
FlagCoverage Δ
uitests3.69% <ø> (ø)
unittests18.74% <0.00%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18262

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-16344)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 50505 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13361-t16344-kvm-ol8.zip
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@bhouse-nexthop

Copy link
Copy Markdown
CollaboratorAuthor

@vladimirpetrov@sureshanaparti please make sure this gets into 4.22.2

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@bhouse-nexthop@DaanHoogland@blueorangutan@vladimirpetrov@weizhouapache@bradh352