shwstppr commented on a change in pull request #5387:
URL: https://github.com/apache/cloudstack/pull/5387#discussion_r699013817
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##########
@@ -293,7 +292,8 @@ public Long getDomainId() {
}
}
if (ApiConstants.BootType.UEFI.equals(getBootType())) {
Review comment:
Not sure abt this as in UI we allow empty boot mode and if we are
skipping here, we will have to handle UI better by not allowing empty boot mode
when UEFI is selected as boot type. Otherwise, user will expect VM to boot with
UEFI just by changing the boot type.
@rhtyd @nvazquez @davidjumani any comments on the behaviour?
I feel maybe we should just handle the NPE in 4.15 and refactor boot options
handling in UI & API in a separate PR for 4.16
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##########
@@ -293,7 +292,8 @@ public Long getDomainId() {
}
}
if (ApiConstants.BootType.UEFI.equals(getBootType())) {
Review comment:
@sureshanaparti ^^
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##########
@@ -293,7 +292,8 @@ public Long getDomainId() {
}
}
if (ApiConstants.BootType.UEFI.equals(getBootType())) {
Review comment:
@sureshanaparti as discussed offline, I've refactored changes to throw
exception when bootmode is empty for boottype=UEFI
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##########
@@ -293,7 +292,8 @@ public Long getDomainId() {
}
}
if (ApiConstants.BootType.UEFI.equals(getBootType())) {
Review comment:
@sureshanaparti as discussed offline, I've refactored changes to throw
exception when bootmode is empty for boottype=UEFI
```
(local) 🙉 > deploy virtualmachine
zoneid=681d6964-e25b-4d5d-866b-7dab67ee292c
templateid=cc517531-076b-11ec-9cda-645d8651f45a
serviceofferingid=c0cdd2af-2126-4f5f-8b21-b7d4353f6b12 boottype=UEFI bootmode=
🙈 Error: (HTTP 431, error code 4350) bootmode must be specified for the VM
with boot type: UEFI. Valid values are: [LEGACY, SECURE]
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
