rhtyd commented on a change in pull request #5008:
URL: https://github.com/apache/cloudstack/pull/5008#discussion_r697353145



##########
File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ChangeOfferingForVolumeCmd.java
##########
@@ -0,0 +1,141 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.offering.DiskOffering;
+import com.cloud.storage.Volume;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.UserCmd;
+import org.apache.cloudstack.api.response.DiskOfferingResponse;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+@APICommand(name = ChangeOfferingForVolumeCmd.APINAME,
+        description = "Change disk offering of the volume and also an option 
to auto migrate if required to apply the new disk offering",
+        responseObject = VolumeResponse.class,
+        requestHasSensitiveInfo = false,
+        responseHasSensitiveInfo = false,
+        authorized = { RoleType.Admin, RoleType.ResourceAdmin, 
RoleType.DomainAdmin, RoleType.User},
+        since = "4.16")
+public class ChangeOfferingForVolumeCmd extends BaseAsyncCmd implements 
UserCmd {
+
+    public static final String APINAME = "changeOfferingForVolume";
+    public static final Logger s_logger = 
Logger.getLogger(ChangeOfferingForVolumeCmd.class.getName());
+    private static final String s_name = "changeofferingforvolumeresponse";

Review comment:
       Nit - we want to look at more modern API implementation which were 
APINAME + response string (see Roles related API for example).

##########
File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ChangeOfferingForVolumeCmd.java
##########
@@ -0,0 +1,141 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.offering.DiskOffering;
+import com.cloud.storage.Volume;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.UserCmd;
+import org.apache.cloudstack.api.response.DiskOfferingResponse;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+@APICommand(name = ChangeOfferingForVolumeCmd.APINAME,
+        description = "Change disk offering of the volume and also an option 
to auto migrate if required to apply the new disk offering",
+        responseObject = VolumeResponse.class,
+        requestHasSensitiveInfo = false,
+        responseHasSensitiveInfo = false,
+        authorized = { RoleType.Admin, RoleType.ResourceAdmin, 
RoleType.DomainAdmin, RoleType.User},
+        since = "4.16")
+public class ChangeOfferingForVolumeCmd extends BaseAsyncCmd implements 
UserCmd {
+
+    public static final String APINAME = "changeOfferingForVolume";
+    public static final Logger s_logger = 
Logger.getLogger(ChangeOfferingForVolumeCmd.class.getName());
+    private static final String s_name = "changeofferingforvolumeresponse";
+
+    /////////////////////////////////////////////////////
+    //////////////// API parameters /////////////////////
+    /////////////////////////////////////////////////////
+
+    @Parameter(name = ApiConstants.ID, entityType = VolumeResponse.class, 
required = true, type = CommandType.UUID, description = "the ID of the volume")
+    private Long id;
+
+    @Parameter(name = ApiConstants.DISK_OFFERING_ID,
+            entityType = DiskOfferingResponse.class,
+            type = CommandType.UUID,
+            required = true,
+            description = "new disk offering id")
+    private Long newDiskOfferingId;
+
+    @Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, required = 
false, description = "New volume size in GB for the custom disk offering")
+    private Long size;
+
+    @Parameter(name = ApiConstants.MIN_IOPS, type = CommandType.LONG, required 
= false, description = "New minimum number of IOPS for the custom disk 
offering")
+    private Long minIops;
+
+    @Parameter(name = ApiConstants.MAX_IOPS, type = CommandType.LONG, required 
= false, description = "New maximum number of IOPS for the custom disk 
offering")
+    private Long maxIops;
+
+    @Parameter(name = ApiConstants.AUTO_MIGRATE, type = CommandType.BOOLEAN, 
required = false, description = "Flag for automatic migration of the volume " +
+            "with new disk offering whenever migration is required to apply 
the offering")
+    private Boolean autoMigrate;
+
+    /////////////////////////////////////////////////////
+    /////////////////// Accessors ///////////////////////
+    /////////////////////////////////////////////////////
+
+    public Long getId() {
+        return id;
+    }
+
+    public Long getNewDiskOfferingId() {
+        return newDiskOfferingId;
+    }
+
+    public Long getSize() {
+        return size;
+    }
+
+    public Long getMinIops() {
+        return minIops;
+    }
+
+    public Long getMaxIops() {
+        return maxIops;
+    }
+
+    public boolean getAutoMigrate() {
+        return autoMigrate == null ? true : autoMigrate;

Review comment:
       Should the default be true or false (for ex. when option not provided?)

##########
File path: 
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ChangeOfferingForVolumeCmd.java
##########
@@ -0,0 +1,141 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.offering.DiskOffering;
+import com.cloud.storage.Volume;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.UserCmd;
+import org.apache.cloudstack.api.response.DiskOfferingResponse;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+@APICommand(name = ChangeOfferingForVolumeCmd.APINAME,
+        description = "Change disk offering of the volume and also an option 
to auto migrate if required to apply the new disk offering",
+        responseObject = VolumeResponse.class,
+        requestHasSensitiveInfo = false,
+        responseHasSensitiveInfo = false,
+        authorized = { RoleType.Admin, RoleType.ResourceAdmin, 
RoleType.DomainAdmin, RoleType.User},
+        since = "4.16")
+public class ChangeOfferingForVolumeCmd extends BaseAsyncCmd implements 
UserCmd {
+

Review comment:
       nit - new line

##########
File path: test/integration/smoke/test_service_offerings.py
##########
@@ -614,6 +615,204 @@ def test_04_change_offering_small(self):
         )
         return
 
+    @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_05_disk_offering_strictness_true(self):
+        """Test to see change service offering is not possible when disk 
offering strictness is set to true
+        """
+        # Validate the following
+        # 1. Create service offering linked a disk offering and disk offering 
strictness is true
+        # 2. Create a VM with that service offering
+        # 3. Create another service offering with a different disk offering
+        # 4. Try change service offering for VM and it will fail since disk 
offering strictness is true (not allowed to change the disk offering)
+
+        if self.hypervisor.lower() == "lxc":
+            self.skipTest("Skipping this test for {} due to bug 
CS-38153".format(self.hypervisor))
+        offering_data = {
+            'displaytext': 'TestDiskOfferingStrictnessTrue',
+            'cpuspeed': 512,
+            'cpunumber': 2,
+            'name': 'TestDiskOfferingStrictnessTrue',
+            'memory': 1024,
+            'diskofferingstrictness': True
+        }
+
+        self.serviceOfferingWithDiskOfferingStrictnessTrue = 
ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        
self._cleanup.append(self.serviceOfferingWithDiskOfferingStrictnessTrue)
+
+        self.virtual_machine_with_diskoffering_strictness_true = 
VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            
serviceofferingid=self.serviceOfferingWithDiskOfferingStrictnessTrue.id,
+            mode=self.services["mode"]
+        )
+
+        try:
+            
self.virtual_machine_with_diskoffering_strictness_true.stop(self.apiclient)
+
+            timeout = self.services["timeout"]
+
+            while True:
+                time.sleep(self.services["sleep"])
+
+                # Ensure that VM is in stopped state
+                list_vm_response = list_virtual_machines(
+                    self.apiclient,
+                    
id=self.virtual_machine_with_diskoffering_strictness_true.id
+                )
+
+                if isinstance(list_vm_response, list):
+                    vm = list_vm_response[0]
+                    if vm.state == 'Stopped':
+                        self.debug("VM state: %s" % vm.state)
+                        break
+
+                if timeout == 0:
+                    raise Exception(
+                        "Failed to stop VM (ID: %s) in change service 
offering" % vm.id)
+
+                timeout = timeout - 1
+        except Exception as e:
+            self.fail("Failed to stop VM: %s" % e)
+
+        offering_data = {

Review comment:
       self.offering_data?

##########
File path: test/integration/smoke/test_service_offerings.py
##########
@@ -614,6 +615,204 @@ def test_04_change_offering_small(self):
         )
         return
 
+    @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_05_disk_offering_strictness_true(self):
+        """Test to see change service offering is not possible when disk 
offering strictness is set to true
+        """
+        # Validate the following
+        # 1. Create service offering linked a disk offering and disk offering 
strictness is true
+        # 2. Create a VM with that service offering
+        # 3. Create another service offering with a different disk offering
+        # 4. Try change service offering for VM and it will fail since disk 
offering strictness is true (not allowed to change the disk offering)
+
+        if self.hypervisor.lower() == "lxc":
+            self.skipTest("Skipping this test for {} due to bug 
CS-38153".format(self.hypervisor))
+        offering_data = {

Review comment:
       You might want to say - self.offering_data?

##########
File path: test/integration/smoke/test_service_offerings.py
##########
@@ -614,6 +615,204 @@ def test_04_change_offering_small(self):
         )
         return
 
+    @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_05_disk_offering_strictness_true(self):
+        """Test to see change service offering is not possible when disk 
offering strictness is set to true
+        """
+        # Validate the following
+        # 1. Create service offering linked a disk offering and disk offering 
strictness is true
+        # 2. Create a VM with that service offering
+        # 3. Create another service offering with a different disk offering
+        # 4. Try change service offering for VM and it will fail since disk 
offering strictness is true (not allowed to change the disk offering)
+
+        if self.hypervisor.lower() == "lxc":
+            self.skipTest("Skipping this test for {} due to bug 
CS-38153".format(self.hypervisor))
+        offering_data = {
+            'displaytext': 'TestDiskOfferingStrictnessTrue',
+            'cpuspeed': 512,
+            'cpunumber': 2,
+            'name': 'TestDiskOfferingStrictnessTrue',
+            'memory': 1024,
+            'diskofferingstrictness': True
+        }
+
+        self.serviceOfferingWithDiskOfferingStrictnessTrue = 
ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        
self._cleanup.append(self.serviceOfferingWithDiskOfferingStrictnessTrue)
+
+        self.virtual_machine_with_diskoffering_strictness_true = 
VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            
serviceofferingid=self.serviceOfferingWithDiskOfferingStrictnessTrue.id,
+            mode=self.services["mode"]
+        )
+
+        try:
+            
self.virtual_machine_with_diskoffering_strictness_true.stop(self.apiclient)
+
+            timeout = self.services["timeout"]
+
+            while True:
+                time.sleep(self.services["sleep"])
+
+                # Ensure that VM is in stopped state
+                list_vm_response = list_virtual_machines(
+                    self.apiclient,
+                    
id=self.virtual_machine_with_diskoffering_strictness_true.id
+                )
+
+                if isinstance(list_vm_response, list):
+                    vm = list_vm_response[0]
+                    if vm.state == 'Stopped':
+                        self.debug("VM state: %s" % vm.state)
+                        break
+
+                if timeout == 0:
+                    raise Exception(
+                        "Failed to stop VM (ID: %s) in change service 
offering" % vm.id)
+
+                timeout = timeout - 1
+        except Exception as e:
+            self.fail("Failed to stop VM: %s" % e)
+
+        offering_data = {
+            'displaytext': 'TestDiskOfferingStrictnessTrue2',
+            'cpuspeed': 1000,
+            'cpunumber': 2,
+            'name': 'TestDiskOfferingStrictnessTrue2',
+            'memory': 1024,
+            'diskofferingstrictness': True
+        }
+
+        self.serviceOfferingWithDiskOfferingStrictnessTrue2 = 
ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        
self._cleanup.append(self.serviceOfferingWithDiskOfferingStrictnessTrue2)
+        cmd = 
changeServiceForVirtualMachine.changeServiceForVirtualMachineCmd()
+        cmd.id = self.virtual_machine_with_diskoffering_strictness_true.id
+        cmd.serviceofferingid = 
self.serviceOfferingWithDiskOfferingStrictnessTrue2.id
+
+        with self.assertRaises(Exception) as e:
+            self.apiclient.changeServiceForVirtualMachine(cmd)
+            self.debug("Upgrade VM with new service offering having different 
disk offering operation failed as expected with exception: %s" %
+                       e.exception)
+        return
+
+    @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_06_disk_offering_strictness_false(self):
+        """Test to see change service offering is possible when disk offering 
strictness is set to false
+        """
+        # Validate the following
+        # 1. Create service offering linked a disk offering and disk offering 
strictness is false
+        # 2. Create a VM with that service offering
+        # 3. Create another service offering with a different disk offering 
and disk offering strictness is false
+        # 4. Try change service offering for VM should succeed
+
+        if self.hypervisor.lower() == "lxc":
+            self.skipTest("Skipping this test for {} due to bug 
CS-38153".format(self.hypervisor))
+
+        offering_data = {
+            'displaytext': 'TestDiskOfferingStrictnessFalse',
+            'cpuspeed': 512,
+            'cpunumber': 2,
+            'name': 'TestDiskOfferingStrictnessFalse',
+            'memory': 1024,
+            'diskofferingstrictness': False
+        }
+
+        self.serviceOfferingWithDiskOfferingStrictnessFalse = 
ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        
self._cleanup.append(self.serviceOfferingWithDiskOfferingStrictnessFalse)
+
+        self.virtual_machine_with_diskoffering_strictness_false = 
VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            
serviceofferingid=self.serviceOfferingWithDiskOfferingStrictnessFalse.id,
+            mode=self.services["mode"]
+        )
+
+        try:
+            
self.virtual_machine_with_diskoffering_strictness_false.stop(self.apiclient)
+
+            timeout = self.services["timeout"]
+
+            while True:
+                time.sleep(self.services["sleep"])
+
+                # Ensure that VM is in stopped state
+                list_vm_response = list_virtual_machines(
+                    self.apiclient,
+                    
id=self.virtual_machine_with_diskoffering_strictness_false.id
+                )
+
+                if isinstance(list_vm_response, list):
+                    vm = list_vm_response[0]
+                    if vm.state == 'Stopped':
+                        self.debug("VM state: %s" % vm.state)
+                        break
+
+                if timeout == 0:
+                    raise Exception(
+                        "Failed to stop VM (ID: %s) in change service 
offering" % vm.id)
+
+                timeout = timeout - 1
+        except Exception as e:
+            self.fail("Failed to stop VM: %s" % e)
+
+        self.disk_offering2 = DiskOffering.create(
+                                    self.apiclient,
+                                    self.services["disk_offering"],
+                                    )
+        self._cleanup.append(self.disk_offering2)
+        offering_data = {

Review comment:
       self.offering_data (actually ignore this and above if this is highly 
test-case specific data)

##########
File path: test/integration/smoke/test_service_offerings.py
##########
@@ -614,6 +615,204 @@ def test_04_change_offering_small(self):
         )
         return
 
+    @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_05_disk_offering_strictness_true(self):
+        """Test to see change service offering is not possible when disk 
offering strictness is set to true
+        """
+        # Validate the following
+        # 1. Create service offering linked a disk offering and disk offering 
strictness is true
+        # 2. Create a VM with that service offering
+        # 3. Create another service offering with a different disk offering
+        # 4. Try change service offering for VM and it will fail since disk 
offering strictness is true (not allowed to change the disk offering)
+
+        if self.hypervisor.lower() == "lxc":
+            self.skipTest("Skipping this test for {} due to bug 
CS-38153".format(self.hypervisor))
+        offering_data = {
+            'displaytext': 'TestDiskOfferingStrictnessTrue',
+            'cpuspeed': 512,
+            'cpunumber': 2,
+            'name': 'TestDiskOfferingStrictnessTrue',
+            'memory': 1024,
+            'diskofferingstrictness': True
+        }
+
+        self.serviceOfferingWithDiskOfferingStrictnessTrue = 
ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        
self._cleanup.append(self.serviceOfferingWithDiskOfferingStrictnessTrue)
+
+        self.virtual_machine_with_diskoffering_strictness_true = 
VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            
serviceofferingid=self.serviceOfferingWithDiskOfferingStrictnessTrue.id,
+            mode=self.services["mode"]
+        )
+
+        try:
+            
self.virtual_machine_with_diskoffering_strictness_true.stop(self.apiclient)
+
+            timeout = self.services["timeout"]
+
+            while True:
+                time.sleep(self.services["sleep"])
+
+                # Ensure that VM is in stopped state
+                list_vm_response = list_virtual_machines(
+                    self.apiclient,
+                    
id=self.virtual_machine_with_diskoffering_strictness_true.id
+                )
+
+                if isinstance(list_vm_response, list):
+                    vm = list_vm_response[0]
+                    if vm.state == 'Stopped':
+                        self.debug("VM state: %s" % vm.state)
+                        break
+
+                if timeout == 0:
+                    raise Exception(
+                        "Failed to stop VM (ID: %s) in change service 
offering" % vm.id)
+
+                timeout = timeout - 1
+        except Exception as e:
+            self.fail("Failed to stop VM: %s" % e)
+
+        offering_data = {
+            'displaytext': 'TestDiskOfferingStrictnessTrue2',
+            'cpuspeed': 1000,
+            'cpunumber': 2,
+            'name': 'TestDiskOfferingStrictnessTrue2',
+            'memory': 1024,
+            'diskofferingstrictness': True
+        }
+
+        self.serviceOfferingWithDiskOfferingStrictnessTrue2 = 
ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        
self._cleanup.append(self.serviceOfferingWithDiskOfferingStrictnessTrue2)
+        cmd = 
changeServiceForVirtualMachine.changeServiceForVirtualMachineCmd()
+        cmd.id = self.virtual_machine_with_diskoffering_strictness_true.id
+        cmd.serviceofferingid = 
self.serviceOfferingWithDiskOfferingStrictnessTrue2.id
+
+        with self.assertRaises(Exception) as e:
+            self.apiclient.changeServiceForVirtualMachine(cmd)
+            self.debug("Upgrade VM with new service offering having different 
disk offering operation failed as expected with exception: %s" %
+                       e.exception)
+        return
+
+    @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true")
+    def test_06_disk_offering_strictness_false(self):
+        """Test to see change service offering is possible when disk offering 
strictness is set to false
+        """
+        # Validate the following
+        # 1. Create service offering linked a disk offering and disk offering 
strictness is false
+        # 2. Create a VM with that service offering
+        # 3. Create another service offering with a different disk offering 
and disk offering strictness is false
+        # 4. Try change service offering for VM should succeed
+
+        if self.hypervisor.lower() == "lxc":
+            self.skipTest("Skipping this test for {} due to bug 
CS-38153".format(self.hypervisor))
+
+        offering_data = {

Review comment:
       self.offering_data?

##########
File path: 
engine/api/src/main/java/org/apache/cloudstack/engine/service/api/OrchestrationService.java
##########
@@ -67,14 +67,15 @@ VirtualMachineEntity createVirtualMachine(@QueryParam("id") 
String id, @QueryPar
         @QueryParam("compute-tags") List<String> computeTags, 
@QueryParam("root-disk-tags") List<String> rootDiskTags,
         @QueryParam("network-nic-map") Map<String, List<NicProfile>> 
networkNicMap, @QueryParam("deploymentplan") DeploymentPlan plan,
         @QueryParam("root-disk-size") Long rootDiskSize, 
@QueryParam("extra-dhcp-option-map") Map<String, Map<Integer, String>> 
extraDhcpOptionMap,
-        @QueryParam("datadisktemplate-diskoffering-map") Map<Long, 
DiskOffering> datadiskTemplateToDiskOfferingMap) throws 
InsufficientCapacityException;
+        @QueryParam("datadisktemplate-diskoffering-map") Map<Long, 
DiskOffering> datadiskTemplateToDiskOfferingMap, 
@QueryParam("disk-offering-id") Long diskOfferingId, 
@QueryParam("root-disk-offering-id") Long rootDiskOfferingId) throws 
InsufficientCapacityException;

Review comment:
       I don't think codebase in engine/api is used (the restful api is not 
maintained/does not run)

##########
File path: server/src/main/java/com/cloud/server/ConfigurationServerImpl.java
##########
@@ -926,16 +926,21 @@ private DiskOfferingVO createDefaultDiskOffering(String 
name, String description
         newDiskOffering.setUniqueName("Cloud.Com-" + name);
         // leaving the above reference to cloud.com in as it is an identifyer 
and has no real world relevance
         newDiskOffering.setSystemUse(isSystemUse);
-        newDiskOffering = 
_diskOfferingDao.persistDeafultDiskOffering(newDiskOffering);
+        newDiskOffering = 
_diskOfferingDao.persistDefaultDiskOffering(newDiskOffering);
         return newDiskOffering;
     }
 
     private ServiceOfferingVO createServiceOffering(long userId, String name, 
int cpu, int ramSize, int speed, String displayText,
             ProvisioningType provisioningType, boolean localStorageRequired, 
boolean offerHA, String tags) {
         tags = cleanupTags(tags);
+        DiskOfferingVO diskOfferingVO = new DiskOfferingVO(name, displayText, 
provisioningType, false, tags, false, false, false, true);
+        diskOfferingVO.setUniqueName("Cloud.Com-" + name);

Review comment:
       Is this just for default offerings?

##########
File path: 
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1655,19 +1660,6 @@ protected boolean hostCanAccessSPool(Host host, 
StoragePool pool) {
                 }
             } else {
                 useLocalStorage = diskOffering.isUseLocalStorage();
-
-                // TODO: this is a hacking fix for the problem of deploy
-                // ISO-based VM on local storage
-                // when deploying VM based on ISO, we have a service offering
-                // and an additional disk offering, use-local storage flag is
-                // actually
-                // saved in service offering, override the flag from service
-                // offering when it is a ROOT disk
-                if (!useLocalStorage && 
vmProfile.getServiceOffering().isUseLocalStorage()) {

Review comment:
       Wont' this case of ISO-based VM cause issue now?




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


Reply via email to