You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2022/11/30 07:28:54 UTC

[cloudstack] branch 4.17 updated: server: fix volume migration on user vm scale (#6704)

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.17
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.17 by this push:
     new 47946db8883 server: fix volume migration on user vm scale (#6704)
47946db8883 is described below

commit 47946db8883fde6257bfcde5a2988f1e2d8aa3f8
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Wed Nov 30 12:58:48 2022 +0530

    server: fix volume migration on user vm scale (#6704)
    
    Fixes #6701
    
    When volume migration is initiated by system, account check is not needed.
    
    Introduces a new global setting - allow.diskoffering.change.during.scale.vm. This determines whether to allow or disallow disk offering change for root volume during scaling of a stopped or running VM.
    
    Signed-off-by: Abhishek Kumar <ab...@gmail.com>
    Signed-off-by: Rohit Yadav <ro...@shapeblue.com>
    Co-authored-by: Harikrishna Patnala <ha...@gmail.com>
    Co-authored-by: Rohit Yadav <ro...@gmail.com>
    Co-authored-by: Daniel Augusto Veronezi Salvador <38...@users.noreply.github.com>
    Co-authored-by: Abhishek Kumar <ab...@gmail.com>
---
 .../java/com/cloud/server/ManagementService.java   |   2 +-
 .../com/cloud/server/ManagementServerImpl.java     |  21 +-
 .../com/cloud/storage/VolumeApiServiceImpl.java    |  20 +-
 .../src/main/java/com/cloud/vm/UserVmManager.java  |   3 +
 .../main/java/com/cloud/vm/UserVmManagerImpl.java  |  15 +-
 .../cloudstack/vm/UnmanagedVMsManagerImpl.java     |   2 +-
 test/integration/smoke/test_scale_vm.py            | 521 +++++++++++++++++++--
 7 files changed, 517 insertions(+), 67 deletions(-)

diff --git a/api/src/main/java/com/cloud/server/ManagementService.java b/api/src/main/java/com/cloud/server/ManagementService.java
index 5c385cc18d9..6aea206c796 100644
--- a/api/src/main/java/com/cloud/server/ManagementService.java
+++ b/api/src/main/java/com/cloud/server/ManagementService.java
@@ -405,7 +405,7 @@ public interface ManagementService {
      */
     Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForMigrationOfVolume(Long volumeId);
 
-    Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForMigrationOfVolumeInternal(Long volumeId, Long newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean keepSourceStoragePool, boolean bypassStorageTypeCheck);
+    Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForSystemMigrationOfVolume(Long volumeId, Long newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean keepSourceStoragePool, boolean bypassStorageTypeCheck);
 
     String[] listEventTypes();
 
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 074fff86be5..540b2e39159 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -1526,7 +1526,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Override
     public Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForMigrationOfVolume(final Long volumeId) {
 
-        Pair<List<? extends StoragePool>, List<? extends StoragePool>> allPoolsAndSuitablePoolsPair = listStoragePoolsForMigrationOfVolumeInternal(volumeId, null, null, null, null, false, true);
+        Pair<List<? extends StoragePool>, List<? extends StoragePool>> allPoolsAndSuitablePoolsPair = listStoragePoolsForMigrationOfVolumeInternal(volumeId, null, null, null, null, false, true, false);
         List<? extends StoragePool> allPools = allPoolsAndSuitablePoolsPair.first();
         List<? extends StoragePool> suitablePools = allPoolsAndSuitablePoolsPair.second();
         List<StoragePool> avoidPools = new ArrayList<>();
@@ -1542,13 +1542,20 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         return new Pair<List<? extends StoragePool>, List<? extends StoragePool>>(allPools, suitablePools);
     }
 
-    public Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForMigrationOfVolumeInternal(final Long volumeId, Long newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean keepSourceStoragePool, boolean bypassStorageTypeCheck) {
-        final Account caller = getCaller();
-        if (!_accountMgr.isRootAdmin(caller.getId())) {
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Caller is not a root admin, permission denied to migrate the volume");
+    @Override
+    public Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForSystemMigrationOfVolume(final Long volumeId, Long newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean keepSourceStoragePool, boolean bypassStorageTypeCheck) {
+        return listStoragePoolsForMigrationOfVolumeInternal(volumeId, newDiskOfferingId, newSize, newMinIops, newMaxIops, keepSourceStoragePool, bypassStorageTypeCheck, true);
+    }
+
+    public Pair<List<? extends StoragePool>, List<? extends StoragePool>> listStoragePoolsForMigrationOfVolumeInternal(final Long volumeId, Long newDiskOfferingId, Long newSize, Long newMinIops, Long newMaxIops, boolean keepSourceStoragePool, boolean bypassStorageTypeCheck, boolean bypassAccountCheck) {
+        if (!bypassAccountCheck) {
+            final Account caller = getCaller();
+            if (!_accountMgr.isRootAdmin(caller.getId())) {
+                if (s_logger.isDebugEnabled()) {
+                    s_logger.debug("Caller is not a root admin, permission denied to migrate the volume");
+                }
+                throw new PermissionDeniedException("No permission to migrate volume, only root admin can migrate a volume");
             }
-            throw new PermissionDeniedException("No permission to migrate volume, only root admin can migrate a volume");
         }
 
         final VolumeVO volume = _volumeDao.findById(volumeId);
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index e6726f6977b..bbebc7a2f3b 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -96,6 +96,7 @@ import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToSt
 import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
@@ -1767,14 +1768,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             return volume;
         }
 
-        if (currentSize != newSize || newMaxIops != volume.getMaxIops() || newMinIops != volume.getMinIops()) {
+        if (currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops())) {
             volumeResizeRequired = true;
             validateVolumeReadyStateAndHypervisorChecks(volume, currentSize, newSize);
         }
 
         StoragePoolVO existingStoragePool = _storagePoolDao.findById(volume.getPoolId());
 
-        Pair<List<? extends StoragePool>, List<? extends StoragePool>> poolsPair = managementService.listStoragePoolsForMigrationOfVolumeInternal(volume.getId(), newDiskOffering.getId(), newSize, newMinIops, newMaxIops, true, false);
+        Pair<List<? extends StoragePool>, List<? extends StoragePool>> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(volume.getId(), newDiskOffering.getId(), newSize, newMinIops, newMaxIops, true, false);
         List<? extends StoragePool> suitableStoragePools = poolsPair.second();
 
         if (!suitableStoragePools.stream().anyMatch(p -> (p.getId() == existingStoragePool.getId()))) {
@@ -1823,6 +1824,21 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         return volume;
     }
 
+    /**
+     * This method is to compare long values, in miniops and maxiops a or b can be null or 0.
+     * Use this method to treat 0 and null as same
+     *
+     * @param a
+     * @param b
+     * @return true if a and b are equal excluding 0 and null values.
+     */
+    private boolean compareEqualsIncludingNullOrZero(Long a, Long b) {
+        a = ObjectUtils.defaultIfNull(a, 0L);
+        b = ObjectUtils.defaultIfNull(b, 0L);
+
+        return a.equals(b);
+    }
+
     /**
      * Returns true if the new disk offering is the same than current offering, and the respective Service offering is a custom (constraint or unconstraint) offering.
      */
diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java
index cde2d04970d..4449dc54860 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManager.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManager.java
@@ -47,9 +47,12 @@ import com.cloud.utils.Pair;
  */
 public interface UserVmManager extends UserVmService {
     String EnableDynamicallyScaleVmCK = "enable.dynamic.scale.vm";
+    String AllowDiskOfferingChangeDuringScaleVmCK = "allow.diskoffering.change.during.scale.vm";
     String AllowUserExpungeRecoverVmCK ="allow.user.expunge.recover.vm";
     ConfigKey<Boolean> EnableDynamicallyScaleVm = new ConfigKey<Boolean>("Advanced", Boolean.class, EnableDynamicallyScaleVmCK, "false",
         "Enables/Disables dynamically scaling a vm", true, ConfigKey.Scope.Zone);
+    ConfigKey<Boolean> AllowDiskOfferingChangeDuringScaleVm = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowDiskOfferingChangeDuringScaleVmCK, "false",
+            "Determines whether to allow or disallow disk offering change for root volume during scaling of a stopped or running vm", true, ConfigKey.Scope.Zone);
     ConfigKey<Boolean> AllowUserExpungeRecoverVm = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowUserExpungeRecoverVmCK, "false",
         "Determines whether users can expunge or recover their vm", true, ConfigKey.Scope.Account);
     ConfigKey<Boolean> DisplayVMOVFProperties = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.display.ovf.properties", "false",
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 748ee4c6e74..3e17cb75809 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -1241,7 +1241,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
         // resize and migrate the root volume if required
         DiskOfferingVO newDiskOffering = _diskOfferingDao.findById(newServiceOffering.getDiskOfferingId());
-        changeDiskOfferingForRootVolume(vmId, newDiskOffering, customParameters);
+        changeDiskOfferingForRootVolume(vmId, newDiskOffering, customParameters, vmInstance.getDataCenterId());
 
         _itMgr.upgradeVmDb(vmId, newServiceOffering, currentServiceOffering);
 
@@ -2000,7 +2000,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
                     // #3 resize or migrate the root volume if required
                     DiskOfferingVO newDiskOffering = _diskOfferingDao.findById(newServiceOffering.getDiskOfferingId());
-                    changeDiskOfferingForRootVolume(vmId, newDiskOffering, customParameters);
+                    changeDiskOfferingForRootVolume(vmId, newDiskOffering, customParameters, vmInstance.getDataCenterId());
 
                     // #4 scale the vm now
                     vmInstance = _vmInstanceDao.findById(vmId);
@@ -2036,7 +2036,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         }
     }
 
-    private void changeDiskOfferingForRootVolume(Long vmId, DiskOfferingVO newDiskOffering, Map<String, String> customParameters) throws ResourceAllocationException {
+    private void changeDiskOfferingForRootVolume(Long vmId, DiskOfferingVO newDiskOffering, Map<String, String> customParameters, Long zoneId) throws ResourceAllocationException {
+
+        if (!AllowDiskOfferingChangeDuringScaleVm.valueIn(zoneId)) {
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug(String.format("Changing the disk offering of the root volume during the compute offering change operation is disabled. Please check the setting [%s].", AllowDiskOfferingChangeDuringScaleVm.key()));
+            }
+            return;
+        }
 
         List<VolumeVO> vols = _volsDao.findReadyAndAllocatedRootVolumesByInstance(vmId);
 
@@ -7791,7 +7798,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax,
+        return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowDiskOfferingChangeDuringScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax,
                 VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties,
                 KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList};
     }
diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
index 0a4599e1560..38f57d1f0bb 100644
--- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
+++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
@@ -794,7 +794,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
                 continue;
             }
             LOGGER.debug(String.format("Volume %s needs to be migrated", volumeVO.getUuid()));
-            Pair<List<? extends StoragePool>, List<? extends StoragePool>> poolsPair = managementService.listStoragePoolsForMigrationOfVolumeInternal(profile.getVolumeId(), null, null, null, null, false, true);
+            Pair<List<? extends StoragePool>, List<? extends StoragePool>> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(profile.getVolumeId(), null, null, null, null, false, true);
             if (CollectionUtils.isEmpty(poolsPair.first()) && CollectionUtils.isEmpty(poolsPair.second())) {
                 cleanupFailedImportVM(vm);
                 throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM import failed for unmanaged vm: %s during volume ID: %s migration as no suitable pool(s) found", userVm.getInstanceName(), volumeVO.getUuid()));
diff --git a/test/integration/smoke/test_scale_vm.py b/test/integration/smoke/test_scale_vm.py
index c9b1bf837ba..7f8b65b8465 100644
--- a/test/integration/smoke/test_scale_vm.py
+++ b/test/integration/smoke/test_scale_vm.py
@@ -25,10 +25,13 @@ from marvin.lib.base import (Account,
                              Host,
                              VirtualMachine,
                              ServiceOffering,
+                             DiskOffering,
                              Template,
-                             Configurations)
+                             Configurations,
+                             Volume)
 from marvin.lib.common import (get_zone,
                                get_template,
+                               get_test_template,
                                get_domain)
 from nose.plugins.attrib import attr
 from marvin.sshClient import SshClient
@@ -54,7 +57,7 @@ class TestScaleVm(cloudstackTestCase):
             return
 
         # Get Zone, Domain and templates
-        domain = get_domain(cls.apiclient)
+        cls.domain = get_domain(cls.apiclient)
         cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
         cls.services['mode'] = cls.zone.networktype
 
@@ -73,14 +76,19 @@ class TestScaleVm(cloudstackTestCase):
                 isdynamicallyscalable='true'
             )
         else:
