Skip to content

CLVM enhancements and fixes#12617

Draft
Pearl1594 wants to merge 44 commits intomainfrom
clvm-enhancements
Draft

CLVM enhancements and fixes#12617
Pearl1594 wants to merge 44 commits intomainfrom
clvm-enhancements

Conversation

@Pearl1594
Copy link
Copy Markdown
Contributor

@Pearl1594 Pearl1594 commented Feb 9, 2026

Description

This PR...

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 3.68%. Comparing base (d3e1976) to head (df61d6f).
⚠️ Report is 15 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (d3e1976) and HEAD (df61d6f). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d3e1976) HEAD (df61d6f)
unittests 1 0
Additional details and impacted files
@@              Coverage Diff              @@
##               main   #12617       +/-   ##
=============================================
- Coverage     17.90%    3.68%   -14.23%     
=============================================
  Files          5938      454     -5484     
  Lines        532864    38798   -494066     
  Branches      65192     7151    -58041     
=============================================
- Hits          95392     1428    -93964     
+ Misses       426793    37183   -389610     
+ Partials      10679      187    -10492     
Flag Coverage Δ
uitests 3.67% <ø> (+<0.01%) ⬆️
unittests ?

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

☔ View full report in Codecov by Sentry.
📢 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.

@Pearl1594
Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@Pearl1594 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.

@blueorangutan
Copy link
Copy Markdown

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

@harikrishna-patnala harikrishna-patnala added this to the 4.23.0 milestone Feb 17, 2026
UserVmVO vm = userVmDao.findById(vmId);
String cantHandleLog = String.format("Default VM snapshot cannot handle VM snapshot for [%s]", vm);

