[VMware] Disk controller mappings by winterhazel · Pull Request #10454 · apache/cloudstack

winterhazel · 2025-02-24T17:27:01Z

Description

This is a refactor of the disk controller related logic for VMware that also adds support for SATA and NVME controllers.

A detailed description of these changes is available at https://cwiki.apache.org/confluence/display/CLOUDSTACK/Disk+Controller+Mappings.

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

How Has This Been Tested?

The tests below were performed for VMs with the following rootDiskController and dataDiskController configurations:

osdefault/osdefault (converted to lsilogic/lsilogic)
ide/ide
pvscsi/pvscsi
sata/sata
nvme/nvme
sata/lsilogic
ide/osdefault
osdefault/ide

VM deployment: I deployed one VM with each of the configurations. I verified in vCenter that they had the correct amount of disk controllers, and that each volume was associated to the expected controller. The sata/lsilogic VM was the only one that had a data disk; the others only had a root disk.
VM start: I stopped the VMs deployed in (1) and started them again. I verified in vCenter that they had the correct amount of disk controllers, and that each volume was associated to the expected controller.
Disk attachment: while the VMs were running, I tried to attach a data disk. All the data disks were attached successfully (expect for the VMs using IDE as the data disk controller, which does not allow hot plugging disks; for these, I attached the disks after stopping the VM). I verified that all the disks were using the expected controller. Then, I stopped and started the VM, and verified that they were still using the expected controllers. Finally, I stoped the VMs and detached the volumes. I verified that they were detached successfully.
VM import: I unmanaged the VMs and imported them back. I verified that their settings were infered successfully according to the existing disk controllers. Then, I started the VMs, and verified that the controllers and the volumes were configured correctly.

The next tests were performed using the following imported VMs:

osdefault/osdefault
ide/ide
nvme/nvme
sata/lsilogic

Volume migration: I migrated the volumes from NFS to local storage, and verified that the migration finished successfully. Then, I started the VMs and verified that both the controllers and the disks were configured correctly.
Volume resize: I expanded all of the disks, and verified in vCenter that their size was changed. Then, I started the VMs and verified that both the controllers and the disks were configured correctly.
VM snapshot: I took some VM snapshots, started the VMs and verified that everything was ok. I changed the configurations of the VM using osdefault/osdefault to sata/sata and started the VM to begin the reconfiguration process. I verified that the disk controllers in use were not removed, and that the disks were still associated with the previous controllers; however, the SATA controllers were also created. The VM was working as expected. Finally, I deleted the VM snapshots.
Template creation from volume: I created templates from the root disks. Then, I deployed VMs from the templates. I verified that all the VMs had the same disk controllers as the original VM, and that the only existing disk was correctly associated with the configured root disk controller.
Template creation from volume snapshot: I took snapshots from the root disks, and created templates from the snapshots. Then, I deployed VMs from the templates. I verified that all the VMs had the same disk controllers as the original VM, and that the only existing disk was correctly associated with the configured root disk controller.
VM scale: with the VMs stopped, I scaled the VM from Small Instance to Medium Instance. I verified that the offering was changed. I started the VMs, and verified that the VMs were correctly reconfigured in vCenter.

Other tests:

System VM creation: after applying the patches, I recreated the SSVM and the CPVM. I verified that they were using a single LSI Logic controller. I also verified the controllers of a new VR and of an existing VR.
I attached 3 disks to the ide/ide controller. When trying to attach a 4th disk, I got an expected exception, as the IDE bus reached the maximum amount of devices (the 4th one was the CD/DVD drive).
I removed all the disks from the sata/lsilogic VM. I tried to attach the root disk again, and verified that it was attached successfully. I started the VM, and verified that it was configured correctly.
I attached 8 disks to the pvscsi/pvscsi VM, and verified that the 8th disk was successfully attached to device number 8 (device number 7 is reserved for the controller).

winterhazel · 2025-02-24T17:27:22Z

@blueorangutan package

blueorangutan · 2025-02-24T17:28:03Z

@winterhazel 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 · 2025-02-24T17:34:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 3.52%. Comparing base (5d61ba3) to head (adce7af).
⚠️ Report is 1 commits behind head on main.

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

HEAD has 1 upload less than BASE

Flag BASE (5d61ba3) HEAD (adce7af)

unittests 1 0

Additional details and impacted files

@@              Coverage Diff              @@
##               main   #10454       +/-   ##
=============================================
- Coverage     18.02%    3.52%   -14.50%     
=============================================
  Files          5973      464     -5509     
  Lines        537466    40063   -497403     
  Branches      65991     7534    -58457     