-            cls.template = Template.register(
-                       cls.apiclient,
-                       cls.services["CentOS7template"],
-                       zoneid=cls.zone.id
-                    )
-            cls._cleanup.append(cls.template)
-            cls.template.download(cls.apiclient)
-            time.sleep(60)
+            cls.template = get_test_template(
+                cls.apiclient,
+                cls.zone.id,
+                cls.hypervisor
+            )
+            if cls.template == FAILED:
+                assert False, "get_test_template() failed to return template\
+                        with hypervisor %s" % cls.hypervisor
+            cls.template = Template.update(
+                cls.template,
+                cls.apiclient,
+                isdynamicallyscalable='true'
+            )
 
         # Set Zones and disk offerings
         cls.services["small"]["zoneid"] = cls.zone.id
@@ -90,7 +98,7 @@ class TestScaleVm(cloudstackTestCase):
         cls.account = Account.create(
             cls.apiclient,
             cls.services["account"],
-            domainid=domain.id
+            domainid=cls.domain.id
         )
         cls._cleanup.append(cls.account)
 
@@ -124,37 +132,6 @@ class TestScaleVm(cloudstackTestCase):
             dynamicscalingenabled=False
         )
 
-        # create a virtual machine
-        cls.virtual_machine = VirtualMachine.create(
-            cls.apiclient,
-            cls.services["small"],
-            accountid=cls.account.name,
-            domainid=cls.account.domainid,
-            serviceofferingid=cls.small_offering.id,
-            mode=cls.services["mode"]
-        )
-
-        # create a virtual machine which cannot be dynamically scalable
-        cls.virtual_machine_with_service_offering_dynamic_scaling_disabled = VirtualMachine.create(
-            cls.apiclient,
-            cls.services["small"],
-            accountid=cls.account.name,
-            domainid=cls.account.domainid,
-            serviceofferingid=cls.small_offering_dynamic_scaling_disabled.id,
-            mode=cls.services["mode"]
-        )
-
-        # create a virtual machine which cannot be dynamically scalable
-        cls.virtual_machine_not_dynamically_scalable = VirtualMachine.create(
-            cls.apiclient,
-            cls.services["small"],
-            accountid=cls.account.name,
-            domainid=cls.account.domainid,
-            serviceofferingid=cls.small_offering.id,
-            mode=cls.services["mode"],
-            dynamicscalingenabled=False
-        )
-
         cls._cleanup = [
             cls.small_offering,
             cls.big_offering,
@@ -170,6 +147,11 @@ class TestScaleVm(cloudstackTestCase):
             name="enable.dynamic.scale.vm",
             value="false"
         )
