kvm: don't always force scsi controller for aarch64 VMs#5802
Conversation
This would allow use of virtio disk controller with Ceph, etc or as defined in the VM's root disk controller setting, rather than always enforce SCSI. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
| return null; | ||
| } | ||
|
|
||
| if (_guestCpuArch != null && _guestCpuArch.equals("aarch64")) { |
There was a problem hiding this comment.
Instead of removing, is it better to move this block to the default else case, because the default case always returns IDE ? I'm thinking in terms of backward compatibility
There was a problem hiding this comment.
This my own hardcoded check for experimental arm64 guest CPU/platform, this is only hit for users using arm64 hosts such as Ampere or Raspberry Pi4 - so there's no backward compatibility to take into consideration.
There was a problem hiding this comment.
... on second thought, I've moved the code to fallback as default for aarch64 when other controller's can't be found/guessed.
|
Following unit test failed @rhtyd Can you update the unit test. Thanks. |
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
|
Thanks @sureshanaparti done (I've tested this in my ceph/rpi lab, works) |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
| verifyDevices(devicesDef, to); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
@rhtyd
code lgtm
is it necessary to remove this unit test ?
There was a problem hiding this comment.
Yes, as it was checking that for aarch64 scsi would always return; now it's not the case.
There was a problem hiding this comment.
good. just tested it can be fixed by adding to.setPlatformEmulator("");
There was a problem hiding this comment.
@rohityadavcloud
I just had a look at the test again. I think we should not remove the test.
we could apply the following change
- libvirtComputingResourceSpy._guestCpuArch = "aarch64";
+ to.setPlatformEmulator("Other PV Virtio-SCSI");
unit test will pass
There was a problem hiding this comment.
Agree with @weizhouapache on keeping the test with the needed adjustments.
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1999 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@blueorangutan test |
|
@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
|
@blueorangutan package |
|
@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2037 |
|
@blueorangutan test |
|
@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2754)
|
|
@rohityadavcloud is further testing needed for this? |
|
@DaanHoogland No, I've manually test the changes with RPi4 and smoketests with x86/kvm has passed. |
7 participants
This would allow use of virtio disk controller with Ceph, etc or as
defined in the VM's root disk controller setting, rather than always
enforce SCSI.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
On ARM64 setup, this would now allow use of virtio disk controller for nfs and ceph. With Ceph, when virtio is used rbd performance increases drastically.