Skip to content

importVM improvements: RBD support, volume format, and cluster selection - #14162

Open
abh1sar wants to merge 4 commits into
apache:4.22from
shapeblue:importvm-impr
Open

abh1sar wants to merge 4 commits into
apache:4.22from
shapeblue:importvm-impr

Conversation

@abh1sar

@abh1sar abh1sar commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes 3 issues in the importVM workflow:

1. Root disk import doesn't work for RBD pools unlike data disk import

importVolume / listVolumesForImport already accept RBD. LibvirtGetVolumesOnStorageCommandWrapper has handled it all along, including building the rbd: URI for qemu-img info. But importVm importsource=shared rejects it, because the neighbouring LibvirtCheckVolumeCommandWrapper was written with a file-only pool list. An operator on Ceph can import a data disk but not a root disk.

The failure is also misleading. The agent answers "Unsupported Storage Pool", but the operator sees:

CloudRuntimeException: Disk not found or is invalid

and the VM is rolled back.

Adds StoragePoolType.RBD to the check wrapper's supported list, takes the virtual size from the disk libvirt already resolved (qemu-img cannot open a bare RBD image name), skips the QCOW2 header check for raw RBD images, and builds the rbd: URI via the existing KVMPhysicalDisk.RBDStringBuilder.

diskpath on importVm and path on importVolume are the file name on file-based pools and the image name on RBD. Both parameter descriptions now say so.

2. An imported volume records the wrong image format

importVolume and updateImportedVolume both stamped the cluster default format for the hypervisor, so a volume imported from an RBD pool was recorded QCOW2 while a natively deployed volume on the same pool is RAW. Both entry points were affected: root disks via importVm importsource=shared, and data disks via importVolume.

They now record the format the hypervisor reported for the image, from the check answer for a root disk and from the volume listed on the pool for a data disk, falling back to the hypervisor default when none is reported. This also corrects a raw image imported from a file-based pool, so it is a general correctness fix rather than an RBD-specific one.

3. The import is planned outside the pool's cluster

importKVMInstanceFromDiskImage planned with pod and cluster unset, so the planner could pick any host in the zone by capacity, even one in a cluster with no access to the pool the caller named. The volume check then ran against whichever pool that cluster does have, and the import failed with "Disk not found or is invalid" although the image was fine.
This is not RBD-specific. Reproduced on a two-cluster zone with plain NFS.

The plan now takes pod and cluster from the pool the caller passed. Both are null for a zone-wide pool, which every host can see, so that case plans exactly as before.

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?

  1. Deployed a VM with a root and data disk on RBD pool
  2. Unmanage it
  3. Import the VM
  4. Checked the guest OS
  5. Took volume snapshots on imported volumes
  6. No failures or errors were observed
  7. Verified the format of the root and data disk was raw
  8. Verified on a two cluster setup that importVM always selects the host that has access to the storage pool.

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

The CheckVolumeCommand wrapper on the KVM agent only accepted file based
pools, so importing a root disk straight from Ceph failed on the agent
with "Unsupported Storage Pool" and surfaced as "Disk not found or is
invalid" on the management server. Add RBD to the supported pool types,
take the virtual size from the disk libvirt resolved (qemu-img cannot
open a bare RBD image name), skip the QCOW2 header check for raw RBD
images, and build the rbd: URI when running qemu-img info, the same way
LibvirtGetVolumesOnStorageCommandWrapper already does for
listVolumesForImport.
importVolume and updateImportedVolume both stamped the cluster default
format for the hypervisor, so a volume imported from an RBD pool was
recorded as QCOW2 while a natively deployed volume on the same pool is
RAW. This affected both entry points: importVm importsource=shared for a
root disk, and importVolume for a data disk.

Pass the format the hypervisor reported for the existing image, from the
check answer for a root disk and from the volume listed on the pool for
a data disk, and fall back to the hypervisor default only when no format
is reported. This also corrects a raw image imported from a file based
pool.
…ted pool

importKVMInstanceFromDiskImage planned with the pod and cluster unset, so
the planner was free to pick any host in the zone by capacity. When it
picked a host in a cluster that cannot see the pool the caller named, the
volume check ran against whichever pool that cluster does have, and the
import failed with "Disk not found or is invalid" although the image was
fine.

Take the pod and cluster from the pool the caller passed, the same way
importVolume already derives its host from the pool's scope
@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.45455% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.94%. Comparing base (2e450cb) to head (8f49d1a).
⚠️ Report is 2 commits behind head on 4.22.

Files with missing lines Patch % Lines
...urce/wrapper/LibvirtCheckVolumeCommandWrapper.java 0.00% 13 Missing ⚠️
...stack/engine/orchestration/VolumeOrchestrator.java 25.00% 2 Missing and 1 partial ⚠️
.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java 89.28% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               4.22   #14162   +/-   ##
=========================================
  Coverage     17.93%   17.94%           
- Complexity    16143    16154   +11     
=========================================
  Files          5928     5928           
  Lines        535174   535223   +49     
  Branches      65494    65507   +13     
=========================================
+ Hits          95998    96044   +46     
+ Misses       428253   428246    -7     
- Partials      10923    10933   +10     
Flag Coverage Δ
uitests 4.02% <ø> (ø)
unittests 19.01% <65.45%> (+<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.

@weizhouapache

Copy link
Copy Markdown
Member

nice improvements and fixed @abh1sar

I was thinking of reopening #13365, which is not needed in favor of this.

@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

return new CheckVolumeAnswer(command, false, "", 0, getVolumeDetails(pool, vol));
final boolean isRbd = Storage.StoragePoolType.RBD.equals(storageFilerTO.getType());

if (!isRbd) {

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.

can we check RBD volumes too ?

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.

qemu can't check raw files, but I have added code to verify that getVolumeDetails is able to read the rbd volume as a check.
8f49d1a

@blueorangutan

Copy link
Copy Markdown

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

@abh1sar

abh1sar commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@weizhouapache 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-16985)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 47797 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14162-t16985-kvm-ol8.zip
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian Build Failed (tid-16987)

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.

3 participants