+        Configurations.update(
+            cls.apiclient,
+            name="allow.diskOffering.change.during.scale.vm",
+            value="false"
+        )
         super(TestScaleVm,cls).tearDownClass()
         return
 
@@ -177,6 +159,7 @@ class TestScaleVm(cloudstackTestCase):
         self.apiclient = self.testClient.getApiClient()
         self.dbclient = self.testClient.getDbConnection()
         self.cleanup = []
+        self.services["disk_offering"]["disksize"] = 2
 
         if self.unsupportedHypervisor:
             self.skipTest("Skipping test because unsupported hypervisor\
@@ -199,6 +182,9 @@ class TestScaleVm(cloudstackTestCase):
 
         return ssh_client
 
+    def is_host_xcpng8(self, hostname):
+        return type(hostname) == list and len(hostname) > 0 and 'XCP-ng 8' in hostname[0]
+
     @attr(hypervisor="xenserver")
     @attr(tags=["advanced", "basic"], required_hardware="false")
     def test_01_scale_vm(self):
@@ -216,6 +202,17 @@ class TestScaleVm(cloudstackTestCase):
         #        scaling is not
         #        guaranteed until tools are installed, vm rebooted
 
+        # create a virtual machine
+        self.virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.small_offering.id,
+            mode=self.services["mode"]
+        )
+        self.cleanup.append(self.virtual_machine)
+
         # If hypervisor is Vmware, then check if
         # the vmware tools are installed and the process is running
         # Vmware tools are necessary for scale VM operation
