-
Notifications
You must be signed in to change notification settings - Fork 1.2k
kvm: fix disk controller for secure boot vm #10213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.19
Are you sure you want to change the base?
kvm: fix disk controller for secure boot vm #10213
Conversation
Fixes apache#9460 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This would fix the issue but not sure if could cause the issue of disks not being identified when virtio drivers are not installed. Maybe we can add some documentation. ping @pavanaravapalli, if you've any advise @blueorangutan package |
@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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10213 +/- ##
=========================================
Coverage 15.16% 15.16%
- Complexity 11332 11339 +7
=========================================
Files 5412 5412
Lines 475033 475038 +5
Branches 57963 57964 +1
=========================================
+ Hits 72048 72057 +9
+ Misses 394930 394924 -6
- Partials 8055 8057 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12127 |
} else { | ||
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2); | ||
} | ||
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for DATADISK, should it use diskBusTypeData
instead ?
} else { | ||
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2); | ||
} | ||
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shwstppr
When the uefi functionality implemented there is a issue with (Windows Guest VM + Secure Boot ) Disk controller for disk using virtio, hence it's been set for windows guest vm -> sata, Linux guest vm -> virtio as disk controller.
these code changes will break the uefi functionality for windows guest vm with secure boot.
right now I don't have setup to test the functionality, it will be good If you can test the scenario and confirm that this won't break windows guest vm along with secure boot use-case .
And this behavior always over-rides the os configuration setting, no matter what controller configured in the OS details. Which needs to be addressed if the above use-case is not broken. Other wise we need to find a mid-way to fix both.
A VM or template detail can be added with key `win.skip.force.disk.controller` and value `true` to allow skipping forcing DATA disk controller for the VM. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@pavanaravapalli @weizhouapache cc @rajujith @vladimirpetrov I've made the behaviour configurable using VM (or template) detail. User/operator can add a VM/template detail - win.skip.force.disk.controller = true to make KVM plugin not force SATA controller for Windows VM with secured boot @blueorangutan package |
@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. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 13161 |
@blueorangutan package |
@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. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13164 |
Description
Fixes #9460
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?