=============================================
- Hits          96855     1414    -95441     
+ Misses       429689    38461   -391228     
+ Partials      10922      188    -10734

Flag	Coverage Δ
uitests	`3.52% <ø> (ø)`
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.

blueorangutan · 2025-02-24T17:55:11Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 12549

winterhazel · 2025-02-24T18:21:08Z

@DaanHoogland it seems there were some merge issues in main. org.apache.cloudstack.backup.VeeamBackupProvider is missing some methods and imports.

DaanHoogland · 2025-02-24T19:25:02Z

@DaanHoogland it seems there were some merge issues in main. org.apache.cloudstack.backup.VeeamBackupProvider is missing some methods and imports.

I'll check and update

DaanHoogland · 2025-02-24T20:10:28Z

@winterhazel , please see #10457 . I have had no time (or infra) to test yet.

winterhazel · 2025-02-26T17:59:38Z

@blueorangutan package

blueorangutan · 2025-02-26T18:00:03Z

@winterhazel 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 · 2025-02-26T19:25:55Z

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

github-actions · 2025-04-16T16:41:29Z

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

JoaoJandre · 2025-06-02T20:03:13Z

@winterhazel could you fix the conflicts?

winterhazel · 2025-06-04T16:06:04Z

@blueorangutan package

blueorangutan · 2025-06-04T16:08:03Z

@winterhazel 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 · 2025-06-04T17:16:51Z

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

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.

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

engine/schema/src/main/java/org/apache/cloudstack/storage/DiskControllerMappingVO.java

vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java

...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java

vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareHelperTest.java

blueorangutan · 2026-03-30T10:25:56Z

[SF] Trillian Build Failed (tid-15764)

blueorangutan · 2026-03-30T10:30:50Z

[SF] Trillian Build Failed (tid-15762)

blueorangutan · 2026-03-30T10:53:13Z

[SF] Trillian Build Failed (tid-15763)

blueorangutan · 2026-03-31T03:06:06Z

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

Test	Result	Time (s)	Test File

DaanHoogland · 2026-03-31T08:07:15Z

@blueorangutan test ol9 vmware-80u3

blueorangutan · 2026-03-31T08:08:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol9 mgmt + vmware-80u3) has been kicked to run smoke tests

blueorangutan · 2026-03-31T08:24:28Z

[SF] Trillian Build Failed (tid-15778)

blueorangutan · 2026-03-31T09:00:56Z

[SF] Trillian Build Failed (tid-15783)

github-actions · 2026-03-31T14:09:29Z

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

DaanHoogland · 2026-03-31T15:30:24Z

conflicts @winterhazel

don’t worry about the failed env builds, those are due to an upstream epel repo not being ready. It is running now in the backend.

…pings

winterhazel · 2026-03-31T20:06:05Z

@blueorangutan package

blueorangutan · 2026-03-31T20:08:04Z

@winterhazel 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 · 2026-03-31T20:36:16Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17316

blueorangutan · 2026-04-01T03:02:32Z

[SF] Trillian test result (tid-15788)
Environment: vmware-70u3 (x2), zone: Advanced Networking with Mgmt server ol9
Total time taken: 46772 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10454-t15788-vmware-70u3.zip
Smoke tests completed. 133 look OK, 7 have errors, 11 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_CRUD_operations_guest_OS_mapping	`Error`	2.40	test_guest_os.py
test_guest_OS_mapping_check_with_hypervisor	`Error`	2.47	test_guest_os.py
ContextSuite context=TestListIdsParams>:setup	`Error`	0.00	test_list_ids_parameter.py
test_01_snapshot_root_disk	`Error`	3.20	test_snapshots.py
test_02_list_snapshots_with_removed_data_store	`Error`	67.90	test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:setup	`Error`	186.15	test_snapshots.py
test_11_destroy_ssvm	`Error`	7.27	test_ssvm.py
test_01_create_template	`Error`	1.18	test_templates.py
test_CreateTemplateWithDuplicateName	`Error`	1.21	test_templates.py
ContextSuite context=TestTemplates>:setup	`Error`	104.96	test_templates.py
test_01_snapshot_usage	`Error`	3.21	test_usage.py
test_01_template_usage	`Error`	1.52	test_usage.py
test_03_live_migrate_VM_with_two_data_disks	`Error`	41.02	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	24.12	test_vm_life_cycle.py
all_test_vnf_templates	`Skipped`	---	test_vnf_templates.py
all_test_volumes	`Skipped`	---	test_volumes.py
all_test_vpc_conserve_mode	`Skipped`	---	test_vpc_conserve_mode.py
all_test_vpc_ipv6	`Skipped`	---	test_vpc_ipv6.py
all_test_vpc_redundant	`Skipped`	---	test_vpc_redundant.py
all_test_vpc_router_nics	`Skipped`	---	test_vpc_router_nics.py
all_test_vpc_vpn	`Skipped`	---	test_vpc_vpn.py
all_test_webhook_delivery	`Skipped`	---	test_webhook_delivery.py
all_test_webhook_lifecycle	`Skipped`	---	test_webhook_lifecycle.py
all_test_host_maintenance	`Skipped`	---	test_host_maintenance.py
all_test_hostha_kvm	`Skipped`	---	test_hostha_kvm.py