@@ -227,6 +224,7 @@ class TestScaleVm(cloudstackTestCase):
             if not "running" in result:
                 self.skipTest("Skipping scale VM operation because\
                     VMware tools are not installed on the VM")
+        res = None
         if self.hypervisor.lower() != 'simulator':
             hostid = self.virtual_machine.hostid
             host = Host.list(
@@ -251,14 +249,27 @@ class TestScaleVm(cloudstackTestCase):
             self.apiclient,
             isdynamicallyscalable='true')
 
+        if self.is_host_xcpng8(res):
+            self.debug("Only scaling for CPU for XCP-ng 8")
+            offering_data = self.services["service_offerings"]["big"]
+            offering_data["cpunumber"] = 2
+            offering_data["memory"] = self.virtual_machine.memory
+            self.bigger_offering = ServiceOffering.create(
+                self.apiclient,
+                offering_data
+            )
+            self.cleanup.append(self.bigger_offering)
+        else:
+            self.bigger_offering = self.big_offering
+
         self.debug("Scaling VM-ID: %s to service offering: %s and state %s" % (
             self.virtual_machine.id,
-            self.big_offering.id,
+            self.bigger_offering.id,
             self.virtual_machine.state
         ))
 
         cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
-        cmd.serviceofferingid = self.big_offering.id
+        cmd.serviceofferingid = self.bigger_offering.id
         cmd.id = self.virtual_machine.id
 
         try:
@@ -297,11 +308,11 @@ class TestScaleVm(cloudstackTestCase):
                     offering %s and the response says %s" %
             (self.virtual_machine.id,
              self.virtual_machine.serviceofferingid,
-             self.big_offering.id,
+             self.bigger_offering.id,
              vm_response.serviceofferingid))
         self.assertEqual(
             vm_response.serviceofferingid,
-            self.big_offering.id,
+            self.bigger_offering.id,
             "Check service offering of the VM"
         )
 
@@ -313,7 +324,7 @@ class TestScaleVm(cloudstackTestCase):
         return
 
     @attr(tags=["advanced", "basic"], required_hardware="false")
-    def test_02_scale_vm(self):
+    def test_02_scale_vm_negative_offering_disable_scaling(self):
         """Test scale virtual machine which is created from a service offering for which dynamicscalingenabled is false. Scaling operation should fail.
         """
 
@@ -325,6 +336,17 @@ class TestScaleVm(cloudstackTestCase):
         #        scaling is not
         #        guaranteed until tools are installed, vm rebooted
 
+        # create a virtual machine which cannot be dynamically scalable
+        self.virtual_machine_with_service_offering_dynamic_scaling_disabled = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.small_offering_dynamic_scaling_disabled.id,
+            mode=self.services["mode"]
+        )
+        self.cleanup.append(self.virtual_machine_with_service_offering_dynamic_scaling_disabled)
+
         # If hypervisor is Vmware, then check if
         # the vmware tools are installed and the process is running
         # Vmware tools are necessary for scale VM operation
@@ -368,7 +390,7 @@ class TestScaleVm(cloudstackTestCase):
         self.debug("Scaling VM-ID: %s to service offering: %s for which dynamic scaling is disabled and VM state %s" % (
             self.virtual_machine_with_service_offering_dynamic_scaling_disabled.id,
             self.big_offering_dynamic_scaling_disabled.id,
-            self.virtual_machine.state
+            self.virtual_machine_with_service_offering_dynamic_scaling_disabled.state
         ))
 
         cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
@@ -388,7 +410,7 @@ class TestScaleVm(cloudstackTestCase):
         self.debug("Scaling VM-ID: %s to service offering: %s for which dynamic scaling is enabled and VM state %s" % (
             self.virtual_machine_with_service_offering_dynamic_scaling_disabled.id,
             self.big_offering.id,
-            self.virtual_machine.state
+            self.virtual_machine_with_service_offering_dynamic_scaling_disabled.state
         ))
 
         cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
@@ -408,7 +430,7 @@ class TestScaleVm(cloudstackTestCase):
         return
 
     @attr(tags=["advanced", "basic"], required_hardware="false")
-    def test_03_scale_vm(self):
+    def test_03_scale_vm_negative_vm_disable_scaling(self):
         """Test scale virtual machine which is not dynamically scalable to a service offering. Scaling operation should fail.
         """
         # Validate the following
@@ -422,6 +444,18 @@ class TestScaleVm(cloudstackTestCase):
         #        scaling is not
         #        guaranteed until tools are installed, vm rebooted
 
+        # create a virtual machine which cannot be dynamically scalable
+        self.virtual_machine_not_dynamically_scalable = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.small_offering.id,
+            mode=self.services["mode"],
+            dynamicscalingenabled=False
+        )
+        self.cleanup.append(self.virtual_machine_not_dynamically_scalable)
+
         # If hypervisor is Vmware, then check if
         # the vmware tools are installed and the process is running
         # Vmware tools are necessary for scale VM operation
@@ -463,7 +497,7 @@ class TestScaleVm(cloudstackTestCase):
         self.debug("Scaling VM-ID: %s to service offering: %s for which dynamic scaling is enabled and VM state %s" % (
             self.virtual_machine_not_dynamically_scalable.id,
             self.big_offering.id,
-            self.virtual_machine.state
+            self.virtual_machine_not_dynamically_scalable.state
         ))
 
         cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
@@ -480,4 +514,387 @@ class TestScaleVm(cloudstackTestCase):
         else:
             self.fail("Expected an exception to be thrown, failing")
 