if (isRunningVMVolumeOnCLVMStorage(vm, cantHandleLog)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Pearl1594
what's the image format on CLVM ? RAW or QCOW2 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 17, 2026

Codecov Report

❌ Patch coverage is 28.73226% with 1456 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.07%. Comparing base (1bff543) to head (a923bdb).

Files with missing lines Patch % Lines
.../hypervisor/kvm/storage/LibvirtStorageAdaptor.java 20.05% 547 Missing and 7 partials ⚠️
...ud/hypervisor/kvm/storage/KVMStorageProcessor.java 44.71% 131 Missing and 5 partials ⚠️
...ervisor/kvm/resource/LibvirtComputingResource.java 11.97% 121 Missing and 4 partials ⚠️
...stack/engine/orchestration/VolumeOrchestrator.java 21.73% 70 Missing and 2 partials ⚠️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java 31.25% 62 Missing and 4 partials ⚠️
...resource/wrapper/LibvirtMigrateCommandWrapper.java 13.23% 52 Missing and 7 partials ⚠️
...torage/motion/StorageSystemDataMotionStrategy.java 0.00% 47 Missing ⚠️
...n/java/com/cloud/vm/VirtualMachineManagerImpl.java 16.36% 45 Missing and 1 partial ⚠️
...wrapper/LibvirtClvmLockTransferCommandWrapper.java 4.76% 40 Missing ⚠️
...rce/wrapper/LibvirtPreMigrationCommandWrapper.java 7.40% 25 Missing ⚠️
... and 29 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #12617      +/-   ##
============================================
+ Coverage     18.02%   18.07%   +0.05%     
- Complexity    16460    16594     +134     
============================================
  Files          5968     5976       +8     
  Lines        537213   539165    +1952     
  Branches      65975    66284     +309     
============================================
+ Hits          96825    97452     +627     
- Misses       429469   430720    +1251     
- Partials      10919    10993      +74     
Flag Coverage Δ
uitests 3.53% <ø> (-0.01%) ⬇️
unittests 19.24% <28.73%> (+0.05%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@Pearl1594
Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@Pearl1594 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.

@blueorangutan
Copy link
Copy Markdown

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

@Pearl1594
Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@Pearl1594 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.

@blueorangutan
Copy link
Copy Markdown

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

@blueorangutan
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a set of management-server, KVM-agent, UI, and scripting changes to improve Clustered LVM (CLVM) behavior and add initial support for a new CLVM_NG storage pool type, focusing on lock handling, migration behavior, snapshots/backup, and secure deletion behavior.

Changes:

  • Add CLVM_NG storage pool type and URL scheme support across MS/UI and primary storage lifecycle.
  • Introduce CLVM lock tracking/transfer (lightweight “migration” via lock move) and wire it into volume attach, VM/volume migration flows, and endpoint selection.
  • Update KVM snapshot management script and KVM storage code paths for CLVM/CLVM_NG (QCOW2-on-block-device, template backing files, secure zero-fill propagation).

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
utils/src/main/java/com/cloud/utils/UriUtils.java Adds URI parsing for clvm:// and clvm_ng:// URLs.
ui/src/views/infra/AddPrimaryStorage.vue UI support for selecting CLVM_NG and building CLVM_NG URLs.
server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java Adds mock for ClvmLockManager in tests.
server/src/test/java/com/cloud/storage/ClvmLockManagerTest.java New unit tests for CLVM lock manager behavior.
server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml Registers clvmLockManager bean.
server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java Blocks VM snapshots on CLVM_NG pools (until later phase).
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java Removes snapshot-store ref after backup for CLVM/CLVM_NG.
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Adds CLVM secure zero-fill config + CLVM lock-transfer logic during attach and expunge.
server/src/main/java/com/cloud/storage/StorageManagerImpl.java Adds scheme param + validates clvm/clvm_ng URL paths.
server/src/main/java/com/cloud/storage/ClvmLockManager.java New manager for tracking and transferring CLVM exclusive locks.
scripts/storage/qcow2/managesnapshot.sh Refactors LVM snapshot handling; adds CLVM_NG QCOW2-on-block logic for backup/revert.
pom.xml Updates Sonar exclusions (currently incorrectly).
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java Adds clvm_ng scheme mapping to StoragePoolType.CLVM_NG.
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java Adjusts endpoint selection for snapshot revert with CLVM.
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java Updates modify-storage-pool tests with CLVM zero-fill detail.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java Treats CLVM_NG as external-snapshot capable; adds setType.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java Major CLVM/CLVM_NG handling (virtual pools, VG stats, direct block access, QCOW2-on-LVM, secure zero-fill).
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java CLVM/CLVM_NG template copy, snapshot backup/delete paths, block-disk attach handling.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java Fixes pool type resolution for CLVM/CLVM_NG; CLVM_NG template/disk creation paths.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java Adds CLVM_SECURE_ZERO_FILL detail key constant.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java Enables managesnapshot-based revert for CLVM_NG.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java Ensures CLVM_NG uses qemu-img resizing path.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java Activates CLVM volumes shared-mode on destination during migration prep.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPreMigrationCommandWrapper.java New source-host pre-migration wrapper to switch CLVM volumes to shared mode.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostMigrationCommandWrapper.java New destination-host post-migration wrapper to claim exclusive locks.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtModifyStoragePoolCommandWrapper.java Ensures CLVM secure zero-fill detail has a default.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java Filters CLVM/CLVM_NG disks from libvirt storage migration list; manages CLVM volume states on success/failure.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java New agent wrapper to execute lvchange operations for lock transfer.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java Adds CLVM migration volume-state helpers and CLVM_NG QCOW2 block-format handling.
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java Adds destination-host hint support (persisted via volume_details).
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java Propagates CLVM secure zero-fill setting on host connect.
engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java Adds CLVM-aware endpoint routing based on destination host hint and lock holder.
engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategyTest.java Adds tests for CLVM snapshot-store-on-secondary behavior.
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java Rejects VM snapshots when running VM volumes are on CLVM storage.
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java Rejects VM snapshots when running VM volumes are on CLVM storage.
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java Allows delete strategy when CLVM snapshots exist on secondary in same zone.
engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java Adds CLVM/CLVM_NG to supported pool types in tests.
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java Adds CLVM/CLVM_NG handling for migration disk info and pre-migration command.
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java Generates CLVM(/NG) /dev/<vg>/<lv> dest paths and supports CLVM types.
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java Changes search criteria operator for state exclusion.
engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java Adds tests for CLVM lock transfer on VM start.
engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java Adds tests for CLVM lock host updates after VM migration.
engine/orchestration/src/main/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml Registers clvmLockManager bean.
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java Adds CLVM destination-host hinting, lock updates after migrations, lock transfer at VM start.
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java Sends pre/post-migration commands; updates CLVM lock host IDs after migration.
engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java Adds CLVM_LOCK_HOST_ID constant and destination-host hint API.
core/src/main/java/org/apache/cloudstack/storage/command/ClvmLockTransferCommand.java New agent command for CLVM lock transfer operations.
core/src/main/java/com/cloud/agent/api/PreMigrationCommand.java New agent command sent to source host before migration.
core/src/main/java/com/cloud/agent/api/PostMigrationCommand.java New agent command sent to destination host after migration.
core/src/main/java/com/cloud/agent/api/PostMigrationAnswer.java New answer type for post-migration command.
core/src/main/java/com/cloud/agent/api/MigrateCommand.java Adds destination pool type field to migrate disk info for filtering.
api/src/main/java/com/cloud/storage/Storage.java Introduces StoragePoolType.CLVM_NG.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +40 to 48
import com.cloud.storage.ClvmLockManager;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.Storage;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolHostVO;
import com.cloud.storage.StorageService;
import com.cloud.storage.VolumeApiServiceImpl;
import com.cloud.storage.dao.StoragePoolHostDao;
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

cloud-engine-storage-volume does not have a dependency on cloud-server, but this class imports com.cloud.storage.ClvmLockManager and com.cloud.storage.VolumeApiServiceImpl (both live under server/). This will fail compilation for the engine module. To fix, avoid referencing server-layer classes here (e.g., move CLVMSecureZeroFill ConfigKey and the CLVM pool-type helper into an api/core module, or query the config via configuration DAOs available in this module).

Copilot uses AI. Check for mistakes.
Comment on lines +957 to +983
private String findAccessibleDeviceNode(String volumeUuid, String vgName, KVMStoragePool pool) {
String lvPath = "/dev/" + vgName + "/" + volumeUuid;
File lvDevice = new File(lvPath);

if (!lvDevice.exists()) {
lvPath = tryDeviceMapperPath(volumeUuid, vgName, lvDevice);

if (!lvDevice.exists()) {
lvPath = handleMissingDeviceNode(volumeUuid, vgName, pool);
}
}

return lvPath;
}

private String tryDeviceMapperPath(String volumeUuid, String vgName, File lvDevice) {
String vgNameEscaped = vgName.replace("-", "--");
String volumeUuidEscaped = volumeUuid.replace("-", "--");
String mapperPath = "/dev/mapper/" + vgNameEscaped + "-" + volumeUuidEscaped;
File mapperDevice = new File(mapperPath);

if (!mapperDevice.exists()) {
lvDevice = mapperDevice;
}

return mapperPath;
}
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

findAccessibleDeviceNode attempts to fall back to /dev/mapper/... if /dev/<vg>/<lv> doesn’t exist, but the logic in tryDeviceMapperPath is broken: it checks if (!mapperDevice.exists()) (reversed) and assigning to lvDevice has no effect on the caller. As a result, even if the mapper node exists, the method will still think it’s missing and may throw incorrectly. Rework this to check the mapper path existence and return the correct path (or return a File/boolean) so the fallback actually works.

Copilot uses AI. Check for mistakes.
Comment on lines 33 to +38
import com.cloud.dc.DedicatedResourceVO;
import com.cloud.dc.dao.DedicatedResourceDao;
import com.cloud.storage.ClvmLockManager;
import com.cloud.storage.Storage;
import com.cloud.storage.VolumeDetailVO;
import com.cloud.storage.dao.VolumeDetailsDao;
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

This module (cloud-engine-storage) does not depend on cloud-server (see the commented-out dependency in engine/storage/pom.xml), but this file imports com.cloud.storage.ClvmLockManager which is defined under server/. That will break compilation for this module. Consider removing this dependency (e.g., inline the pool-type check here), or move ClvmLockManager (or at least its isClvmPoolType helper) into a module that cloud-engine-storage depends on (api/core/engine).

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
23.4% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube Cloud

Sign up for free to 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.

5 participants