DaanHoogland · 2026-04-01T12:58:48Z

@blueorangutan test ol9 vmware-80u3

blueorangutan · 2026-04-01T13:00:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol9 mgmt + vmware-80u3) has been kicked to run smoke tests

winterhazel added 3 commits February 24, 2025 10:41

Disk controller mappings

f834504

Merge branch 'main' into disk-controller-mappings

5459efe

Fix merge errors

ad7ca02

boring-cyborg bot added component:api component:build component:secondary-storage component:ssvm component:storage component:vmware labels Feb 24, 2025

winterhazel changed the title ~~Disk controller mappings~~ [VMware] Disk controller mappings Feb 24, 2025

Merge branch 'main' into disk-controller-mappings

de794a0

github-actions bot added the status:has-conflicts label Apr 16, 2025

Merge branch 'main' into disk-controller-mappings

7476f60

github-actions bot removed the status:has-conflicts label Jun 4, 2025

sureshanaparti requested a review from nvazquez June 5, 2025 09:30

Copilot AI reviewed Mar 30, 2026

View reviewed changes

github-actions bot added the status:has-conflicts label Mar 31, 2026

winterhazel requested a review from jbampton as a code owner March 31, 2026 18:23

boring-cyborg bot added component:compute component:integration component:orchestration labels Mar 31, 2026

Merge remote-tracking branch 'upstream/main' into disk-controller-map…

bb64294

…pings

winterhazel force-pushed the disk-controller-mappings branch from 079083f to bb64294 Compare March 31, 2026 18:26

winterhazel removed the request for review from jbampton March 31, 2026 18:26

winterhazel removed component:compute component:orchestration component:integration labels Mar 31, 2026

github-actions bot removed the status:has-conflicts label Mar 31, 2026

Address Copilot reviews

adce7af

Conversation

winterhazel commented Feb 24, 2025

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

winterhazel commented Feb 24, 2025

Uh oh!

blueorangutan commented Feb 24, 2025

Uh oh!

codecov bot commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Feb 24, 2025

Uh oh!

winterhazel commented Feb 24, 2025

Uh oh!

DaanHoogland commented Feb 24, 2025

Uh oh!

DaanHoogland commented Feb 24, 2025

Uh oh!

winterhazel commented Feb 26, 2025

Uh oh!

blueorangutan commented Feb 26, 2025

Uh oh!

blueorangutan commented Feb 26, 2025

Uh oh!

github-actions bot commented Apr 16, 2025

Uh oh!

JoaoJandre commented Jun 2, 2025

Uh oh!

winterhazel commented Jun 4, 2025

Uh oh!

blueorangutan commented Jun 4, 2025

Uh oh!

blueorangutan commented Jun 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blueorangutan commented Mar 30, 2026

Uh oh!

blueorangutan commented Mar 30, 2026

Uh oh!

blueorangutan commented Mar 30, 2026

Uh oh!

blueorangutan commented Mar 31, 2026

Uh oh!

DaanHoogland commented Mar 31, 2026

Uh oh!

blueorangutan commented Mar 31, 2026

Uh oh!

blueorangutan commented Mar 31, 2026

Uh oh!

blueorangutan commented Mar 31, 2026

Uh oh!

github-actions bot commented Mar 31, 2026

Uh oh!

DaanHoogland commented Mar 31, 2026

Uh oh!

winterhazel commented Mar 31, 2026

Uh oh!

blueorangutan commented Mar 31, 2026

Uh oh!

blueorangutan commented Mar 31, 2026

Uh oh!

blueorangutan commented Apr 1, 2026

Uh oh!

DaanHoogland commented Apr 1, 2026

codecov bot commented Feb 24, 2025 •

edited

Loading