kvm: allow skip forcing disk controller #11750

shwstppr · 2025-09-29T10:41:08Z

Description

A VM or template detail can be added with key skip.force.disk.controller and value true to allow skipping forcing disk controllers for the VM especially in case of UEFI VMs.
Otherwise, current behaviour of disk controllers depend on the guest OS, UEFI secure boot where Windows VM may always be provisioned with sata and other OS VMs with virtio.

Doc PR: apache/cloudstack-documentation#616

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

Screenshots (if appropriate):

How Has This Been Tested?

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

shwstppr · 2025-09-29T10:51:23Z

@blueorangutan package

blueorangutan · 2025-09-29T10:52:03Z

@shwstppr 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-09-29T11:46:15Z

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

weizhouapache · 2025-09-29T12:18:37Z

@blueorangutan package

blueorangutan · 2025-09-29T12:20:04Z

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

blueorangutan · 2025-09-29T13:35:07Z

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

vishesh92 · 2025-09-30T13:01:58Z

@blueorangutan package

Copilot

Pull Request Overview

This PR adds functionality to allow skipping disk controller forcing for KVM VMs by introducing a new VM detail flag. The enhancement improves flexibility for UEFI VMs that may have specific disk controller requirements.

Key changes:

Introduces a new VM detail constant skip.force.disk.controller to bypass default disk controller logic
Extracts disk definition logic into a separate method for better testability and maintainability
Adds comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
VmDetailConstants.java	Adds new constant for the skip force disk controller feature
LibvirtComputingResource.java	Refactors disk definition logic and implements the skip functionality
LibvirtComputingResourceTest.java	Adds test coverage for the new disk definition method

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

codecov · 2025-10-01T04:03:41Z

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.23%. Comparing base (750290b) to head (88d9334).
⚠️ Report is 3 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...ervisor/kvm/resource/LibvirtComputingResource.java	78.57%	1 Missing and 2 partials ⚠️
...ain/java/com/cloud/api/query/QueryManagerImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #11750      +/-   ##
============================================
- Coverage     16.23%   16.23%   -0.01%     
- Complexity    13378    13380       +2     
============================================
  Files          5657     5657              
  Lines        498932   498942      +10     
  Branches      60552    60554       +2     
============================================
- Hits          81016    81005      -11     
- Misses       408882   408902      +20     
- Partials       9034     9035       +1

Flag	Coverage Δ
uitests	`4.00% <ø> (ø)`
unittests	`17.09% <73.33%> (-0.01%)`	⬇️

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.

vishesh92

clgtm

vishesh92 · 2025-10-01T06:36:34Z

@blueorangutan package

blueorangutan · 2025-10-01T06:38:04Z

@vishesh92 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-10-01T08:02:17Z

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

weizhouapache · 2025-10-03T07:09:39Z

@blueorangutan test

blueorangutan · 2025-10-03T07:10:04Z

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

blueorangutan · 2025-10-03T20:45:50Z

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

Test	Result	Time (s)	Test File

pavanaravapalli · 2025-10-18T08:11:01Z

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