-        return
\ No newline at end of file
+        return
+
+    @attr(hypervisor="xenserver")
+    @attr(tags=["advanced", "basic"], required_hardware="false")
+    def test_04_scale_vm_with_user_account(self):
+        """Test scale virtual machine under useraccount
+        """
+        # Validate the following
+        # Create a user Account and create a VM in it.
+        # Scale up the vm and see if it scales to the new svc offering
+
+        # Create an user account
+        self.userAccount = Account.create(
+            self.apiclient,
+            self.services["account"],
+            admin=False,
+            domainid=self.domain.id
+        )
+
+        self.cleanup.append(self.userAccount)
+        # Create user api client of the user account
+        self.userapiclient = self.testClient.getUserApiClient(
+            UserName=self.userAccount.name,
+            DomainName=self.userAccount.domain
+        )
+
+        # create a virtual machine
+        self.virtual_machine_in_user_account = VirtualMachine.create(
+            self.userapiclient,
+            self.services["small"],
+            accountid=self.userAccount.name,
+            domainid=self.userAccount.domainid,
+            serviceofferingid=self.small_offering.id,
+            mode=self.services["mode"]
+        )
+
+        if self.hypervisor.lower() == "vmware":
+            sshClient = self.virtual_machine_in_user_account.get_ssh_client()
+            result = str(
+                sshClient.execute("service vmware-tools status")).lower()
+            self.debug("and result is: %s" % result)
+            if not "running" in result:
+                self.skipTest("Skipping scale VM operation because\
+                    VMware tools are not installed on the VM")
+        res = None
+        if self.hypervisor.lower() != 'simulator':
+            list_vm_response = VirtualMachine.list(
+                self.apiclient,
+                id=self.virtual_machine_in_user_account.id
+            )[0]
+            hostid = list_vm_response.hostid
+            host = Host.list(
+                       self.apiclient,
+                       zoneid=self.zone.id,
+                       hostid=hostid,
+                       type='Routing'
+                   )[0]
+
+            try:
+                username = self.hostConfig["username"]
+                password = self.hostConfig["password"]
+                ssh_client = self.get_ssh_client(host.ipaddress, username, password)
+                res = ssh_client.execute("hostnamectl | grep 'Operating System' | cut -d':' -f2")
+            except Exception as e:
+                pass
+
+            if 'XenServer' in res[0]:
+                self.skipTest("Skipping test for XenServer as it's License does not allow scaling")
+
+        self.virtual_machine_in_user_account.update(
+            self.userapiclient,
+            isdynamicallyscalable='true')
+
+        if self.is_host_xcpng8(res):
+            self.debug("Only scaling for CPU for XCP-ng 8")
+            offering_data = self.services["service_offerings"]["big"]
+            offering_data["cpunumber"] = 2
+            offering_data["memory"] = self.virtual_machine_in_user_account.memory
+            self.bigger_offering = ServiceOffering.create(
+                self.apiclient,
+                offering_data
+            )
+            self.cleanup.append(self.bigger_offering)
+        else:
+            self.bigger_offering = self.big_offering
+
+        self.debug("Scaling VM-ID: %s to service offering: %s and state %s" % (
+            self.virtual_machine_in_user_account.id,
+            self.bigger_offering.id,
+            self.virtual_machine_in_user_account.state
+        ))
+
+        cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
+        cmd.serviceofferingid = self.bigger_offering.id
+        cmd.id = self.virtual_machine_in_user_account.id
+
+        try:
+            self.userapiclient.scaleVirtualMachine(cmd)
+        except Exception as e:
+            if "LicenceRestriction" in str(e):
+                self.skipTest("Your XenServer License does not allow scaling")
+            else:
+                self.fail("Scaling failed with the following exception: " + str(e))
+
+        list_vm_response = VirtualMachine.list(
+            self.userapiclient,
+            id=self.virtual_machine_in_user_account.id
+        )
+
+        vm_response = list_vm_response[0]
+
+        self.debug(
+            "Scaling VM-ID: %s from service offering: %s to new service\
+                    offering %s and the response says %s" %
+            (self.virtual_machine_in_user_account.id,
+             self.virtual_machine_in_user_account.serviceofferingid,
+             self.bigger_offering.id,
+             vm_response.serviceofferingid))
+        self.assertEqual(
+            vm_response.serviceofferingid,
+            self.bigger_offering.id,
+            "Check service offering of the VM"
+        )
+
+        self.assertEqual(
+            vm_response.state,
+            'Running',
+            "Check the state of VM"
+        )
+        return
+
+    @attr(hypervisor="xenserver")
+    @attr(tags=["advanced", "basic"], required_hardware="false")
+    def test_05_scale_vm_dont_allow_disk_offering_change(self):
+        """Test scale virtual machine with disk offering changes
+        """
+        # Validate the following two cases
+
+        # 1
+        # create serviceoffering1 with diskoffering1
+        # create serviceoffering2 with diskoffering2
+        # create a VM with serviceoffering1
+        # Scale up the vm to serviceoffering2
+        # Check disk offering of root volume to be diskoffering1 since setting allow.diskOffering.change.during.scale.vm is false
+
+        # 2
+        # create serviceoffering3 with diskoffering3
+        # update setting allow.diskOffering.change.during.scale.vm to true
+        # scale up the VM to serviceoffering3
+        # Check disk offering of root volume to be diskoffering3 since setting allow.diskOffering.change.during.scale.vm is true
+
+        self.disk_offering1 = DiskOffering.create(
+                                    self.apiclient,
+                                    self.services["disk_offering"],
+                                    )
+        self._cleanup.append(self.disk_offering1)
+        offering_data = {
+            'displaytext': 'ServiceOffering1WithDiskOffering1',
+            'cpuspeed': 500,
+            'cpunumber': 1,
+            'name': 'ServiceOffering1WithDiskOffering1',
+            'memory': 512,
+            'diskofferingid': self.disk_offering1.id
+        }
+
+        self.ServiceOffering1WithDiskOffering1 = ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        self._cleanup.append(self.ServiceOffering1WithDiskOffering1)
+
+        # create a virtual machine
+        self.virtual_machine_test = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.ServiceOffering1WithDiskOffering1.id,
+            mode=self.services["mode"]
+        )
+
+        if self.hypervisor.lower() == "vmware":
+            sshClient = self.virtual_machine_test.get_ssh_client()
+            result = str(
+                sshClient.execute("service vmware-tools status")).lower()
+            self.debug("and result is: %s" % result)
+            if not "running" in result:
+                self.skipTest("Skipping scale VM operation because\
+                    VMware tools are not installed on the VM")
+        res = None
+        if self.hypervisor.lower() != 'simulator':
+            hostid = self.virtual_machine_test.hostid
+            host = Host.list(
+                       self.apiclient,
+                       zoneid=self.zone.id,
+                       hostid=hostid,
+                       type='Routing'
+                   )[0]
+
+            try:
+                username = self.hostConfig["username"]
+                password = self.hostConfig["password"]
+                ssh_client = self.get_ssh_client(host.ipaddress, username, password)
+                res = ssh_client.execute("hostnamectl | grep 'Operating System' | cut -d':' -f2")
+            except Exception as e:
+                pass
+
+            if 'XenServer' in res[0]:
+                self.skipTest("Skipping test for XenServer as it's License does not allow scaling")
+
+        self.virtual_machine_test.update(
+            self.apiclient,
+            isdynamicallyscalable='true')
+
+        self.disk_offering2 = DiskOffering.create(
+                                    self.apiclient,
+                                    self.services["disk_offering"],
+                                    )
+        self._cleanup.append(self.disk_offering2)
+        offering_data = {
+            'displaytext': 'ServiceOffering2WithDiskOffering2',
+            'cpuspeed': 1000,
+            'cpunumber': 2,
+            'name': 'ServiceOffering2WithDiskOffering2',
+            'memory': 1024,
+            'diskofferingid': self.disk_offering2.id
+        }
+        if self.is_host_xcpng8(res):
+            self.debug("Only scaling for CPU for XCP-ng 8")
+            offering_data["memory"] = self.virtual_machine_test.memory
+
+        self.ServiceOffering2WithDiskOffering2 = ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        self._cleanup.append(self.ServiceOffering2WithDiskOffering2)
+
+        self.debug("Scaling VM-ID: %s to service offering: %s and state %s" % (
+            self.virtual_machine_test.id,
+            self.ServiceOffering2WithDiskOffering2.id,
+            self.virtual_machine_test.state
+        ))
+
+        cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
+        cmd.serviceofferingid = self.ServiceOffering2WithDiskOffering2.id
+        cmd.id = self.virtual_machine_test.id
+
+        try:
+            self.apiclient.scaleVirtualMachine(cmd)
+        except Exception as e:
+            if "LicenceRestriction" in str(e):
+                self.skipTest("Your XenServer License does not allow scaling")
+            else:
+                self.fail("Scaling failed with the following exception: " + str(e))
+
+        list_vm_response = VirtualMachine.list(
+            self.apiclient,
+            id=self.virtual_machine_test.id
+        )
+
+        vm_response = list_vm_response[0]
+
+        self.debug(
+            "Scaling VM-ID: %s from service offering: %s to new service\
+                    offering %s and the response says %s" %
+            (self.virtual_machine_test.id,
+             self.virtual_machine_test.serviceofferingid,
+             self.ServiceOffering2WithDiskOffering2.id,
+             vm_response.serviceofferingid))
+
+        vm_response = list_vm_response[0]
+
+        self.assertEqual(
+            vm_response.serviceofferingid,
+            self.ServiceOffering2WithDiskOffering2.id,
+            "Check service offering of the VM"
+        )
+
+        volume_response = Volume.list(
+            self.apiclient,
+            virtualmachineid=self.virtual_machine_test.id,
+            listall=True
+        )[0]
+
+        self.assertEqual(
+            volume_response.diskofferingid,
+            self.disk_offering1.id,
+            "Check disk offering of the VM, this should not change since allow.diskOffering.change.during.scale.vm is false"
+        )
+
+        # Do same scale vm operation with allow.diskOffering.change.during.scale.vm value to true
+        if self.hypervisor.lower() in ['simulator']:
+            self.debug("Simulator doesn't support changing disk offering, volume resize")
+            return
+        disk_offering_data = self.services["disk_offering"]
+        if self.hypervisor.lower() in ['xenserver']:
+            self.debug("For hypervisor %s, do not resize volume and just change try to change the disk offering")
+            volume_response = Volume.list(
+                self.apiclient,
+                virtualmachineid=self.virtual_machine_test.id,
+                listall=True
+            )[0]
+            disk_offering_data["disksize"] = int(volume_response.size / (1024 ** 3))
+        self.disk_offering3 = DiskOffering.create(
+                                    self.apiclient,
+                                    disk_offering_data,
+                                    )
+        self._cleanup.append(self.disk_offering3)
+        offering_data = {
+            'displaytext': 'ServiceOffering3WithDiskOffering3',
+            'cpuspeed': 1500,
+            'cpunumber': 2,
+            'name': 'ServiceOffering3WithDiskOffering3',
+            'memory': 1024,
+            'diskofferingid': self.disk_offering3.id
+        }
+        if self.is_host_xcpng8(res):
+            self.debug("Only scaling for CPU for XCP-ng 8")
+            offering_data["memory"] = vm_response.memory
+
+        self.ServiceOffering3WithDiskOffering3 = ServiceOffering.create(
+            self.apiclient,
+            offering_data,
+        )
+        self._cleanup.append(self.ServiceOffering3WithDiskOffering3)
+
+        Configurations.update(
+            self.apiclient,
+            name="allow.diskOffering.change.during.scale.vm",
+            value="true"
+        )
+
+        self.debug("Scaling VM-ID: %s to service offering: %s and state %s" % (
+            self.virtual_machine_test.id,
+            self.ServiceOffering3WithDiskOffering3.id,
+            self.virtual_machine_test.state
+        ))
+
+        cmd = scaleVirtualMachine.scaleVirtualMachineCmd()
+        cmd.serviceofferingid = self.ServiceOffering3WithDiskOffering3.id
+        cmd.id = self.virtual_machine_test.id
+
+        try:
+            self.apiclient.scaleVirtualMachine(cmd)
+        except Exception as e:
+            if "LicenceRestriction" in str(e):
+                self.skipTest("Your XenServer License does not allow scaling")
+            else:
+                self.fail("Scaling failed with the following exception: " + str(e))
+
+        list_vm_response = VirtualMachine.list(
+            self.apiclient,
+            id=self.virtual_machine_test.id
+        )
+
+        vm_response = list_vm_response[0]
+
+        self.debug(
+            "Scaling VM-ID: %s from service offering: %s to new service\
+                    offering %s and the response says %s" %
+            (self.virtual_machine_test.id,
+             self.virtual_machine_test.serviceofferingid,
+             self.ServiceOffering3WithDiskOffering3.id,
+             vm_response.serviceofferingid))
+
+        self.assertEqual(
+            vm_response.serviceofferingid,
+            self.ServiceOffering3WithDiskOffering3.id,
+            "Check service offering of the VM"
+        )
+
+        volume_response = Volume.list(
+            self.apiclient,
+            virtualmachineid=self.virtual_machine_test.id,
+            listall=True
+        )[0]
+
+        self.assertEqual(
+            volume_response.diskofferingid,
+            self.disk_offering3.id,
+            "Check disk offering of the VM, this should change since allow.diskOffering.change.during.scale.vm is true"
+        )
+
+        return