[winterhazel]

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

1. 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.

2. 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.

3. 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.

4. 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

1. 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.

2. 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.

3. 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.

4. 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.

5. 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.

6. 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]

@blueorangutan package

[blueorangutan]

@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]

Codecov Report

❌ Patch coverage is 69.40789% with 186 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.86%. Comparing base (f3cb957) to head (664fb71).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...oud/hypervisor/vmware/resource/VmwareResource.java	66.46%	52 Missing and 4 partials ⚠️
...oud/hypervisor/vmware/mo/HypervisorHostHelper.java	0.00%	47 Missing ⚠️
...m/cloud/hypervisor/vmware/mo/VirtualMachineMO.java	81.56%	23 Missing and 10 partials ⚠️
...com/cloud/hypervisor/vmware/util/VmwareHelper.java	86.01%	15 Missing and 5 partials ⚠️
...cloud/storage/resource/VmwareStorageProcessor.java	0.00%	7 Missing ⚠️
...java/com/cloud/hypervisor/vmware/mo/ClusterMO.java	0.00%	7 Missing ⚠️
.../org/apache/cloudstack/util/vmware/VMwareUtil.java	0.00%	4 Missing ⚠️
...ain/java/com/cloud/api/query/QueryManagerImpl.java	0.00%	3 Missing ⚠️
.../src/main/java/com/cloud/vm/UserVmManagerImpl.java	0.00%	3 Missing ⚠️
...cloud/hypervisor/vmware/mo/DiskControllerType.java	93.02%	3 Missing ⚠️
... and 2 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #10454      +/-   ##
============================================
+ Coverage     20.78%   20.86%   +0.07%     
- Complexity    21928    22062     +134     
============================================
  Files          6496     6496              
  Lines        591657   591574      -83     
  Branches      72592    72529      -63     
============================================
+ Hits         122956   123409     +453     
+ Misses       455356   454801     -555     
- Partials      13345    13364      +19     

Flag	Coverage Δ
uitests	3.88% <ø> (ø)
unittests	22.17% <69.40%> (+0.08%)	⬆️

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]

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

[winterhazel]

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

[DaanHoogland]

@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]

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

[winterhazel]

@blueorangutan package

[blueorangutan]

@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]

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

[github-actions]

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

[JoaoJandre]

@winterhazel could you fix the conflicts?

[winterhazel]

@blueorangutan package

[blueorangutan]

@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]

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

[Copilot]

Pull Request Overview

This PR refactors VMware disk controller handling by introducing a dynamic disk controller mapping backed by a new database table, adds support for SATA and NVMe controllers, and updates all callers to use the new DiskControllerMappingVO model.

• Centralize disk controller metadata in a new disk_controller_mapping table, DAO, VO, and loader logic.
• Refactor VmwareResource and related classes to use DiskControllerMappingVO instead of static enums, with full end-to-end support in start/attach flows.
• Update API, schema, manager, and tests to load and propagate supported disk controllers at runtime.

Reviewed Changes

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

Show a summary per file

File	Description
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/ClusterMO.java	Change createBlankVm signature to use DiskControllerMappingVO.
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/BaseMO.java	Add protected no-arg constructor
vmware-base/pom.xml	Add dependency on cloud-engine-schema
services/secondary-storage/controller/.../SecondaryStorageManagerImpl.java	Inject DiskControllerMappingDao and set supported controllers on setup
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java	Replace hard-coded controller lists with dynamic DAO lookup
plugins/hypervisors/vmware/.../VmwareStorageProcessor.java	Simplify attachVolume to use DiskControllerMappingVO
plugins/hypervisors/vmware/.../VmwareSecondaryStorageResourceHandler.java	Handle new SecStorageVMSetupCommand
plugins/hypervisors/vmware/.../VmwareResource.java	Major refactor to use DiskControllerMappingVO for disk controller ops
plugins/hypervisors/vmware/.../VmwareManagerImpl.java	Load mappings at configuration time
plugins/hypervisors/vmware/.../VmwareVmImplementer.java	Update default root-controller detail to use enum
engine/schema/.../schema-42010to42100.sql	Create disk_controller_mapping table and seed default mappings
engine/schema/src/.../DiskControllerMappingDaoImpl.java & DiskControllerMappingDao.java	New DAO for disk controller mapping
engine/schema/src/.../DiskControllerMappingVO.java	New JPA entity for disk controller mapping
core/src/main/java/com/cloud/agent/api/SecStorageVMSetupCommand.java	Add field to carry supported disk controllers

[winterhazel]

@blueorangutan package

[blueorangutan]

@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]

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

[github-actions]

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

[winterhazel]

@blueorangutan package

[blueorangutan]

@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]

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

[github-actions]

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

[JoaoJandre]

@winterhazel could you fix the conflicts here?

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland 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]

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

[DaanHoogland]

@winterhazel , some unit test failures during build:

14:02:36 [ERROR] Failures: 
14:02:36 [ERROR]   VmwareHelperTest.configureDiskControllerMappingsInVmwareBaseModuleTestLsiLogicIsConfigured:458 expected:<[{"busName":null,"controllerReference":"com.vmware.vim25.VirtualLsiLogicController","hypervisor":null,"maxControllerCount":null,"maxDeviceCount":null,"minHardwareVersion":null,"name":"lsilogic","vmdkAdapterType":null}]> but was:<[]>
14:02:36 [ERROR] Errors: 
14:02:36 [ERROR]   VmwareHelperTest.getRequiredDiskControllersTestRequiresAllControllersWhenInstanceIsNotSystemVm:250 ? NullPointer
14:02:36 [ERROR]   VmwareHelperTest.getRequiredDiskControllersTestRequiresOnlyRootDiskControllerWhenInstanceIsSystemVm:260 ? NullPointer

[SadiJr]

LGTM, I tested locally, all tests worked and each volume was mapped correctly to the expected controller.

[weizhouapache]

@nvazquez @harikrishna-patnala @DaanHoogland
does this look good to you ?

[winterhazel]

@nvazquez @harikrishna-patnala @DaanHoogland does this look good to you ?

@weizhouapache I converted this one to a draft. There are some changes I now want to do here.

[github-actions]

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

[nvazquez]

Hi @winterhazel is this PR still in progress?

[winterhazel]

Hi @winterhazel is this PR still in progress?

@nvazquez it was ready, but the main change here does not make sense anymore. I'll remove it to just keep the refactorings I made.

[nvazquez]

Thanks @winterhazel - please advise when it is ready

[winterhazel]

@blueorangutan package

[blueorangutan]

@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]

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