+                    DiskDef.DiskBus diskBusType, DiskDef.DiskBus diskBusTypeData, Map<String, String> details) {
+        boolean skipForceDiskController = MapUtils.getBoolean(details, VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER,
+                false);
+        if (skipForceDiskController) {


@shwstppr
This condition forces the root disk controller to use its configured settings instead of the UEFI workflow settings, even when the VM is deployed with UEFI and this setting "skip.force.disk.controller" is enabled. Consequently, a UEFI-deployed VM may fail to boot because some guest operating systems still depend on SATA disk. Please include a comment or a note detailing the change in behavior."

RosiKyu · 2025-12-17T00:23:50Z

@blueorangutan package

blueorangutan · 2025-12-17T00:26:06Z

@RosiKyu 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-12-17T01:55:26Z

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

Fixes apache#9460 A VM or template detail can be added with key `skip.force.disk.controller` and value `true` to allow skipping forcing disk controllers for the VM especially in case of UEFI VMs. Otherwise, current behaviour of disk controllers depend on the guest OS, UEFI secure boot where Windows VM may always be provisioned with `sata` and other OS VMs with `virtio`. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2026-01-07T11:14:11Z

@blueorangutan package

blueorangutan · 2026-01-07T11:16:04Z

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

Related apache/cloudstack#11750 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

blueorangutan · 2026-01-07T12:30:23Z

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

RosiKyu · 2026-01-08T18:15:11Z

@blueorangutan test

blueorangutan · 2026-01-08T18:16:04Z

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

blueorangutan · 2026-01-09T10:10:09Z

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

Test	Result	Time (s)	Test File

RosiKyu

LGTM

Test Results

PR: #11750 - kvm: allow skip forcing disk controller
Issue: #9460
Tested on: CloudStack 4.20.3.0-SNAPSHOT
Environment: KVM (Oracle Linux 9), 2 hosts

API Tests

Test	Description	Status
TC-001	API detail availability via listDetailOptions	$\color{green}{\textsf{PASSED}}$
TC-002	Windows UEFI + skip flag - VirtIO persistence (main fix)	$\color{green}{\textsf{PASSED}}$
TC-003	Windows UEFI default behavior (SATA enforced)	$\color{green}{\textsf{PASSED}}$
TC-004	Linux UEFI default behavior (VirtIO enforced)	$\color{green}{\textsf{PASSED}}$
TC-005	Data disk attachment with skip flag	$\color{green}{\textsf{PASSED}}$
TC-006	Windows BIOS VM - no regression	$\color{green}{\textsf{PASSED}}$
TC-007	Template-level skip.force.disk.controller setting	$\color{green}{\textsf{PASSED}}$
TC-008	Detail priority - Instance overrides Template	$\color{green}{\textsf{PASSED}}$
TC-009	VM live migration with skip flag	$\color{green}{\textsf{PASSED}}$
TC-010	Different disk bus types (virtio, scsi, sata)	$\color{green}{\textsf{PASSED}}$
TC-011	Null/empty details map handling	$\color{green}{\textsf{PASSED}}$

UI Tests

Test	Description	Status
UI-001	Verify skip.force.disk.controller appears in VM Settings	$\color{green}{\textsf{PASSED}}$
UI-002	Set skip.force.disk.controller via UI	$\color{green}{\textsf{PASSED}}$
UI-003	Verify Template Settings	$\color{green}{\textsf{PASSED}}$

Key Observations

Main bug fix verified: Windows UEFI Secure Boot VMs with skip.force.disk.controller=true maintain VirtIO disk controllers through stop/start cycles.
API: listDetailOptions requires a resourceid (VM UUID) parameter to return hypervisor-specific details - the hypervisor parameter alone is insufficient.
Detail Priority: Instance-level details override template-level details (standard CloudStack behavior).
No Regressions: Default UEFI behavior (SATA for Windows, VirtIO for Linux) preserved when skip flag is not set.

Detailed Test Results

Can be found in the following pdf: Detailed Test Results PR #11750.pdf

boring-cyborg bot added component:api component:kvm labels Sep 29, 2025

This was referenced Sep 29, 2025

kvm: fix disk controller for secure boot vm #10213

Closed

RootDiskController setting not Honored throughout Instance lifecycle #9460

Closed

shwstppr changed the base branch from main to 4.19 September 29, 2025 10:51

weizhouapache added this to the 4.20.2 milestone Sep 29, 2025

vishesh92 requested a review from Copilot September 30, 2025 13:01

Copilot AI reviewed Sep 30, 2025

View reviewed changes

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java Outdated Show resolved Hide resolved

shwstppr marked this pull request as ready for review September 30, 2025 13:21

shwstppr commented Oct 1, 2025

View reviewed changes

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java Outdated Show resolved Hide resolved

vishesh92 approved these changes Oct 1, 2025

View reviewed changes

DaanHoogland added the status:needs-testing label Oct 6, 2025

RosiKyu self-assigned this Oct 13, 2025

pavanaravapalli reviewed Oct 18, 2025

View reviewed changes

weizhouapache modified the milestones: 4.20.2, 4.20.3 Oct 23, 2025

DaanHoogland changed the base branch from 4.19 to 4.20 November 11, 2025 13:12

DaanHoogland changed the base branch from 4.20 to 4.19 November 11, 2025 13:14

DaanHoogland mentioned this pull request Nov 12, 2025

Change default RootDiskController from ide to scsi for VMWare guests #12041

Closed

DaanHoogland changed the base branch from 4.19 to 4.20 January 7, 2026 10:41

DaanHoogland changed the base branch from 4.20 to 4.19 January 7, 2026 10:42

shwstppr force-pushed the fix-kvm-securebootvm-diskcontroller branch from dfa2832 to d1d9890 Compare January 7, 2026 11:00

shwstppr added 2 commits January 7, 2026 16:40

address comment

88d9334

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr force-pushed the fix-kvm-securebootvm-diskcontroller branch from d1d9890 to 88d9334 Compare January 7, 2026 11:13

boring-cyborg bot added component:agent component:build component:integration labels Jan 7, 2026

shwstppr changed the base branch from 4.19 to 4.20 January 7, 2026 11:13

shwstppr added a commit to shapeblue/cloudstack-documentation that referenced this pull request Jan 7, 2026

vm: note on skip forced disk controllers uefi secure boot

fd4689a

Related apache/cloudstack#11750 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr mentioned this pull request Jan 7, 2026

vm: note on skip forced disk controllers uefi secure boot apache/cloudstack-documentation#616

Open

RosiKyu added the status:testing label Jan 8, 2026

RosiKyu approved these changes Jan 9, 2026

View reviewed changes

RosiKyu removed status:testing status:needs-testing labels Jan 9, 2026

DaanHoogland merged commit ef1aaa0 into apache:4.20 Jan 9, 2026
26 checks passed

DaanHoogland deleted the fix-kvm-securebootvm-diskcontroller branch January 9, 2026 12:56

kvm: allow skip forcing disk controller #11750

kvm: allow skip forcing disk controller #11750

Conversation

shwstppr commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

shwstppr commented Sep 29, 2025

Uh oh!

blueorangutan commented Sep 29, 2025

Uh oh!

blueorangutan commented Sep 29, 2025

Uh oh!

weizhouapache commented Sep 29, 2025

Uh oh!

blueorangutan commented Sep 29, 2025

Uh oh!

blueorangutan commented Sep 29, 2025

Uh oh!

vishesh92 commented Sep 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

vishesh92 commented Oct 1, 2025

Uh oh!

blueorangutan commented Oct 1, 2025

Uh oh!

blueorangutan commented Oct 1, 2025

Uh oh!

weizhouapache commented Oct 3, 2025

Uh oh!

blueorangutan commented Oct 3, 2025

Uh oh!

blueorangutan commented Oct 3, 2025

Uh oh!

pavanaravapalli Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

RosiKyu commented Dec 17, 2025

Uh oh!

blueorangutan commented Dec 17, 2025

Uh oh!

blueorangutan commented Dec 17, 2025

Uh oh!

shwstppr commented Jan 7, 2026

Uh oh!

blueorangutan commented Jan 7, 2026

Uh oh!

blueorangutan commented Jan 7, 2026

Uh oh!

RosiKyu commented Jan 8, 2026

Uh oh!

blueorangutan commented Jan 8, 2026

Uh oh!

blueorangutan commented Jan 9, 2026

Uh oh!

RosiKyu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Test Results

API Tests

shwstppr commented Sep 29, 2025 •

edited

Loading

codecov bot commented Oct 1, 2025 •

edited

Loading

RosiKyu left a comment •

edited

Loading