You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/01/10 04:32:50 UTC

[GitHub] [cloudstack] jairov4 opened a new pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

jairov4 opened a new pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576


   ### Description
   Fix #4245 
   
   This PR uses Q35 chipset for UEFI in x86_64.
   Currently this mistakenly only enabled for secure boot
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x ] 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)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ x] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [x] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   Build and run a UEFI VM using OVMF firmware.
   Both of Secure Boot and Legacy modes are working now
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-767399817


   @blueorangutan test 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-759852758


   @jairov4 can you rebase to 4.15 branch? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-766685051






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r561824111



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       Well, I did get some problem reports back on my quick search, but the general tone is that it should work indeed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r564346096



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       I personally run a toy PI4 env for fun, so I would prefer a suitable if-else be added based on platform, arch parameters (I'm not sure if the hard-coded chipset works on RPi4) 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r560788888



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       i wanted to see that `AMD` thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] wido commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
wido commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r559996950



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       I know it's super fun to run on a Pi and people use it to dev with it.
   
   But I think 99,999999% of all CloudStack installations runs on x86_64 CPUs from either Intel or AMD.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r559638176



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       should we make assumptions about running on intel platforms and/or would that hurt? cc @wido @rhtyd #raspberryrules




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-767796310


   <b>Trillian test result (tid-3415)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38478 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4576-t3415-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 82 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3611.80 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 102.81 | test_kubernetes_clusters.py
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-766684773


   @blueorangutan package


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-766685051


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-766684773


   @blueorangutan package


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher edited a comment on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-760935788


   Considering that the bug was reported on 4.14.0.0, maybe it would be a nice bug to be fixed in branch 4.14, and then forwarded to 4.15 and master.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] jairov4 commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
jairov4 commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-760975028


   Hi, I rebased it to 4.14


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-766723928


   Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2584


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r561824111



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       Well, I did get some problem reports back on my quick search, but the general tone is that it should work indeed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd merged pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] wido commented on a change in pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
wido commented on a change in pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#discussion_r560790445



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2247,8 +2247,8 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
         if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
             guest.setBootType(GuestDef.BootType.UEFI);
             guest.setBootMode(GuestDef.BootMode.LEGACY);
+            guest.setMachineType("q35");

Review comment:
       > i wanted to see that `AMD` thanks
   
   Keep in mind though that both AMD and Intel produce CPUs with the x86_64 Architecture.
   
   As this is all virtualization I think this also works on AMD Epyc CPUs.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-767400187


   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4576: Fix: Use Q35 chipset for UEFI x86_64

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4576:
URL: https://github.com/apache/cloudstack/pull/4576#issuecomment-760935788


   Considering that the bug was reported on 4.14.0.0, maybe this would be nice to have it rebased to branch 4.14, and fast forwarded to 4.15 and then master.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org