[GitHub] [cloudstack] nvazquez commented on a change in pull request #4643: VM dynamic scaling option granularity

[GitBox <gi...@apache.org>]

nvazquez commented on a change in pull request #4643:
URL: https://github.com/apache/cloudstack/pull/4643#discussion_r571438409



##########
File path: api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##########
@@ -623,6 +627,10 @@ public Boolean getBootIntoSetup() {
         return bootIntoSetup;
     }
 
+    public Boolean isDynamicScalingEnabled() {
+        return dynamicScalingEnabled == null ? Boolean.TRUE : dynamicScalingEnabled;

Review comment:
       Same as above

##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
##########
@@ -433,6 +437,10 @@ public Long getStoragePolicy() {
         return storagePolicy;
     }
 
+    public Boolean getDynamicScalingEnabled() {
+        return isDynamicScalingEnabled == null ? Boolean.TRUE : isDynamicScalingEnabled;

Review comment:
       As the null check is in place, what about returning boolean instead of Boolean on this function? Also, this will mean that all the service offerings registered before 4.16 will now become dynamically scallable as well?

##########
File path: engine/schema/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/db/VMEntityVO.java
##########
@@ -561,6 +564,11 @@ public boolean isDisplay() {
         return display;
     }
 
+    @Override
+    public Boolean isDynamicallyScalable() {

Review comment:
       This can return a boolean as well

##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -3899,6 +3914,16 @@ private UserVm createVirtualMachine(DataCenter zone, ServiceOffering serviceOffe
         return vm;
     }
 
+    @Override
+    public Boolean checkIfDynamicScalingCanBeEnabled(ServiceOffering offering, VirtualMachineTemplate template, Long zoneId) {

Review comment:
       Can this method also accept a VM as a parameter and check if the VM is scalable as well?




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