[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435103#comment-16435103
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10230:
---------------------------------------------

DaanHoogland closed pull request #2404: [CLOUDSTACK-10230] User should not be 
able to use removed “Guest OS type”
URL: https://github.com/apache/cloudstack/pull/2404
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java 
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index f2f764b3dc7..df81dd3ff2b 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -39,10 +39,6 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import org.apache.commons.codec.binary.Base64;
-import org.apache.commons.lang.StringUtils;
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.affinity.AffinityGroupService;
@@ -80,7 +76,6 @@
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
-import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService;
@@ -89,7 +84,6 @@
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.framework.jobs.AsyncJobManager;
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
 import org.apache.cloudstack.storage.command.DeleteCommand;
 import org.apache.cloudstack.storage.command.DettachCommand;
@@ -97,6 +91,10 @@
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.collections.MapUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
@@ -125,7 +123,6 @@
 import com.cloud.agent.manager.Commands;
 import com.cloud.alert.AlertManager;
 import com.cloud.api.ApiDBUtils;
-import com.cloud.api.query.dao.UserVmJoinDao;
 import com.cloud.capacity.Capacity;
 import com.cloud.capacity.CapacityManager;
 import com.cloud.configuration.Config;
@@ -136,14 +133,14 @@
 import com.cloud.dc.DataCenterVO;
 import com.cloud.dc.DedicatedResourceVO;
 import com.cloud.dc.HostPodVO;
+import com.cloud.dc.Vlan;
+import com.cloud.dc.Vlan.VlanType;
+import com.cloud.dc.VlanVO;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.dc.dao.DedicatedResourceDao;
 import com.cloud.dc.dao.HostPodDao;
 import com.cloud.dc.dao.VlanDao;
-import com.cloud.dc.Vlan;
-import com.cloud.dc.Vlan.VlanType;
-import com.cloud.dc.VlanVO;
 import com.cloud.deploy.DataCenterDeployment;
 import com.cloud.deploy.DeployDestination;
 import com.cloud.deploy.DeploymentPlanner;
@@ -211,9 +208,7 @@
 import com.cloud.network.security.SecurityGroup;
 import com.cloud.network.security.SecurityGroupManager;
 import com.cloud.network.security.dao.SecurityGroupDao;
-import com.cloud.network.security.dao.SecurityGroupVMMapDao;
 import com.cloud.network.vpc.VpcManager;
-import com.cloud.network.vpc.dao.VpcDao;
 import com.cloud.offering.DiskOffering;
 import com.cloud.offering.NetworkOffering;
 import com.cloud.offering.NetworkOffering.Availability;
@@ -222,10 +217,8 @@
 import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.org.Cluster;
 import com.cloud.org.Grouping;
-import com.cloud.projects.ProjectManager;
 import com.cloud.resource.ResourceManager;
 import com.cloud.resource.ResourceState;
-import com.cloud.server.ConfigurationServer;
 import com.cloud.server.ManagementService;
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
@@ -234,15 +227,15 @@
 import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.GuestOSCategoryVO;
 import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.Storage;
-import com.cloud.storage.Snapshot;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.storage.Storage.TemplateType;
-import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
 import com.cloud.storage.StoragePoolStatus;
+import com.cloud.storage.VMTemplateStorageResourceAssoc;
 import com.cloud.storage.VMTemplateVO;
 import com.cloud.storage.VMTemplateZoneVO;
 import com.cloud.storage.Volume;
@@ -253,11 +246,8 @@
 import com.cloud.storage.dao.GuestOSDao;
 import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.VMTemplateDao;
-import com.cloud.storage.dao.VMTemplateDetailsDao;
 import com.cloud.storage.dao.VMTemplateZoneDao;
 import com.cloud.storage.dao.VolumeDao;
-import com.cloud.storage.snapshot.SnapshotManager;
-import com.cloud.tags.dao.ResourceTagDao;
 import com.cloud.template.TemplateApiService;
 import com.cloud.template.TemplateManager;
 import com.cloud.template.VirtualMachineTemplate;
@@ -305,254 +295,224 @@
 import com.cloud.vm.dao.InstanceGroupVMMapDao;
 import com.cloud.vm.dao.NicDao;
 import com.cloud.vm.dao.NicExtraDhcpOptionDao;
-import com.cloud.vm.dao.SecondaryStorageVmDao;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
 import com.cloud.vm.snapshot.VMSnapshotManager;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
-import com.cloud.storage.snapshot.SnapshotApiService;
-import com.cloud.storage.VMTemplateStorageResourceAssoc;
+
 
 public class UserVmManagerImpl extends ManagerBase implements UserVmManager, 
VirtualMachineGuru, UserVmService, Configurable {
     private static final Logger s_logger = 
Logger.getLogger(UserVmManagerImpl.class);
 
-    private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 3; 
// 3 seconds
-    private static final long GB_TO_BYTES = 1024 * 1024 * 1024;
+    /**
+     * The number of seconds to wait before timing out when trying to acquire 
a global lock.
+     */
+    private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 3;
+
+    private static final long GiB_TO_BYTES = 1024 * 1024 * 1024;
 
     @Inject
-    EntityManager _entityMgr;
-    @Inject
-    protected HostDao _hostDao = null;
-    @Inject
-    protected ServiceOfferingDao _offeringDao = null;
-    @Inject
-    protected DiskOfferingDao _diskOfferingDao = null;
-    @Inject
-    protected VMTemplateDao _templateDao = null;
-    @Inject
-    protected VMTemplateDetailsDao _templateDetailsDao = null;
-    @Inject
-    protected VMTemplateZoneDao _templateZoneDao = null;
-    @Inject
-    protected TemplateDataStoreDao _templateStoreDao;
-    @Inject
-    protected DomainDao _domainDao = null;
-    @Inject
-    protected UserVmDao _vmDao = null;
-    @Inject
-    protected UserVmJoinDao _vmJoinDao = null;
-    @Inject
-    protected VolumeDao _volsDao = null;
-    @Inject
-    protected DataCenterDao _dcDao = null;
-    @Inject
-    protected FirewallRulesDao _rulesDao = null;
-    @Inject
-    protected LoadBalancerVMMapDao _loadBalancerVMMapDao = null;
+    private EntityManager _entityMgr;
     @Inject
-    protected PortForwardingRulesDao _portForwardingDao;
+    private HostDao _hostDao;
     @Inject
-    protected IPAddressDao _ipAddressDao = null;
+    private ServiceOfferingDao _offeringDao;
     @Inject
-    protected HostPodDao _podDao = null;
+    private DiskOfferingDao _diskOfferingDao;
     @Inject
-    protected NetworkModel _networkModel = null;
+    private VMTemplateDao _templateDao;
     @Inject
-    protected NetworkOrchestrationService _networkMgr = null;
+    private VMTemplateZoneDao _templateZoneDao;
     @Inject
-    protected StorageManager _storageMgr = null;
+    private TemplateDataStoreDao _templateStoreDao;
     @Inject
-    protected SnapshotManager _snapshotMgr = null;
+    private DomainDao _domainDao;
     @Inject
-    protected AgentManager _agentMgr = null;
+    private UserVmDao _vmDao;
     @Inject
-    protected ConfigurationManager _configMgr = null;
+    private VolumeDao _volsDao;
     @Inject
-    protected AccountDao _accountDao = null;
+    private DataCenterDao _dcDao;
     @Inject
-    protected UserDao _userDao = null;
+    private FirewallRulesDao _rulesDao;
     @Inject
-    protected SnapshotDao _snapshotDao = null;
+    private LoadBalancerVMMapDao _loadBalancerVMMapDao;
     @Inject
-    protected GuestOSDao _guestOSDao = null;
+    private PortForwardingRulesDao _portForwardingDao;
     @Inject
-    protected HighAvailabilityManager _haMgr = null;
+    private IPAddressDao _ipAddressDao;
     @Inject
-    protected AlertManager _alertMgr = null;
+    private HostPodDao _podDao;
     @Inject
-    protected AccountManager _accountMgr;
+    private NetworkModel _networkModel;
     @Inject
-    protected AccountService _accountService;
+    private NetworkOrchestrationService _networkMgr;
     @Inject
-    protected AsyncJobManager _asyncMgr;
+    private AgentManager _agentMgr;
     @Inject
-    protected ClusterDao _clusterDao;
+    private ConfigurationManager _configMgr;
     @Inject
-    protected PrimaryDataStoreDao _storagePoolDao;
+    private AccountDao _accountDao;
     @Inject
-    protected SecurityGroupManager _securityGroupMgr;
+    private UserDao _userDao;
     @Inject
-    protected ServiceOfferingDao _serviceOfferingDao;
+    private SnapshotDao _snapshotDao;
     @Inject
-    protected NetworkOfferingDao _networkOfferingDao;
+    private GuestOSDao _guestOSDao;
     @Inject
-    protected InstanceGroupDao _vmGroupDao;
+    private HighAvailabilityManager _haMgr;
     @Inject
-    protected InstanceGroupVMMapDao _groupVMMapDao;
+    private AlertManager _alertMgr;
     @Inject
-    protected VirtualMachineManager _itMgr;
+    private AccountManager _accountMgr;
     @Inject
-    protected NetworkDao _networkDao;
+    private AccountService _accountService;
     @Inject
-    protected NicDao _nicDao;
+    private ClusterDao _clusterDao;
     @Inject
-    protected ServiceOfferingDao _offerringDao;
+    private PrimaryDataStoreDao _storagePoolDao;
     @Inject
-    protected VpcDao _vpcDao;
+    private SecurityGroupManager _securityGroupMgr;
     @Inject
-    protected RulesManager _rulesMgr;
+    private ServiceOfferingDao _serviceOfferingDao;
     @Inject
-    protected LoadBalancingRulesManager _lbMgr;
+    private NetworkOfferingDao _networkOfferingDao;
     @Inject
-    protected SSHKeyPairDao _sshKeyPairDao;
+    private InstanceGroupDao _vmGroupDao;
     @Inject
-    protected UserVmDetailsDao _vmDetailsDao;
+    private InstanceGroupVMMapDao _groupVMMapDao;
     @Inject
-    protected HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
+    private VirtualMachineManager _itMgr;
     @Inject
-    protected SecurityGroupDao _securityGroupDao;
+    private NetworkDao _networkDao;
     @Inject
-    protected CapacityManager _capacityMgr;
+    private NicDao _nicDao;
     @Inject
-    protected VMInstanceDao _vmInstanceDao;
+    private RulesManager _rulesMgr;
     @Inject
-    protected ResourceLimitService _resourceLimitMgr;
+    private LoadBalancingRulesManager _lbMgr;
     @Inject
-    protected FirewallManager _firewallMgr;
+    private SSHKeyPairDao _sshKeyPairDao;
     @Inject
-    protected ProjectManager _projectMgr;
+    private UserVmDetailsDao _vmDetailsDao;
     @Inject
-    protected ResourceManager _resourceMgr;
+    private HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
     @Inject
-    protected NetworkServiceMapDao _ntwkSrvcDao;
+    private SecurityGroupDao _securityGroupDao;
     @Inject
-    SecurityGroupVMMapDao _securityGroupVMMapDao;
+    private CapacityManager _capacityMgr;
     @Inject
-    protected ItWorkDao _workDao;
+    private VMInstanceDao _vmInstanceDao;
     @Inject
-    ResourceTagDao _resourceTagDao;
+    private ResourceLimitService _resourceLimitMgr;
     @Inject
-    PhysicalNetworkDao _physicalNetworkDao;
+    private FirewallManager _firewallMgr;
     @Inject
-    VpcManager _vpcMgr;
+    private ResourceManager _resourceMgr;
     @Inject
-    TemplateManager _templateMgr;
+    private NetworkServiceMapDao _ntwkSrvcDao;
     @Inject
-    protected GuestOSCategoryDao _guestOSCategoryDao;
+    private PhysicalNetworkDao _physicalNetworkDao;
     @Inject
-    UsageEventDao _usageEventDao;
+    private VpcManager _vpcMgr;
     @Inject
-    SecondaryStorageVmDao _secondaryDao;
+    private TemplateManager _templateMgr;
     @Inject
-    VmDiskStatisticsDao _vmDiskStatsDao;
+    private GuestOSCategoryDao _guestOSCategoryDao;
     @Inject
-    protected VMSnapshotDao _vmSnapshotDao;
+    private UsageEventDao _usageEventDao;
     @Inject
-    protected VMSnapshotManager _vmSnapshotMgr;
+    private VmDiskStatisticsDao _vmDiskStatsDao;
     @Inject
-    AffinityGroupVMMapDao _affinityGroupVMMapDao;
+    private VMSnapshotDao _vmSnapshotDao;
     @Inject
-    AffinityGroupDao _affinityGroupDao;
+    private VMSnapshotManager _vmSnapshotMgr;
     @Inject
-    TemplateDataFactory templateFactory;
+    private AffinityGroupVMMapDao _affinityGroupVMMapDao;
     @Inject
-    DedicatedResourceDao _dedicatedDao;
+    private AffinityGroupDao _affinityGroupDao;
     @Inject
-    ConfigurationServer _configServer;
+    private DedicatedResourceDao _dedicatedDao;
     @Inject
-    AffinityGroupService _affinityGroupService;
+    private AffinityGroupService _affinityGroupService;
     @Inject
-    PlannerHostReservationDao _plannerHostReservationDao;
+    private PlannerHostReservationDao _plannerHostReservationDao;
     @Inject
     private ServiceOfferingDetailsDao serviceOfferingDetailsDao;
     @Inject
     private UserStatisticsDao _userStatsDao;
     @Inject
-    protected VlanDao _vlanDao;
+    private VlanDao _vlanDao;
     @Inject
-    VolumeService _volService;
+    private VolumeService _volService;
     @Inject
-    VolumeDataFactory volFactory;
+    private VolumeDataFactory volFactory;
     @Inject
-    UserVmDetailsDao _uservmDetailsDao;
+    private UserVmDetailsDao _uservmDetailsDao;
     @Inject
-    UUIDManager _uuidMgr;
+    private UUIDManager _uuidMgr;
     @Inject
-    DeploymentPlanningManager _planningMgr;
+    private DeploymentPlanningManager _planningMgr;
     @Inject
-    VolumeApiService _volumeService;
+    private VolumeApiService _volumeService;
     @Inject
-    DataStoreManager _dataStoreMgr;
+    private DataStoreManager _dataStoreMgr;
     @Inject
-    VpcVirtualNetworkApplianceManager _virtualNetAppliance;
+    private VpcVirtualNetworkApplianceManager _virtualNetAppliance;
     @Inject
-    DomainRouterDao _routerDao;
+    private DomainRouterDao _routerDao;
     @Inject
-    protected VMNetworkMapDao _vmNetworkMapDao;
+    private VMNetworkMapDao _vmNetworkMapDao;
     @Inject
-    protected IpAddressManager _ipAddrMgr;
+    private IpAddressManager _ipAddrMgr;
     @Inject
-    private SnapshotApiService _snapshotService;
+    private NicExtraDhcpOptionDao _nicExtraDhcpOptionDao;
     @Inject
-    NicExtraDhcpOptionDao _nicExtraDhcpOptionDao;
+    private TemplateApiService _tmplService;
     @Inject
-    protected TemplateApiService _tmplService;
+    private ConfigurationDao _configDao;
 
-    protected ScheduledExecutorService _executor = null;
-    protected ScheduledExecutorService _vmIpFetchExecutor = null;
-    protected int _expungeInterval;
-    protected int _expungeDelay;
-    protected boolean _dailyOrHourly = false;
+    private ScheduledExecutorService _executor = null;
+    private ScheduledExecutorService _vmIpFetchExecutor = null;
+    private int _expungeInterval;
+    private int _expungeDelay;
+    private boolean _dailyOrHourly = false;
     private int capacityReleaseInterval;
-    ExecutorService _vmIpFetchThreadExecutor;
+    private ExecutorService _vmIpFetchThreadExecutor;
 
 
-    protected String _instance;
-    protected String _zone;
-    protected boolean _instanceNameFlag;
-    protected int _scaleRetry;
-    protected  Map<Long, VmAndCountDetails> vmIdCountMap = new 
ConcurrentHashMap<>();
+    private String _instance;
+    private boolean _instanceNameFlag;
+    private int _scaleRetry;
+    private Map<Long, VmAndCountDetails> vmIdCountMap = new 
ConcurrentHashMap<>();
 
-    @Inject
-    ConfigurationDao _configDao;
-    private static final int MAX_VM_NAME_LEN = 80;
     private static final int MAX_HTTP_GET_LENGTH = 2 * 
MAX_USER_DATA_LENGTH_BYTES;
     private static final int MAX_HTTP_POST_LENGTH = 16 * 
MAX_USER_DATA_LENGTH_BYTES;
 
     @Inject
-    protected OrchestrationService _orchSrvc;
+    private OrchestrationService _orchSrvc;
 
     @Inject
-    VolumeOrchestrationService volumeMgr;
+    private VolumeOrchestrationService volumeMgr;
 
     @Inject
-    ManagementService _mgr;
+    private ManagementService _mgr;
 
-    static final ConfigKey<Integer> VmIpFetchWaitInterval = new 
ConfigKey<Integer>("Advanced", Integer.class, 
"externaldhcp.vmip.retrieval.interval", "180",
+    private static final ConfigKey<Integer> VmIpFetchWaitInterval = new 
ConfigKey<Integer>("Advanced", Integer.class, 
"externaldhcp.vmip.retrieval.interval", "180",
             "Wait Interval (in seconds) for shared network vm dhcp ip addr 
fetch for next iteration ", true);
 
-    static final ConfigKey<Integer> VmIpFetchTrialMax = new 
ConfigKey<Integer>("Advanced", Integer.class, "externaldhcp.vmip.max.retry", 
"10",
+    private static final ConfigKey<Integer> VmIpFetchTrialMax = new 
ConfigKey<Integer>("Advanced", Integer.class, "externaldhcp.vmip.max.retry", 
"10",
             "The max number of retrieval times for shared entwork vm dhcp ip 
fetch, in case of failures", true);
 
-    static final ConfigKey<Integer> VmIpFetchThreadPoolMax = new 
ConfigKey<Integer>("Advanced", Integer.class, 
"externaldhcp.vmipFetch.threadPool.max", "10",
+    private static final ConfigKey<Integer> VmIpFetchThreadPoolMax = new 
ConfigKey<Integer>("Advanced", Integer.class, 
"externaldhcp.vmipFetch.threadPool.max", "10",
             "number of threads for fetching vms ip address", true);
 
-    static final ConfigKey<Integer> VmIpFetchTaskWorkers = new 
ConfigKey<Integer>("Advanced", Integer.class, 
"externaldhcp.vmipfetchtask.workers", "10",
-                        "number of worker threads for vm ip fetch task ", 
true);
+    private static final ConfigKey<Integer> VmIpFetchTaskWorkers = new 
ConfigKey<Integer>("Advanced", Integer.class, 
"externaldhcp.vmipfetchtask.workers", "10",
+            "number of worker threads for vm ip fetch task ", true);
 
-    static final ConfigKey<Boolean> AllowDeployVmIfGivenHostFails = new 
ConfigKey<Boolean>("Advanced", Boolean.class, 
"allow.deploy.vm.if.deploy.on.given.host.fails", "false",
+    private static final ConfigKey<Boolean> AllowDeployVmIfGivenHostFails = 
new ConfigKey<Boolean>("Advanced", Boolean.class, 
"allow.deploy.vm.if.deploy.on.given.host.fails", "false",
             "allow vm to deploy on different host if vm fails to deploy on the 
given host ", true);
 
 
@@ -566,19 +526,19 @@ public UserVmVO getVirtualMachine(long vmId) {
         return _vmDao.listByHostId(hostId);
     }
 
-    protected void resourceLimitCheck(Account owner, Boolean displayVm, Long 
cpu, Long memory) throws ResourceAllocationException {
+    private void resourceLimitCheck(Account owner, Boolean displayVm, Long 
cpu, Long memory) throws ResourceAllocationException {
         _resourceLimitMgr.checkResourceLimit(owner, ResourceType.user_vm, 
displayVm);
         _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, 
displayVm, cpu);
         _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, 
displayVm, memory);
     }
 
-    protected void resourceCountIncrement(long accountId, Boolean displayVm, 
Long cpu, Long memory) {
+    private void resourceCountIncrement(long accountId, Boolean displayVm, 
Long cpu, Long memory) {
         _resourceLimitMgr.incrementResourceCount(accountId, 
ResourceType.user_vm, displayVm);
         _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.cpu, 
displayVm, cpu);
         _resourceLimitMgr.incrementResourceCount(accountId, 
ResourceType.memory, displayVm, memory);
     }
 
-    protected void resourceCountDecrement(long accountId, Boolean displayVm, 
Long cpu, Long memory) {
+    private void resourceCountDecrement(long accountId, Boolean displayVm, 
Long cpu, Long memory) {
         _resourceLimitMgr.decrementResourceCount(accountId, 
ResourceType.user_vm, displayVm);
         _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.cpu, 
displayVm, cpu);
         _resourceLimitMgr.decrementResourceCount(accountId, 
ResourceType.memory, displayVm, memory);
@@ -623,7 +583,7 @@ public void decrementCount() {
         }
     }
 
-    protected class VmIpAddrFetchThread extends ManagedContextRunnable {
+    private class VmIpAddrFetchThread extends ManagedContextRunnable {
 
 
         long nicId;
@@ -633,9 +593,6 @@ public void decrementCount() {
         Long hostId;
         String networkCidr;
 
-        public VmIpAddrFetchThread() {
-        }
-
         public VmIpAddrFetchThread(long vmId, long nicId, String instanceName, 
boolean windows, Long hostId, String networkCidr) {
             this.vmId = vmId;
             this.nicId = nicId;
@@ -827,7 +784,7 @@ public UserVm resetVMSSHKey(ResetVMSSHKeyCmd cmd) throws 
ResourceUnavailableExce
         SSHKeyPairVO s = _sshKeyPairDao.findByName(owner.getAccountId(), 
owner.getDomainId(), cmd.getName());
         if (s == null) {
             throw new InvalidParameterValueException("A key pair with name '" 
+ cmd.getName() + "' does not exist for account " + owner.getAccountName()
-                    + " in specified domain id");
+            + " in specified domain id");
         }
 
         _accountMgr.checkAccess(caller, null, true, userVm);
@@ -989,7 +946,7 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) 
throws ResourceAllocationE
             throw new InvalidParameterValueException("unable to find a virtual 
machine with id " + vmId);
         } else if (!(vmInstance.getState().equals(State.Stopped))) {
             throw new InvalidParameterValueException("Unable to upgrade 
virtual machine " + vmInstance.toString() + " " + " in state " + 
vmInstance.getState()
-                    + "; make sure the virtual machine is stopped");
+            + "; make sure the virtual machine is stopped");
         }
 
         _accountMgr.checkAccess(caller, null, true, vmInstance);
@@ -1057,7 +1014,7 @@ public void validateCustomParameters(ServiceOfferingVO 
serviceOffering, Map<Stri
                 }
             } else if 
(customParameters.containsKey(UsageEventVO.DynamicParameters.cpuNumber.name())) 
{
                 throw new InvalidParameterValueException("The cpu cores of 
this offering id:" + serviceOffering.getId()
-                        + " is not customizable. This is predefined in the 
template.");
+                + " is not customizable. This is predefined in the template.");
             }
 
             if (serviceOffering.getSpeed() == null) {
@@ -1067,7 +1024,7 @@ public void validateCustomParameters(ServiceOfferingVO 
serviceOffering, Map<Stri
                 }
             } else if 
(customParameters.containsKey(UsageEventVO.DynamicParameters.cpuSpeed.name())) {
                 throw new InvalidParameterValueException("The cpu speed of 
this offering id:" + serviceOffering.getId()
-                        + " is not customizable. This is predefined in the 
template.");
+                + " is not customizable. This is predefined in the template.");
             }
 
             if (serviceOffering.getRamSize() == null) {
@@ -1196,8 +1153,9 @@ public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) 
throws InvalidParameterV
 
         List<NicVO> allNics = _nicDao.listByVmId(vmInstance.getId());
         for (NicVO nic : allNics) {
-            if (nic.getNetworkId() == network.getId())
+            if (nic.getNetworkId() == network.getId()) {
                 throw new CloudRuntimeException("A NIC already exists for VM:" 
+ vmInstance.getInstanceName() + " in network: " + network.getUuid());
+            }
         }
 
         if(_nicDao.findByNetworkIdAndMacAddress(networkId, macAddress) != 
null) {
@@ -1265,10 +1223,6 @@ public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) 
throws InvalidParameterV
                 }
             }
         }
-
-        if (guestNic == null) {
-            throw new CloudRuntimeException("Unable to add NIC to " + 
vmInstance);
-        }
         CallContext.current().putContextParameter(Nic.class, 
guestNic.getUuid());
         s_logger.debug("Successful addition of " + network + " from " + 
vmInstance);
         return _vmDao.findById(vmInstance.getId());
@@ -1623,7 +1577,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_VM_UPGRADE, eventDescription = 
"Upgrading VM", async = true)
     public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws 
ResourceUnavailableException, ConcurrentOperationException, 
ManagementServerException,
-            VirtualMachineMigrationException {
+    VirtualMachineMigrationException {
 
         Long vmId = cmd.getId();
         Long newServiceOfferingId = cmd.getServiceOfferingId();
@@ -1683,7 +1637,7 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) 
throws ResourceUnavailableEx
 
     @Override
     public boolean upgradeVirtualMachine(Long vmId, Long newServiceOfferingId, 
Map<String, String> customParameters) throws ResourceUnavailableException,
-            ConcurrentOperationException, ManagementServerException, 
VirtualMachineMigrationException {
+    ConcurrentOperationException, ManagementServerException, 
VirtualMachineMigrationException {
 
         // Verify input parameters
         VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId);
@@ -1697,11 +1651,11 @@ public boolean upgradeVirtualMachine(Long vmId, Long 
newServiceOfferingId, Map<S
                 return upgradeRunningVirtualMachine(vmId, 
newServiceOfferingId, customParameters);
             }
         }
-    return false;
+        return false;
     }
 
     private boolean upgradeRunningVirtualMachine(Long vmId, Long 
newServiceOfferingId, Map<String, String> customParameters) throws 
ResourceUnavailableException,
-            ConcurrentOperationException, ManagementServerException, 
VirtualMachineMigrationException {
+    ConcurrentOperationException, ManagementServerException, 
VirtualMachineMigrationException {
 
         Account caller = CallContext.current().getCallingAccount();
         VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId);
@@ -1770,7 +1724,7 @@ private boolean upgradeRunningVirtualMachine(Long vmId, 
Long newServiceOfferingI
             // Check zone wide flag
             boolean enableDynamicallyScaleVm = 
EnableDynamicallyScaleVm.valueIn(vmInstance.getDataCenterId());
             if (!enableDynamicallyScaleVm) {
-               throw new PermissionDeniedException("Dynamically scaling 
virtual machines is disabled for this zone, please contact your admin");
+                throw new PermissionDeniedException("Dynamically scaling 
virtual machines is disabled for this zone, please contact your admin");
             }
 
             // Check vm flag
@@ -2017,8 +1971,8 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) 
throws ResourceAllocationE
                             }
                         }
                         UsageEventUtils
-                                
.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), 
volume.getDataCenterId(), volume.getId(), volume.getName(), offeringId,
-                                        templateId, volume.getSize(), 
Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume());
+                        .publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, 
volume.getAccountId(), volume.getDataCenterId(), volume.getId(), 
volume.getName(), offeringId,
+                                templateId, volume.getSize(), 
Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume());
                     }
                 }
 
@@ -2135,9 +2089,6 @@ public boolean stop() {
         return true;
     }
 
-    protected UserVmManagerImpl() {
-    }
-
     public String getRandomPrivateTemplateName() {
         return UUID.randomUUID().toString();
     }
@@ -2304,16 +2255,11 @@ private void updateVmStateForFailedVmCreation(Long 
vmId, Long hostId) {
 
 
 
-    protected class VmIpFetchTask extends ManagedContextRunnable {
-
-        public VmIpFetchTask() {
-            GlobalLock scanLock = GlobalLock.getInternLock("vmIpFetch");
-        }
+    private class VmIpFetchTask extends ManagedContextRunnable {
 
         @Override
         protected void runInContext() {
             GlobalLock scanLock = GlobalLock.getInternLock("vmIpFetch");
-
             try {
                 if 
(scanLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION)) {
                     try {
@@ -2362,7 +2308,7 @@ protected void runInContext() {
     }
 
 
-    protected class ExpungeTask extends ManagedContextRunnable {
+    private class ExpungeTask extends ManagedContextRunnable {
         public ExpungeTask() {
         }
 
@@ -2403,6 +2349,8 @@ protected void runInContext() {
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription = 
"updating Vm")
     public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws 
ResourceUnavailableException, InsufficientCapacityException {
+        validateInputsAndPermissionForUpdateVirtualMachineCommand(cmd);
+
         String displayName = cmd.getDisplayName();
         String group = cmd.getGroup();
         Boolean ha = cmd.getHaEnable();
@@ -2413,59 +2361,70 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) 
throws ResourceUnavailableEx
         Boolean isDynamicallyScalable = cmd.isDynamicallyScalable();
         String hostName = cmd.getHostName();
         Map<String,String> details = cmd.getDetails();
-        Account caller = CallContext.current().getCallingAccount();
         List<Long> securityGroupIdList = getSecurityGroupIdList(cmd);
         boolean cleanupDetails = cmd.isCleanupDetails();
 
-        // Input validation and permission checks
-        UserVmVO vmInstance = _vmDao.findById(id);
-        if (vmInstance == null) {
-            throw new InvalidParameterValueException("unable to find virtual 
machine with id " + id);
-        }
-
-        _accountMgr.checkAccess(caller, null, true, vmInstance);
-
-        //If the flag is specified and is changed
-        if (isDisplayVm != null && isDisplayVm != vmInstance.isDisplayVm()) {
-
-            //update vm
-            vmInstance.setDisplayVm(isDisplayVm);
-
-            // Resource limit changes
-            ServiceOffering offering = 
_serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getServiceOfferingId());
-            _resourceLimitMgr.changeResourceCount(vmInstance.getAccountId(), 
ResourceType.user_vm, isDisplayVm);
-            _resourceLimitMgr.changeResourceCount(vmInstance.getAccountId(), 
ResourceType.cpu, isDisplayVm, new Long(offering.getCpu()));
-            _resourceLimitMgr.changeResourceCount(vmInstance.getAccountId(), 
ResourceType.memory, isDisplayVm, new Long(offering.getRamSize()));
-
-            // Usage
-            saveUsageEvent(vmInstance);
-
-            // take care of the root volume as well.
-            List<VolumeVO> rootVols = _volsDao.findByInstanceAndType(id, 
Volume.Type.ROOT);
-            if(!rootVols.isEmpty()){
-                _volumeService.updateDisplay(rootVols.get(0), isDisplayVm);
-            }
-
-            // take care of the data volumes as well.
-            List<VolumeVO> dataVols = _volsDao.findByInstanceAndType(id, 
Volume.Type.DATADISK);
-            for(Volume dataVol : dataVols){
-                _volumeService.updateDisplay(dataVol, isDisplayVm);
-            }
+        UserVmVO vmInstance = _vmDao.findById(cmd.getId());
 
+        if (isDisplayVm != null && isDisplayVm != vmInstance.isDisplay()) {
+            updateDisplayVmFlag(isDisplayVm, id, vmInstance);
         }
-
         if (cleanupDetails){
             _vmDetailsDao.removeDetails(id);
         }
-        else if (details != null && !details.isEmpty()) {
+        else if (MapUtils.isNotEmpty(details)) {
             vmInstance.setDetails(details);
             _vmDao.saveDetails(vmInstance);
         }
-
         return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, 
osTypeId, userData, isDynamicallyScalable,
                 cmd.getHttpMethod(), cmd.getCustomId(), hostName, 
cmd.getInstanceName(), securityGroupIdList, cmd.getDhcpOptionsMap());
     }
 
+    protected void updateDisplayVmFlag(Boolean isDisplayVm, Long id, UserVmVO 
vmInstance) {
+        vmInstance.setDisplayVm(isDisplayVm);
+
+        // Resource limit changes
+        ServiceOffering offering = 
_serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getServiceOfferingId());
+        _resourceLimitMgr.changeResourceCount(vmInstance.getAccountId(), 
ResourceType.user_vm, isDisplayVm);
+        _resourceLimitMgr.changeResourceCount(vmInstance.getAccountId(), 
ResourceType.cpu, isDisplayVm, new Long(offering.getCpu()));
+        _resourceLimitMgr.changeResourceCount(vmInstance.getAccountId(), 
ResourceType.memory, isDisplayVm, new Long(offering.getRamSize()));
+
+        // Usage
+        saveUsageEvent(vmInstance);
+
+        // take care of the root volume as well.
+        List<VolumeVO> rootVols = _volsDao.findByInstanceAndType(id, 
Volume.Type.ROOT);
+        if (!rootVols.isEmpty()) {
+            _volumeService.updateDisplay(rootVols.get(0), isDisplayVm);
+        }
+
+        // take care of the data volumes as well.
+        List<VolumeVO> dataVols = _volsDao.findByInstanceAndType(id, 
Volume.Type.DATADISK);
+        for (Volume dataVol : dataVols) {
+            _volumeService.updateDisplay(dataVol, isDisplayVm);
+        }
+    }
+
+    protected void 
validateInputsAndPermissionForUpdateVirtualMachineCommand(UpdateVMCmd cmd) {
+        UserVmVO vmInstance = _vmDao.findById(cmd.getId());
+        if (vmInstance == null) {
+            throw new InvalidParameterValueException("unable to find virtual 
machine with id: " + cmd.getId());
+        }
+        validateGuestOsIdForUpdateVirtualMachineCommand(cmd);
+        Account caller = CallContext.current().getCallingAccount();
+        _accountMgr.checkAccess(caller, null, true, vmInstance);
+    }
+
+    protected void validateGuestOsIdForUpdateVirtualMachineCommand(UpdateVMCmd 
cmd) {
+        Long osTypeId = cmd.getOsTypeId();
+        if (osTypeId != null) {
+            GuestOSVO guestOS = _guestOSDao.findById(osTypeId);
+            if (guestOS == null) {
+                throw new InvalidParameterValueException("Please specify a 
valid guest OS ID.");
+            }
+        }
+    }
+
     private void saveUsageEvent(UserVmVO vm) {
 
         // If vm not destroyed
@@ -2664,8 +2623,8 @@ private boolean updateUserDataInternal(UserVm vm) throws 
ResourceUnavailableExce
 
         List<? extends Nic> nics = _nicDao.listByVmId(vm.getId());
         if (nics == null || nics.isEmpty()) {
-           s_logger.error("unable to find any nics for vm " + vm.getUuid());
-           return false;
+            s_logger.error("unable to find any nics for vm " + vm.getUuid());
+            return false;
         }
 
         boolean userDataApplied = false;
@@ -2806,7 +2765,7 @@ public InstanceGroupVO createVmGroup(CreateVMGroupCmd 
cmd) {
     }
 
     @DB
-    protected InstanceGroupVO createVmGroup(String groupName, long accountId) {
+    private InstanceGroupVO createVmGroup(String groupName, long accountId) {
         Account account = null;
         try {
             account = _accountDao.acquireInLockTable(accountId); // to ensure
@@ -2886,26 +2845,26 @@ public boolean addInstanceToGroup(final long userVmId, 
String groupName) {
                 Transaction.execute(new TransactionCallbackNoReturn() {
                     @Override
                     public void doInTransactionWithoutResult(TransactionStatus 
status) {
-                // don't let the group be deleted when we are assigning vm to
-                // it.
+                        // don't let the group be deleted when we are 
assigning vm to
+                        // it.
                         InstanceGroupVO ngrpLock = 
_vmGroupDao.lockRow(groupFinal.getId(), false);
-                if (ngrpLock == null) {
+                        if (ngrpLock == null) {
                             s_logger.warn("Failed to acquire lock on vm group 
id=" + groupFinal.getId() + " name=" + groupFinal.getName());
                             throw new CloudRuntimeException("Failed to acquire 
lock on vm group id=" + groupFinal.getId() + " name=" + groupFinal.getName());
-                }
+                        }
 
-                // Currently don't allow to assign a vm to more than one group
-                if (_groupVMMapDao.listByInstanceId(userVmId) != null) {
-                    // Delete all mappings from group_vm_map table
+                        // Currently don't allow to assign a vm to more than 
one group
+                        if (_groupVMMapDao.listByInstanceId(userVmId) != null) 
{
+                            // Delete all mappings from group_vm_map table
                             List<InstanceGroupVMMapVO> groupVmMaps = 
_groupVMMapDao.listByInstanceId(userVmId);
-                    for (InstanceGroupVMMapVO groupMap : groupVmMaps) {
+                            for (InstanceGroupVMMapVO groupMap : groupVmMaps) {
                                 SearchCriteria<InstanceGroupVMMapVO> sc = 
_groupVMMapDao.createSearchCriteria();
                                 sc.addAnd("instanceId", SearchCriteria.Op.EQ, 
groupMap.getInstanceId());
-                        _groupVMMapDao.expunge(sc);
-                    }
-                }
+                                _groupVMMapDao.expunge(sc);
+                            }
+                        }
                         InstanceGroupVMMapVO groupVmMapVO = new 
InstanceGroupVMMapVO(groupFinal.getId(), userVmId);
-                _groupVMMapDao.persist(groupVmMapVO);
+                        _groupVMMapDao.persist(groupVmMapVO);
 
                     }
                 });
@@ -2953,7 +2912,7 @@ public void removeInstanceFromInstanceGroup(long vmId) {
         }
     }
 
-    protected boolean validPassword(String password) {
+    private boolean validPassword(String password) {
         if (password == null || password.length() == 0) {
             return false;
         }
@@ -2971,7 +2930,7 @@ public UserVm 
createBasicSecurityGroupVirtualMachine(DataCenter zone, ServiceOff
             Account owner, String hostName, String displayName, Long 
diskOfferingId, Long diskSize, String group, HypervisorType hypervisor, 
HTTPMethod httpmethod,
             String userData, String sshKeyPair, Map<Long, IpAddresses> 
requestedIps, IpAddresses defaultIps, Boolean displayVm, String keyboard, 
List<Long> affinityGroupIdList,
             Map<String, String> customParametes, String customId, Map<String, 
Map<Integer, String>> dhcpOptionMap, Map<Long, DiskOffering> 
dataDiskTemplateToDiskOfferingMap) throws InsufficientCapacityException, 
ConcurrentOperationException, ResourceUnavailableException,
-            StorageUnavailableException, ResourceAllocationException {
+    StorageUnavailableException, ResourceAllocationException {
 
         Account caller = CallContext.current().getCallingAccount();
         List<NetworkVO> networkList = new ArrayList<NetworkVO>();
@@ -3028,7 +2987,7 @@ public UserVm 
createAdvancedSecurityGroupVirtualMachine(DataCenter zone, Service
             List<Long> securityGroupIdList, Account owner, String hostName, 
String displayName, Long diskOfferingId, Long diskSize, String group, 
HypervisorType hypervisor,
             HTTPMethod httpmethod, String userData, String sshKeyPair, 
Map<Long, IpAddresses> requestedIps, IpAddresses defaultIps, Boolean displayVm, 
String keyboard,
             List<Long> affinityGroupIdList, Map<String, String> 
customParameters, String customId, Map<String, Map<Integer, String>> 
dhcpOptionMap, Map<Long, DiskOffering> dataDiskTemplateToDiskOfferingMap) 
throws InsufficientCapacityException, ConcurrentOperationException,
-            ResourceUnavailableException, StorageUnavailableException, 
ResourceAllocationException {
+    ResourceUnavailableException, StorageUnavailableException, 
ResourceAllocationException {
 
         Account caller = CallContext.current().getCallingAccount();
         List<NetworkVO> networkList = new ArrayList<NetworkVO>();
@@ -3138,7 +3097,7 @@ public UserVm createAdvancedVirtualMachine(DataCenter 
zone, ServiceOffering serv
             String hostName, String displayName, Long diskOfferingId, Long 
diskSize, String group, HypervisorType hypervisor, HTTPMethod httpmethod, 
String userData,
             String sshKeyPair, Map<Long, IpAddresses> requestedIps, 
IpAddresses defaultIps, Boolean displayvm, String keyboard, List<Long> 
affinityGroupIdList,
             Map<String, String> customParametrs, String customId, Map<String, 
Map<Integer, String>> dhcpOptionsMap, Map<Long, DiskOffering> 
dataDiskTemplateToDiskOfferingMap) throws InsufficientCapacityException, 
ConcurrentOperationException, ResourceUnavailableException,
-            StorageUnavailableException, ResourceAllocationException {
+    StorageUnavailableException, ResourceAllocationException {
 
         Account caller = CallContext.current().getCallingAccount();
         List<NetworkVO> networkList = new ArrayList<NetworkVO>();
@@ -3165,7 +3124,7 @@ public UserVm createAdvancedVirtualMachine(DataCenter 
zone, ServiceOffering serv
             List<NetworkOfferingVO> requiredOfferings = 
_networkOfferingDao.listByAvailability(Availability.Required, false);
             if (requiredOfferings.size() < 1) {
                 throw new InvalidParameterValueException("Unable to find 
network offering with availability=" + Availability.Required
-                                + " to automatically create the network as a 
part of vm creation");
+                        + " to automatically create the network as a part of 
vm creation");
             }
 
             if (requiredOfferings.get(0).getState() == 
NetworkOffering.State.Enabled) {
@@ -3184,8 +3143,8 @@ public UserVm createAdvancedVirtualMachine(DataCenter 
zone, ServiceOffering serv
                     }
                     s_logger.debug("Creating network for account " + owner + " 
from the network offering id=" + requiredOfferings.get(0).getId() + " as a part 
of deployVM process");
                     Network newNetwork = 
_networkMgr.createGuestNetwork(requiredOfferings.get(0).getId(), 
owner.getAccountName() + "-network", owner.getAccountName() + "-network",
-                                                                        null, 
null, null, false, null, owner, null, physicalNetwork, zone.getId(), 
ACLType.Account, null, null, null, null, true, null,
-                                                                        null);
+                            null, null, null, false, null, owner, null, 
physicalNetwork, zone.getId(), ACLType.Account, null, null, null, null, true, 
null,
+                            null);
                     if (newNetwork != null) {
                         defaultNetwork = 
_networkDao.findById(newNetwork.getId());
                     }
@@ -3263,11 +3222,11 @@ public void checkNameForRFCCompliance(String name) {
     }
 
     @DB
-    protected UserVm createVirtualMachine(DataCenter zone, ServiceOffering 
serviceOffering, VirtualMachineTemplate tmplt, String hostName, String 
displayName, Account owner,
+    private UserVm createVirtualMachine(DataCenter zone, ServiceOffering 
serviceOffering, VirtualMachineTemplate tmplt, String hostName, String 
displayName, Account owner,
             Long diskOfferingId, Long diskSize, List<NetworkVO> networkList, 
List<Long> securityGroupIdList, String group, HTTPMethod httpmethod, String 
userData,
             String sshKeyPair, HypervisorType hypervisor, Account caller, 
Map<Long, IpAddresses> requestedIps, IpAddresses defaultIps, Boolean 
isDisplayVm, String keyboard,
             List<Long> affinityGroupIdList, Map<String, String> 
customParameters, String customId, Map<String, Map<Integer, String>> 
dhcpOptionMap, Map<Long, DiskOffering> datadiskTemplateToDiskOfferringMap) 
throws InsufficientCapacityException, ResourceUnavailableException,
-            ConcurrentOperationException, StorageUnavailableException, 
ResourceAllocationException {
+    ConcurrentOperationException, StorageUnavailableException, 
ResourceAllocationException {
 
         _accountMgr.checkAccess(caller, null, true, owner);
 
@@ -3336,7 +3295,7 @@ protected UserVm createVirtualMachine(DataCenter zone, 
ServiceOffering serviceOf
             if (rootDiskSize <= 0) {
                 throw new InvalidParameterValueException("Root disk size 
should be a positive number.");
             }
-            size = rootDiskSize * GB_TO_BYTES;
+            size = rootDiskSize * GiB_TO_BYTES;
         } else {
             // For baremetal, size can be null
             Long templateSize = 
_templateDao.findById(template.getId()).getSize();
@@ -3356,7 +3315,7 @@ protected UserVm createVirtualMachine(DataCenter zone, 
ServiceOffering serviceOf
                     throw new InvalidParameterValueException("VM Creation 
failed. Volume size: " + diskSize + "GB is out of allowed range. Max: " + 
customDiskOfferingMaxSize
                             + " Min:" + customDiskOfferingMinSize);
                 }
-                size += diskSize * GB_TO_BYTES;
+                size += diskSize * GiB_TO_BYTES;
             }
             size += _diskOfferingDao.findById(diskOfferingId).getDiskSize();
         }
@@ -3418,7 +3377,7 @@ protected UserVm createVirtualMachine(DataCenter zone, 
ServiceOffering serviceOf
                     throw new InvalidParameterValueException("Unable to find 
affinity group " + ag);
                 } else if 
(!_affinityGroupService.isAffinityGroupProcessorAvailable(ag.getType())) {
                     throw new InvalidParameterValueException("Affinity group 
type is not supported for group: " + ag + " ,type: " + ag.getType()
-                            + " , Please try again after removing the affinity 
group");
+                    + " , Please try again after removing the affinity group");
                 } else {
                     // verify permissions
                     if (ag.getAclType() == ACLType.Domain) {
@@ -3499,12 +3458,7 @@ protected UserVm createVirtualMachine(DataCenter zone, 
ServiceOffering serviceOf
                 }
 
                 NetworkOffering ntwkOffering = 
_networkOfferingDao.findById(network.getNetworkOfferingId());
-                Long physicalNetworkId = 
_networkModel.findPhysicalNetworkId(zone.getId(), ntwkOffering.getTags(),
-                        ntwkOffering.getTrafficType());
-                if (physicalNetworkId == null) {
-                    throw new InvalidParameterValueException("Network in which 
is VM getting deployed could not be" +
-                            " streched to the zone, as we could not find a 
valid physical network");
-                }
+                Long physicalNetworkId = 
_networkModel.findPhysicalNetworkId(zone.getId(), ntwkOffering.getTags(), 
ntwkOffering.getTrafficType());
 
                 String provider = 
_ntwkSrvcDao.getProviderForServiceInNetwork(network.getId(), 
Service.Connectivity);
                 if 
(!_networkModel.isProviderEnabledInPhysicalNetwork(physicalNetworkId, 
provider)) {
@@ -3515,10 +3469,10 @@ protected UserVm createVirtualMachine(DataCenter zone, 
ServiceOffering serviceOf
 
             //relax the check if the caller is admin account
             if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
-            if (!(network.getGuestType() == Network.GuestType.Shared && 
network.getAclType() == ACLType.Domain)
-                    && !(network.getAclType() == ACLType.Account && 
network.getAccountId() == accountId)) {
-                throw new InvalidParameterValueException("only shared network 
or isolated network with the same account_id can be added to vm");
-            }
+                if (!(network.getGuestType() == Network.GuestType.Shared && 
network.getAclType() == ACLType.Domain)
+                        && !(network.getAclType() == ACLType.Account && 
network.getAccountId() == accountId)) {
+                    throw new InvalidParameterValueException("only shared 
network or isolated network with the same account_id can be added to vm");
+                }
             }
 
             IpAddresses requestedIpPair = null;
@@ -3613,7 +3567,7 @@ protected UserVm createVirtualMachine(DataCenter zone, 
ServiceOffering serviceOf
             // In case of VMware since VM name must be unique within a DC, 
check if VM with the same hostname already exists in the zone.
             VMInstanceVO vmByHostName = 
_vmInstanceDao.findVMByHostNameInZone(hostName, zone.getId());
             if (vmByHostName != null && vmByHostName.getState() != 
VirtualMachine.State.Expunging) {
-                 throw new InvalidParameterValueException("There already 
exists a VM by the name: " + hostName + ".");
+                throw new InvalidParameterValueException("There already exists 
a VM by the name: " + hostName + ".");
             }
         } else {
             if (hostName == null) {
@@ -3703,9 +3657,9 @@ private String generateHostName(String uuidName) {
     }
 
     private UserVmVO commitUserVm(final DataCenter zone, final 
VirtualMachineTemplate template, final String hostName, final String 
displayName, final Account owner,
-                                  final Long diskOfferingId, final Long 
diskSize, final String userData, final Account caller, final Boolean 
isDisplayVm, final String keyboard,
-                                  final long accountId, final long userId, 
final ServiceOfferingVO offering, final boolean isIso, final String 
sshPublicKey, final LinkedHashMap<String, NicProfile> networkNicMap,
-                                  final long id, final String instanceName, 
final String uuidName, final HypervisorType hypervisorType, final Map<String, 
String> customParameters, final Map<String, Map<Integer, String>> 
extraDhcpOptionMap, final Map<Long, DiskOffering> 
dataDiskTemplateToDiskOfferingMap) throws InsufficientCapacityException {
+            final Long diskOfferingId, final Long diskSize, final String 
userData, final Account caller, final Boolean isDisplayVm, final String 
keyboard,
+            final long accountId, final long userId, final ServiceOfferingVO 
offering, final boolean isIso, final String sshPublicKey, final 
LinkedHashMap<String, NicProfile> networkNicMap,
+            final long id, final String instanceName, final String uuidName, 
final HypervisorType hypervisorType, final Map<String, String> 
customParameters, final Map<String, Map<Integer, String>> extraDhcpOptionMap, 
final Map<Long, DiskOffering> dataDiskTemplateToDiskOfferingMap) throws 
InsufficientCapacityException {
         return Transaction.execute(new 
TransactionCallbackWithException<UserVmVO, InsufficientCapacityException>() {
             @Override
             public UserVmVO doInTransaction(TransactionStatus status) throws 
InsufficientCapacityException {
@@ -3723,8 +3677,9 @@ public UserVmVO doInTransaction(TransactionStatus status) 
throws InsufficientCap
                     vm.setDetail("SSH.PublicKey", sshPublicKey);
                 }
 
-                if (keyboard != null && !keyboard.isEmpty())
+                if (keyboard != null && !keyboard.isEmpty()) {
                     vm.setDetail(VmDetailConstants.KEYBOARD, keyboard);
+                }
 
                 if (isIso) {
                     vm.setIsoId(template.getId());
@@ -3834,7 +3789,7 @@ public void validateRootDiskResize(final HypervisorType 
hypervisorType, Long roo
     {
         // rootdisksize must be larger than template.
         if ((rootDiskSize << 30) < templateVO.getSize()) {
-            Long templateVOSizeGB = templateVO.getSize() / GB_TO_BYTES;
+            Long templateVOSizeGB = templateVO.getSize() / GiB_TO_BYTES;
             String error = "Unsupported: rootdisksize override is smaller than 
template size " + templateVO.getSize() + "B (" + templateVOSizeGB + "GB)";
             s_logger.error(error);
             throw new InvalidParameterValueException(error);
@@ -3846,7 +3801,7 @@ public void validateRootDiskResize(final HypervisorType 
hypervisorType, Long roo
                 s_logger.error(error);
                 throw new InvalidParameterValueException(error);
             } else {
-                s_logger.debug("Rootdisksize override validation successful. 
Template root disk size " + (templateVO.getSize() / GB_TO_BYTES) + "GB Root 
disk size specified " + rootDiskSize + "GB");
+                s_logger.debug("Rootdisksize override validation successful. 
Template root disk size " + (templateVO.getSize() / GiB_TO_BYTES) + "GB Root 
disk size specified " + rootDiskSize + "GB");
             }
         } else {
             s_logger.debug("Root disk size specified is " + (rootDiskSize << 
30) + "B and Template root disk size is " + templateVO.getSize() + "B. Both are 
equal so no need to override");
@@ -3911,18 +3866,19 @@ public void generateUsageEvent(VirtualMachine vm, 
boolean isDisplay, String even
 
     @Override
     public void collectVmNetworkStatistics (final UserVm userVm) {
-       if (!userVm.getHypervisorType().equals(HypervisorType.KVM))
+        if (!userVm.getHypervisorType().equals(HypervisorType.KVM)) {
             return;
-       s_logger.debug("Collect vm network statistics from host before stopping 
Vm");
-       long hostId = userVm.getHostId();
-       List<String> vmNames = new ArrayList<String>();
-       vmNames.add(userVm.getInstanceName());
-       final HostVO host = _hostDao.findById(hostId);
-
-       GetVmNetworkStatsAnswer networkStatsAnswer = null;
-       try {
+        }
+        s_logger.debug("Collect vm network statistics from host before 
stopping Vm");
+        long hostId = userVm.getHostId();
+        List<String> vmNames = new ArrayList<String>();
+        vmNames.add(userVm.getInstanceName());
+        final HostVO host = _hostDao.findById(hostId);
+
+        GetVmNetworkStatsAnswer networkStatsAnswer = null;
+        try {
             networkStatsAnswer = (GetVmNetworkStatsAnswer) 
_agentMgr.easySend(hostId, new GetVmNetworkStatsCommand(vmNames, 
host.getGuid(), host.getName()));
-       } catch (Exception e) {
+        } catch (Exception e) {
             s_logger.warn("Error while collecting network stats for vm: " + 
userVm.getHostName() + " from host: " + host.getName(), e);
             return;
         }
@@ -3937,11 +3893,13 @@ public void collectVmNetworkStatistics (final UserVm 
userVm) {
                     @Override
                     public void doInTransactionWithoutResult(TransactionStatus 
status) {
                         HashMap<String, List<VmNetworkStatsEntry>> 
vmNetworkStatsByName = networkStatsAnswerFinal.getVmNetworkStatsMap();
-                        if (vmNetworkStatsByName == null)
-                           return;
+                        if (vmNetworkStatsByName == null) {
+                            return;
+                        }
                         List<VmNetworkStatsEntry> vmNetworkStats = 
vmNetworkStatsByName.get(userVm.getInstanceName());
-                        if (vmNetworkStats == null)
-                           return;
+                        if (vmNetworkStats == null) {
+                            return;
+                        }
 
                         for (VmNetworkStatsEntry vmNetworkStat:vmNetworkStats) 
{
                             SearchCriteria<NicVO> sc_nic = 
_nicDao.createSearchCriteria();
@@ -3949,7 +3907,9 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             NicVO nic = _nicDao.search(sc_nic, null).get(0);
                             List<VlanVO> vlan = 
_vlanDao.listVlansByNetworkId(nic.getNetworkId());
                             if (vlan == null || vlan.size() == 0 || 
vlan.get(0).getVlanType() != VlanType.DirectAttached)
+                            {
                                 break; // only get network statistics for 
DirectAttached network (shared networks in Basic zone and Advanced zone 
with/without SG)
+                            }
                             UserStatisticsVO previousvmNetworkStats = 
_userStatsDao.findBy(userVm.getAccountId(), userVm.getDataCenterId(), 
nic.getNetworkId(), nic.getIPv4Address(), userVm.getId(), "UserVm");
                             if (previousvmNetworkStats == null) {
                                 previousvmNetworkStats = new 
UserStatisticsVO(userVm.getAccountId(), 
userVm.getDataCenterId(),nic.getIPv4Address(), userVm.getId(), "UserVm", 
nic.getNetworkId());
@@ -3969,7 +3929,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
 
                             if (previousvmNetworkStats != null
                                     && 
((previousvmNetworkStats.getCurrentBytesSent() != 
vmNetworkStat_lock.getCurrentBytesSent())
-                                    || 
(previousvmNetworkStats.getCurrentBytesReceived() != 
vmNetworkStat_lock.getCurrentBytesReceived()))) {
+                                            || 
(previousvmNetworkStats.getCurrentBytesReceived() != 
vmNetworkStat_lock.getCurrentBytesReceived()))) {
                                 s_logger.debug("vm network stats changed from 
the time GetNmNetworkStatsCommand was sent. " +
                                         "Ignoring current answer. Host: " + 
host.getName()  + " . VM: " + vmNetworkStat.getVmName() +
                                         " Sent(Bytes): " + 
vmNetworkStat.getBytesSent() + " Received(Bytes): " + 
vmNetworkStat.getBytesReceived());
@@ -4061,7 +4021,8 @@ public UserVm startVirtualMachine(DeployVMCmd cmd) throws 
ResourceUnavailableExc
         return startVirtualMachine(cmd, null, cmd.getDeploymentPlanner());
     }
 
-    protected UserVm startVirtualMachine(DeployVMCmd cmd, 
Map<VirtualMachineProfile.Param, Object> additonalParams, String 
deploymentPlannerToUse) throws ResourceUnavailableException,
+    private UserVm startVirtualMachine(DeployVMCmd cmd, 
Map<VirtualMachineProfile.Param, Object> additonalParams, String 
deploymentPlannerToUse)
+            throws ResourceUnavailableException,
             InsufficientCapacityException, ConcurrentOperationException {
 
         long vmId = cmd.getEntityId();
@@ -4196,7 +4157,7 @@ public boolean finalizeDeployment(Commands cmds, 
VirtualMachineProfile profile,
             diskstats = _vmDiskStatsDao.findBy(userVm.getAccountId(), 
userVm.getDataCenterId(), userVm.getId(), volume.getId());
             if (diskstats == null) {
                 diskstats = new VmDiskStatisticsVO(userVm.getAccountId(), 
userVm.getDataCenterId(), userVm.getId(), volume.getId());
-               _vmDiskStatsDao.persist(diskstats);
+                _vmDiskStatsDao.persist(diskstats);
             }
         }
 
@@ -4209,8 +4170,9 @@ public boolean finalizeCommandsOnStart(Commands cmds, 
VirtualMachineProfile prof
         UserVmVO vm = _vmDao.findById(profile.getId());
         List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.findByVm(vm.getId());
         RestoreVMSnapshotCommand command = 
_vmSnapshotMgr.createRestoreCommand(vm, vmSnapshots);
-        if (command != null)
+        if (command != null) {
             cmds.addCommand("restoreVMSnapshot", command);
+        }
         return true;
     }
 
@@ -4259,7 +4221,7 @@ public boolean  finalizeStart(VirtualMachineProfile 
profile, long hostId, Comman
                         }
                     }
                 }
-        }
+            }
         }
         boolean ipChanged = false;
         if (originalIp != null && !originalIp.equalsIgnoreCase(returnedIp)) {
@@ -4353,9 +4315,9 @@ public UserVm stopVirtualMachine(long vmId, boolean 
forced) throws ConcurrentOpe
                 status = vmEntity.stop(Long.toString(userId));
             }
             if (status) {
-               return _vmDao.findById(vmId);
+                return _vmDao.findById(vmId);
             } else {
-               return null;
+                return null;
             }
         } catch (ResourceUnavailableException e) {
             throw new CloudRuntimeException("Unable to contact the agent to 
stop the virtual machine " + vm, e);
@@ -4474,9 +4436,9 @@ public void finalizeStop(VirtualMachineProfile profile, 
Answer answer) {
             if (template.getEnablePassword()) {
                 if (vm.getDetail("password") != null) {
                     password = 
DBEncryptionUtil.decrypt(vm.getDetail("password"));
-                 } else {
+                } else {
                     password = _mgr.generateRandomPassword();
-                 }
+                }
             }
 
             if (!validPassword(password)) {
@@ -4585,8 +4547,9 @@ public UserVm destroyVm(long vmId, boolean expunge) 
throws ResourceUnavailableEx
     @Override
     public void collectVmDiskStatistics(final UserVm userVm) {
         // support KVM only util 2013.06.25
-        if (!userVm.getHypervisorType().equals(HypervisorType.KVM))
+        if (!userVm.getHypervisorType().equals(HypervisorType.KVM)) {
             return;
+        }
         s_logger.debug("Collect vm disk statistics from host before stopping 
Vm");
         long hostId = userVm.getHostId();
         List<String> vmNames = new ArrayList<String>();
@@ -4611,18 +4574,21 @@ public void collectVmDiskStatistics(final UserVm 
userVm) {
                     @Override
                     public void doInTransactionWithoutResult(TransactionStatus 
status) {
                         HashMap<String, List<VmDiskStatsEntry>> 
vmDiskStatsByName = diskStatsAnswerFinal.getVmDiskStatsMap();
-                        if (vmDiskStatsByName == null)
+                        if (vmDiskStatsByName == null) {
                             return;
+                        }
                         List<VmDiskStatsEntry> vmDiskStats = 
vmDiskStatsByName.get(userVm.getInstanceName());
-                        if (vmDiskStats == null)
+                        if (vmDiskStats == null) {
                             return;
+                        }
 
                         for (VmDiskStatsEntry vmDiskStat : vmDiskStats) {
                             SearchCriteria<VolumeVO> sc_volume = 
_volsDao.createSearchCriteria();
                             sc_volume.addAnd("path", SearchCriteria.Op.EQ, 
vmDiskStat.getPath());
                             List<VolumeVO> volumes = 
_volsDao.search(sc_volume, null);
-                            if ((volumes == null) || (volumes.size() == 0))
+                            if ((volumes == null) || (volumes.size() == 0)) {
                                 break;
+                            }
                             VolumeVO volume = volumes.get(0);
                             VmDiskStatisticsVO previousVmDiskStats = 
_vmDiskStatsDao.findBy(userVm.getAccountId(), userVm.getDataCenterId(), 
userVm.getId(), volume.getId());
                             VmDiskStatisticsVO vmDiskStat_lock = 
_vmDiskStatsDao.lock(userVm.getAccountId(), userVm.getDataCenterId(), 
userVm.getId(), volume.getId());
@@ -4640,19 +4606,19 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
 
                             if (previousVmDiskStats != null
                                     && 
((previousVmDiskStats.getCurrentIORead() != vmDiskStat_lock.getCurrentIORead()) 
|| ((previousVmDiskStats.getCurrentIOWrite() != vmDiskStat_lock
-                                            .getCurrentIOWrite())
+                                    .getCurrentIOWrite())
                                             || 
(previousVmDiskStats.getCurrentBytesRead() != 
vmDiskStat_lock.getCurrentBytesRead()) || (previousVmDiskStats
-                                            .getCurrentBytesWrite() != 
vmDiskStat_lock.getCurrentBytesWrite())))) {
+                                                    .getCurrentBytesWrite() != 
vmDiskStat_lock.getCurrentBytesWrite())))) {
                                 s_logger.debug("vm disk stats changed from the 
time GetVmDiskStatsCommand was sent. " + "Ignoring current answer. Host: " + 
host.getName()
-                                        + " . VM: " + vmDiskStat.getVmName() + 
" IO Read: " + vmDiskStat.getIORead() + " IO Write: " + vmDiskStat.getIOWrite() 
+ " Bytes Read: "
-                                        + vmDiskStat.getBytesRead() + " Bytes 
Write: " + vmDiskStat.getBytesWrite());
+                                + " . VM: " + vmDiskStat.getVmName() + " IO 
Read: " + vmDiskStat.getIORead() + " IO Write: " + vmDiskStat.getIOWrite() + " 
Bytes Read: "
+                                + vmDiskStat.getBytesRead() + " Bytes Write: " 
+ vmDiskStat.getBytesWrite());
                                 continue;
                             }
 
                             if (vmDiskStat_lock.getCurrentIORead() > 
vmDiskStat.getIORead()) {
                                 if (s_logger.isDebugEnabled()) {
                                     s_logger.debug("Read # of IO that's less 
than the last one.  " + "Assuming something went wrong and persisting it. Host: 
" + host.getName()
-                                            + " . VM: " + 
vmDiskStat.getVmName() + " Reported: " + vmDiskStat.getIORead() + " Stored: " + 
vmDiskStat_lock.getCurrentIORead());
+                                    + " . VM: " + vmDiskStat.getVmName() + " 
Reported: " + vmDiskStat.getIORead() + " Stored: " + 
vmDiskStat_lock.getCurrentIORead());
                                 }
                                 
vmDiskStat_lock.setNetIORead(vmDiskStat_lock.getNetIORead() + 
vmDiskStat_lock.getCurrentIORead());
                             }
@@ -4660,7 +4626,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             if (vmDiskStat_lock.getCurrentIOWrite() > 
vmDiskStat.getIOWrite()) {
                                 if (s_logger.isDebugEnabled()) {
                                     s_logger.debug("Write # of IO that's less 
than the last one.  " + "Assuming something went wrong and persisting it. Host: 
" + host.getName()
-                                            + " . VM: " + 
vmDiskStat.getVmName() + " Reported: " + vmDiskStat.getIOWrite() + " Stored: " 
+ vmDiskStat_lock.getCurrentIOWrite());
+                                    + " . VM: " + vmDiskStat.getVmName() + " 
Reported: " + vmDiskStat.getIOWrite() + " Stored: " + 
vmDiskStat_lock.getCurrentIOWrite());
                                 }
                                 
vmDiskStat_lock.setNetIOWrite(vmDiskStat_lock.getNetIOWrite() + 
vmDiskStat_lock.getCurrentIOWrite());
                             }
@@ -4668,7 +4634,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             if (vmDiskStat_lock.getCurrentBytesRead() > 
vmDiskStat.getBytesRead()) {
                                 if (s_logger.isDebugEnabled()) {
                                     s_logger.debug("Read # of Bytes that's 
less than the last one.  " + "Assuming something went wrong and persisting it. 
Host: " + host.getName()
-                                            + " . VM: " + 
vmDiskStat.getVmName() + " Reported: " + vmDiskStat.getBytesRead() + " Stored: 
" + vmDiskStat_lock.getCurrentBytesRead());
+                                    + " . VM: " + vmDiskStat.getVmName() + " 
Reported: " + vmDiskStat.getBytesRead() + " Stored: " + 
vmDiskStat_lock.getCurrentBytesRead());
                                 }
                                 
vmDiskStat_lock.setNetBytesRead(vmDiskStat_lock.getNetBytesRead() + 
vmDiskStat_lock.getCurrentBytesRead());
                             }
@@ -4676,8 +4642,8 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             if (vmDiskStat_lock.getCurrentBytesWrite() > 
vmDiskStat.getBytesWrite()) {
                                 if (s_logger.isDebugEnabled()) {
                                     s_logger.debug("Write # of Bytes that's 
less than the last one.  " + "Assuming something went wrong and persisting it. 
Host: " + host.getName()
-                                            + " . VM: " + 
vmDiskStat.getVmName() + " Reported: " + vmDiskStat.getBytesWrite() + " Stored: 
"
-                                            + 
vmDiskStat_lock.getCurrentBytesWrite());
+                                    + " . VM: " + vmDiskStat.getVmName() + " 
Reported: " + vmDiskStat.getBytesWrite() + " Stored: "
+                                    + vmDiskStat_lock.getCurrentBytesWrite());
                                 }
                                 
vmDiskStat_lock.setNetBytesWrite(vmDiskStat_lock.getNetBytesWrite() + 
vmDiskStat_lock.getCurrentBytesWrite());
                             }
@@ -4760,7 +4726,7 @@ public HypervisorType getHypervisorTypeOfUserVM(long 
vmId) {
 
     @Override
     public UserVm createVirtualMachine(DeployVMCmd cmd) throws 
InsufficientCapacityException, ResourceUnavailableException, 
ConcurrentOperationException,
-            StorageUnavailableException, ResourceAllocationException {
+    StorageUnavailableException, ResourceAllocationException {
         //Verify that all objects exist before passing them to the service
         Account owner = 
_accountService.getActiveAccountById(cmd.getEntityOwnerId());
 
@@ -4862,13 +4828,13 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) 
throws InsufficientCapacityE
         return vm;
     }
 
-    private List<Long> getSecurityGroupIdList(SecurityGroupAction cmd) {
+    protected List<Long> getSecurityGroupIdList(SecurityGroupAction cmd) {
         if (cmd.getSecurityGroupNameList() != null && 
cmd.getSecurityGroupIdList() != null) {
             throw new InvalidParameterValueException("securitygroupids 
parameter is mutually exclusive with securitygroupnames parameter");
         }
 
-       //transform group names to ids here
-       if (cmd.getSecurityGroupNameList() != null) {
+        //transform group names to ids here
+        if (cmd.getSecurityGroupNameList() != null) {
             List<Long> securityGroupIds = new ArrayList<Long>();
             for (String groupName : cmd.getSecurityGroupNameList()) {
                 SecurityGroup sg = 
_securityGroupMgr.getSecurityGroup(groupName, cmd.getEntityOwnerId());
@@ -4889,13 +4855,13 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) 
throws InsufficientCapacityE
     // if specified, minIops should be <= maxIops
     private void verifyDetails(Map<String,String> details) {
         if (details != null) {
-            String minIops = (String)details.get("minIops");
-            String maxIops = (String)details.get("maxIops");
+            String minIops = details.get("minIops");
+            String maxIops = details.get("maxIops");
 
             verifyMinAndMaxIops(minIops, maxIops);
 
-            minIops = (String)details.get("minIopsDo");
-            maxIops = (String)details.get("maxIopsDo");
+            minIops = details.get("minIopsDo");
+            maxIops = details.get("maxIopsDo");
 
             verifyMinAndMaxIops(minIops, maxIops);
         }
@@ -4983,7 +4949,7 @@ public VirtualMachine vmStorageMigration(Long vmId, 
StoragePool destPool) {
         HypervisorType destHypervisorType = destPool.getHypervisor();
         if (destHypervisorType == null) {
             destHypervisorType = _clusterDao.findById(
-                destPool.getClusterId()).getHypervisorType();
+                    destPool.getClusterId()).getHypervisorType();
         }
 
         if (vm.getHypervisorType() != destHypervisorType && destHypervisorType 
!= HypervisorType.Any) {
@@ -5011,12 +4977,12 @@ private boolean isVMUsingLocalStorage(VMInstanceVO vm) {
             }
         }
         return usesLocalStorage;
-}
+    }
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_VM_MIGRATE, eventDescription = 
"migrating VM", async = true)
     public VirtualMachine migrateVirtualMachine(Long vmId, Host 
destinationHost) throws ResourceUnavailableException, 
ConcurrentOperationException, ManagementServerException,
-            VirtualMachineMigrationException {
+    VirtualMachineMigrationException {
         // access check - only root admin can migrate VM
         Account caller = CallContext.current().getCallingAccount();
         if (!_accountMgr.isRootAdmin(caller.getId())) {
@@ -5076,7 +5042,7 @@ public VirtualMachine migrateVirtualMachine(Long vmId, 
Host destinationHost) thr
         // check if host is UP
         if (destinationHost.getState() != com.cloud.host.Status.Up || 
destinationHost.getResourceState() != ResourceState.Enabled) {
             throw new InvalidParameterValueException("Cannot migrate VM, 
destination host is not in correct state, has status: " + 
destinationHost.getState() + ", state: "
-                            + destinationHost.getResourceState());
+                    + destinationHost.getResourceState());
         }
 
         if (vm.getType() != VirtualMachine.Type.User) {
@@ -5092,7 +5058,7 @@ public VirtualMachine migrateVirtualMachine(Long vmId, 
Host destinationHost) thr
 
         checkHostsDedication(vm, srcHostId, destinationHost.getId());
 
-         // call to core process
+        // call to core process
         DataCenterVO dcVO = _dcDao.findById(destinationHost.getDataCenterId());
         HostPodVO pod = _podDao.findById(destinationHost.getPodId());
         Cluster cluster = _clusterDao.findById(destinationHost.getClusterId());
@@ -5103,10 +5069,10 @@ public VirtualMachine migrateVirtualMachine(Long vmId, 
Host destinationHost) thr
         if (_capacityMgr.checkIfHostReachMaxGuestLimit(destinationHostVO)) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("Host name: " + destinationHost.getName() + ", 
hostId: " + destinationHost.getId()
-                        + " already has max Running VMs(count includes system 
VMs), cannot migrate to this host");
+                + " already has max Running VMs(count includes system VMs), 
cannot migrate to this host");
             }
             throw new VirtualMachineMigrationException("Destination host, 
hostId: " + destinationHost.getId()
-                            + " already has max Running VMs(count includes 
system VMs), cannot migrate to this host");
+            + " already has max Running VMs(count includes system VMs), cannot 
migrate to this host");
         }
         //check if there are any ongoing volume snapshots on the volumes 
associated with the VM.
         s_logger.debug("Checking if there are any ongoing snapshots volumes 
associated with VM with ID " + vmId);
@@ -5123,7 +5089,7 @@ public VirtualMachine migrateVirtualMachine(Long vmId, 
Host destinationHost) thr
         _itMgr.migrate(vm.getUuid(), srcHostId, dest);
         VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId);
         if (vmInstance.getType().equals(VirtualMachine.Type.User)) {
-        return _vmDao.findById(vmId);
+            return _vmDao.findById(vmId);
         } else {
             return vmInstance;
         }
@@ -5240,7 +5206,7 @@ public void checkHostsDedication(VMInstanceVO vm, long 
srcHostId, long destHostI
                     //Check if all vms on destination host are created using 
strict implicit mode
                     if (!checkIfAllVmsCreatedInStrictMode(accountOfVm, 
vmsOnDest)) {
                         msg = "VM of account " + accountOfVm + " with strict 
implicit deployment planner being migrated to host " + destHost.getName()
-                                + " not having all vms strict implicitly 
dedicated to account " + accountOfVm;
+                        + " not having all vms strict implicitly dedicated to 
account " + accountOfVm;
                     }
                 } else {
                     //If vm is deployed using preferred implicit planner, 
check if all vms on destination host must be
@@ -5249,7 +5215,7 @@ public void checkHostsDedication(VMInstanceVO vm, long 
srcHostId, long destHostI
                         ServiceOfferingVO destPlanner = 
_offeringDao.findById(vm.getId(), vmsDest.getServiceOfferingId());
                         if (!((destPlanner.getDeploymentPlanner() != null && 
destPlanner.getDeploymentPlanner().equals("ImplicitDedicationPlanner")) && 
vmsDest.getAccountId() == accountOfVm)) {
                             msg = "VM of account " + accountOfVm + " with 
preffered implicit deployment planner being migrated to host " + 
destHost.getName()
-                                    + " not having all vms implicitly 
dedicated to account " + accountOfVm;
+                            + " not having all vms implicitly dedicated to 
account " + accountOfVm;
                         }
                     }
                 }
@@ -5319,8 +5285,9 @@ private boolean 
isServiceOfferingUsingPlannerInPreferredMode(long serviceOfferin
 
     private boolean checkIfAllVmsCreatedInStrictMode(Long accountId, 
List<VMInstanceVO> allVmsOnHost) {
         boolean createdByImplicitStrict = true;
-        if (allVmsOnHost.isEmpty())
+        if (allVmsOnHost.isEmpty()) {
             return false;
+        }
         for (VMInstanceVO vm : allVmsOnHost) {
             if (!isImplicitPlannerUsedByOffering(vm.getServiceOfferingId()) || 
vm.getAccountId() != accountId) {
                 s_logger.info("Host " + vm.getHostId() + " found to be running 
a vm created by a planner other" + " than implicit, or running vms of other 
account");
@@ -5355,7 +5322,7 @@ private boolean isImplicitPlannerUsedByOffering(long 
offeringId) {
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_VM_MIGRATE, eventDescription = 
"migrating VM", async = true)
     public VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host 
destinationHost, Map<String, String> volumeToPool) throws 
ResourceUnavailableException,
-            ConcurrentOperationException, ManagementServerException, 
VirtualMachineMigrationException {
+    ConcurrentOperationException, ManagementServerException, 
VirtualMachineMigrationException {
         // Access check - only root administrator can migrate VM.
         Account caller = CallContext.current().getCallingAccount();
         if (!_accountMgr.isRootAdmin(caller.getId())) {
@@ -5502,7 +5469,7 @@ public VirtualMachine 
migrateVirtualMachineWithVolume(Long vmId, Host destinatio
         HostVO destinationHostVO = _hostDao.findById(destinationHost.getId());
         if (_capacityMgr.checkIfHostReachMaxGuestLimit(destinationHostVO)) {
             throw new VirtualMachineMigrationException("Host name: " + 
destinationHost.getName() + ", hostId: " + destinationHost.getId()
-                    + " already has max running vms (count includes system 
VMs). Cannot" + " migrate to this host");
+            + " already has max running vms (count includes system VMs). 
Cannot" + " migrate to this host");
         }
 
         checkHostsDedication(vm, srcHostId, destinationHost.getId());
@@ -5632,8 +5599,8 @@ public UserVm moveVMToUser(final AssignVMCmd cmd) throws 
ResourceAllocationExcep
             public void doInTransactionWithoutResult(TransactionStatus status) 
{
                 //generate destroy vm event for usage
                 UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, 
vm.getAccountId(), vm.getDataCenterId(),
-                                                  vm.getId(), 
vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
-                                                  
vm.getHypervisorType().toString(), VirtualMachine.class.getName(), 
vm.getUuid(), vm.isDisplayVm());
+                        vm.getId(), vm.getHostName(), 
vm.getServiceOfferingId(), vm.getTemplateId(),
+                        vm.getHypervisorType().toString(), 
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
                 // update resource counts for old account
                 resourceCountDecrement(oldAccount.getAccountId(), 
vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize()));
 
@@ -5645,7 +5612,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                 // OS 2: update volume
                 for (VolumeVO volume : volumes) {
                     
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE, 
volume.getAccountId(), volume.getDataCenterId(), volume.getId(), 
volume.getName(),
-                                                      Volume.class.getName(), 
volume.getUuid(), volume.isDisplayVolume());
+                            Volume.class.getName(), volume.getUuid(), 
volume.isDisplayVolume());
                     
_resourceLimitMgr.decrementResourceCount(oldAccount.getAccountId(), 
ResourceType.volume);
                     
_resourceLimitMgr.decrementResourceCount(oldAccount.getAccountId(), 
ResourceType.primary_storage, new Long(volume.getSize()));
                     volume.setAccountId(newAccount.getAccountId());
@@ -5654,8 +5621,8 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                     
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(), 
ResourceType.volume);
                     
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(), 
ResourceType.primary_storage, new Long(volume.getSize()));
                     
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, 
volume.getAccountId(), volume.getDataCenterId(), volume.getId(), 
volume.getName(),
-                                                      
volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), 
Volume.class.getName(),
-                                                      volume.getUuid(), 
volume.isDisplayVolume());
+                            volume.getDiskOfferingId(), 
volume.getTemplateId(), volume.getSize(), Volume.class.getName(),
+                            volume.getUuid(), volume.isDisplayVolume());
                 }
 
                 //update resource count of new account
@@ -5663,10 +5630,10 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
 
                 //generate usage events to account for this change
                 UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, 
vm.getAccountId(), vm.getDataCenterId(), vm.getId(),
-                                                  vm.getHostName(), 
vm.getServiceOfferingId(), vm.getTemplateId(), 
vm.getHypervisorType().toString(),
-                                                  
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
+                        vm.getHostName(), vm.getServiceOfferingId(), 
vm.getTemplateId(), vm.getHypervisorType().toString(),
+                        VirtualMachine.class.getName(), vm.getUuid(), 
vm.isDisplayVm());
             }
-            });
+        });
 
         VirtualMachine vmoi = _itMgr.findById(vm.getId());
         VirtualMachineProfileImpl vmOldProfile = new 
VirtualMachineProfileImpl(vmoi);
@@ -5921,7 +5888,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                     List<NetworkOfferingVO> requiredOfferings = 
_networkOfferingDao.listByAvailability(Availability.Required, false);
                     if (requiredOfferings.size() < 1) {
                         throw new InvalidParameterValueException("Unable to 
find network offering with availability=" + Availability.Required
-                                        + " to automatically create the 
network as a part of vm creation");
+                                + " to automatically create the network as a 
part of vm creation");
                     }
                     if (requiredOfferings.get(0).getState() == 
NetworkOffering.State.Enabled) {
                         // get Virtual networks
@@ -5938,9 +5905,9 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             s_logger.debug("Creating network for account " + 
newAccount + " from the network offering id=" + requiredOfferings.get(0).getId()
                                     + " as a part of deployVM process");
                             Network newNetwork = 
_networkMgr.createGuestNetwork(requiredOfferings.get(0).getId(), 
newAccount.getAccountName() + "-network",
-                                                                               
 newAccount.getAccountName() + "-network", null, null, null, false, null, 
newAccount,
-                                                                               
 null, physicalNetwork, zone.getId(), ACLType.Account, null, null,
-                                                                               
 null, null, true, null, null);
+                                    newAccount.getAccountName() + "-network", 
null, null, null, false, null, newAccount,
+                                    null, physicalNetwork, zone.getId(), 
ACLType.Account, null, null,
+                                    null, null, true, null, null);
                             // if the network offering has persistent set to 
true, implement the network
                             if (requiredOfferings.get(0).getIsPersistent()) {
                                 DeployDestination dest = new 
DeployDestination(zone, null, null, null);
@@ -5966,7 +5933,7 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
                             defaultNetwork = 
_networkDao.findById(newNetwork.getId());
                         } else if (virtualNetworks.size() > 1) {
                             throw new InvalidParameterValueException("More 
than 1 default Isolated networks are found " + "for account " + newAccount
-                                            + "; please specify networkIds");
+                                    + "; please specify networkIds");
                         } else {
                             defaultNetwork = 
_networkDao.findById(virtualNetworks.get(0).getId());
                         }
@@ -6067,7 +6034,7 @@ public UserVm restoreVMInternal(Account caller, UserVmVO 
vm, Long newTemplateId)
             if (templateId == null) {
                 // Assuming that for a vm deployed using ISO, template ID is 
set to NULL
                 isISO = true;
-               templateId = vm.getIsoId();
+                templateId = vm.getIsoId();
             }
 
             // If target VM has associated VM snapshots then don't allow 
restore of VM
@@ -6126,10 +6093,10 @@ public UserVm restoreVMInternal(Account caller, 
UserVmVO vm, Long newTemplateId)
                     vm.setTemplateId(newTemplateId);
                     _vmDao.update(vmId, vm);
                 } else {
-                newVol = volumeMgr.allocateDuplicateVolume(root, 
newTemplateId);
-                vm.setGuestOSId(template.getGuestOSId());
-                vm.setTemplateId(newTemplateId);
-                _vmDao.update(vmId, vm);
+                    newVol = volumeMgr.allocateDuplicateVolume(root, 
newTemplateId);
+                    vm.setGuestOSId(template.getGuestOSId());
+                    vm.setTemplateId(newTemplateId);
+                    _vmDao.update(vmId, vm);
                 }
             } else {
                 newVol = volumeMgr.allocateDuplicateVolume(root, null);
@@ -6157,7 +6124,7 @@ public UserVm restoreVMInternal(Account caller, UserVmVO 
vm, Long newTemplateId)
                     s_logger.info("Expunging volume " + root.getId() + " from 
primary data store");
                     AsyncCallFuture<VolumeApiResult> future = 
_volService.expungeVolumeAsync(volFactory.getVolume(root.getId()));
                     try {
-                    future.get();
+                        future.get();
                     } catch (Exception e) {
                         s_logger.debug("Failed to expunge volume:" + 
root.getId(), e);
                     }
@@ -6218,7 +6185,8 @@ public UserVm restoreVMInternal(Account caller, UserVmVO 
vm, Long newTemplateId)
     }
 
     /**
-     * Perform basic checkings to make sure restore is possible. If not, 
#InvalidParameterValueException is thrown
+     * Perform basic checkings to make sure restore is possible. If not, 
#InvalidParameterValueException is thrown.
+     *
      * @param vm vm
      * @param template template
      * @throws InvalidParameterValueException if restore is not possible
@@ -6391,6 +6359,7 @@ private void encryptAndStorePassword(UserVmVO vm, String 
password) {
         }
     }
 
+    @Override
     public void persistDeviceBusInfo(UserVmVO vm, String rootDiskController) {
         String existingVmRootDiskController = 
vm.getDetail(VmDetailConstants.ROOT_DISK_CONTROLLER);
         if (StringUtils.isEmpty(existingVmRootDiskController) && 
!StringUtils.isEmpty(rootDiskController)) {
@@ -6410,7 +6379,7 @@ public String getConfigComponentName() {
     @Override
     public ConfigKey<?>[] getConfigKeys() {
         return new ConfigKey<?>[] {EnableDynamicallyScaleVm, 
AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, 
VmIpFetchThreadPoolMax,
-                VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails};
+            VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails};
     }
 
     @Override
@@ -6420,7 +6389,6 @@ public String getVmUserData(long vmId) {
             throw new InvalidParameterValueException("Unable to find virual 
machine with id " + vmId);
         }
 
-        //check permissions
         _accountMgr.checkAccess(CallContext.current().getCallingAccount(), 
null, true, vm);
         return vm.getUserData();
     }
@@ -6432,7 +6400,7 @@ public boolean isDisplayResourceEnabled(Long vmId) {
             return vm.isDisplayVm();
         }
 
-        return true; // no info then default to true
+        return true;
     }
 
     private boolean checkStatusOfVolumeSnapshots(long vmId, Volume.Type type) {
@@ -6458,4 +6426,4 @@ private boolean checkStatusOfVolumeSnapshots(long vmId, 
Volume.Type type) {
         }
         return false;
     }
-}
+}
\ No newline at end of file
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java 
b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java
index e58cd4077e9..b3df1050584 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java
@@ -16,6 +16,7 @@
 // under the License.
 
 package com.cloud.vm;
+
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -44,17 +45,6 @@
 import java.util.Map;
 import java.util.UUID;
 
-import com.cloud.dc.VlanVO;
-import com.cloud.dc.dao.VlanDao;
-import com.cloud.network.dao.IPAddressVO;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
-import org.mockito.Spy;
-
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.BaseCmd;
@@ -71,12 +61,23 @@
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
 
 import com.cloud.capacity.CapacityManager;
 import com.cloud.configuration.ConfigurationManager;
 import com.cloud.dc.DataCenter.NetworkType;
 import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.VlanVO;
 import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.dc.dao.VlanDao;
 import com.cloud.deploy.DeployDestination;
 import com.cloud.event.dao.UsageEventDao;
 import com.cloud.exception.ConcurrentOperationException;
@@ -93,6 +94,7 @@
 import com.cloud.network.Network.Service;
 import com.cloud.network.NetworkModel;
 import com.cloud.network.dao.IPAddressDao;
+import com.cloud.network.dao.IPAddressVO;
 import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkVO;
 import com.cloud.network.element.UserDataServiceProvider;
@@ -127,138 +129,107 @@
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
+@RunWith(MockitoJUnitRunner.class)
 public class UserVmManagerTest {
 
     @Spy
-    UserVmManagerImpl _userVmMgr = new UserVmManagerImpl();
+    @InjectMocks
+    private UserVmManagerImpl _userVmMgr;
     @Mock
-    VirtualMachineManager _itMgr;
+    private VirtualMachineManager _itMgr;
     @Mock
-    VolumeOrchestrationService _storageMgr;
+    private VolumeOrchestrationService _storageMgr;
     @Mock
-    Account _account;
+    private Account _account;
     @Mock
-    AccountManager _accountMgr;
+    private AccountManager _accountMgr;
     @Mock
-    AccountService _accountService;
+    private AccountService _accountService;
     @Mock
-    ConfigurationManager _configMgr;
+    private ConfigurationManager _configMgr;
     @Mock
-    CapacityManager _capacityMgr;
+    private CapacityManager _capacityMgr;
     @Mock
-    AccountDao _accountDao;
+    private AccountDao _accountDao;
     @Mock
-    ConfigurationDao _configDao;
+    private ConfigurationDao _configDao;
     @Mock
-    UserDao _userDao;
+    private UserDao _userDao;
     @Mock
-    UserVmDao _vmDao;
+    private UserVmDao _vmDao;
     @Mock
-    VMInstanceDao _vmInstanceDao;
+    private VMInstanceDao _vmInstanceDao;
     @Mock
-    VMTemplateDao _templateDao;
+    private VMTemplateDao _templateDao;
     @Mock
-    TemplateDataStoreDao _templateStoreDao;
+    private TemplateDataStoreDao _templateStoreDao;
     @Mock
-    VolumeDao _volsDao;
+    private VolumeDao _volsDao;
     @Mock
-    RestoreVMCmd _restoreVMCmd;
+    private RestoreVMCmd _restoreVMCmd;
     @Mock
-    AccountVO _accountMock;
+    private AccountVO _accountMock;
     @Mock
-    UserVO _userMock;
+    private UserVO _userMock;
     @Mock
-    UserVmVO _vmMock;
+    private UserVmVO _vmMock;
     @Mock
-    VMInstanceVO _vmInstance;
+    private VMInstanceVO _vmInstance;
     @Mock
-    VMTemplateVO _templateMock;
+    private VMTemplateVO _templateMock;
     @Mock
-    TemplateDataStoreVO _templateDataStoreMock;
+    private TemplateDataStoreVO _templateDataStoreMock;
     @Mock
-    VolumeVO _volumeMock;
+    private VolumeVO _volumeMock;
     @Mock
-    List<VolumeVO> _rootVols;
+    private List<VolumeVO> _rootVols;
     @Mock
-    Account _accountMock2;
+    private Account _accountMock2;
     @Mock
-    ServiceOfferingDao _offeringDao;
+    private ServiceOfferingDao _offeringDao;
     @Mock
-    ServiceOfferingVO _offeringVo;
+    private ServiceOfferingVO _offeringVo;
     @Mock
-    EntityManager _entityMgr;
+    private EntityManager _entityMgr;
     @Mock
-    ResourceLimitService _resourceLimitMgr;
+    private ResourceLimitService _resourceLimitMgr;
     @Mock
-    PrimaryDataStoreDao _storagePoolDao;
+    private PrimaryDataStoreDao _storagePoolDao;
     @Mock
-    UsageEventDao _usageEventDao;
+    private UsageEventDao _usageEventDao;
     @Mock
-    VMSnapshotDao _vmSnapshotDao;
+    private VMSnapshotDao _vmSnapshotDao;
     @Mock
-    UpdateVmNicIpCmd _updateVmNicIpCmd;
+    private UpdateVmNicIpCmd _updateVmNicIpCmd;
     @Mock
-    NicDao _nicDao;
+    private NicDao _nicDao;
     @Mock
-    VlanDao _vlanDao;
+    private VlanDao _vlanDao;
     @Mock
-    NicVO _nicMock;
+    private NicVO _nicMock;
     @Mock
-    NetworkModel _networkModel;
+    private NetworkModel _networkModel;
     @Mock
-    NetworkDao _networkDao;
+    private NetworkDao _networkDao;
     @Mock
-    NetworkVO _networkMock;
+    private NetworkVO _networkMock;
     @Mock
-    DataCenterDao _dcDao;
+    private DataCenterDao _dcDao;
     @Mock
-    DataCenterVO _dcMock;
+    private DataCenterVO _dcMock;
     @Mock
-    IpAddressManager _ipAddrMgr;
+    private IpAddressManager _ipAddrMgr;
     @Mock
-    IPAddressDao _ipAddressDao;
+    private IPAddressDao _ipAddressDao;
     @Mock
-    NetworkOfferingDao _networkOfferingDao;
+    private NetworkOfferingDao _networkOfferingDao;
     @Mock
-    NetworkOfferingVO _networkOfferingMock;
+    private NetworkOfferingVO _networkOfferingMock;
     @Mock
-    NetworkOrchestrationService _networkMgr;
+    private NetworkOrchestrationService _networkMgr;
 
     @Before
     public void setup() {
-        MockitoAnnotations.initMocks(this);
-
-        _userVmMgr._vmDao = _vmDao;
-        _userVmMgr._vmInstanceDao = _vmInstanceDao;
-        _userVmMgr._templateDao = _templateDao;
-        _userVmMgr._templateStoreDao = _templateStoreDao;
-        _userVmMgr._volsDao = _volsDao;
-        _userVmMgr._usageEventDao = _usageEventDao;
-        _userVmMgr._itMgr = _itMgr;
-        _userVmMgr.volumeMgr = _storageMgr;
-        _userVmMgr._accountDao = _accountDao;
-        _userVmMgr._accountService = _accountService;
-        _userVmMgr._userDao = _userDao;
-        _userVmMgr._accountMgr = _accountMgr;
-        _userVmMgr._configMgr = _configMgr;
-        _userVmMgr._offeringDao = _offeringDao;
-        _userVmMgr._capacityMgr = _capacityMgr;
-        _userVmMgr._resourceLimitMgr = _resourceLimitMgr;
-        _userVmMgr._scaleRetry = 2;
-        _userVmMgr._entityMgr = _entityMgr;
-        _userVmMgr._storagePoolDao = _storagePoolDao;
-        _userVmMgr._vmSnapshotDao = _vmSnapshotDao;
-        _userVmMgr._configDao = _configDao;
-        _userVmMgr._nicDao = _nicDao;
-        _userVmMgr._vlanDao = _vlanDao;
-        _userVmMgr._networkModel = _networkModel;
-        _userVmMgr._networkDao = _networkDao;
-        _userVmMgr._dcDao = _dcDao;
-        _userVmMgr._ipAddrMgr = _ipAddrMgr;
-        _userVmMgr._ipAddressDao = _ipAddressDao;
-        _userVmMgr._networkOfferingDao = _networkOfferingDao;
-        _userVmMgr._networkMgr = _networkMgr;
-
         doReturn(3L).when(_account).getId();
         doReturn(8L).when(_vmMock).getAccountId();
         when(_accountDao.findById(anyLong())).thenReturn(_accountMock);
@@ -267,43 +238,40 @@ public void setup() {
         when(_vmMock.getId()).thenReturn(314L);
         when(_vmInstance.getId()).thenReturn(1L);
         when(_vmInstance.getServiceOfferingId()).thenReturn(2L);
-        List<VMSnapshotVO> mockList = mock(List.class);
+
+        List<VMSnapshotVO> mockList = new ArrayList<>();
         when(_vmSnapshotDao.findByVm(anyLong())).thenReturn(mockList);
-        when(mockList.size()).thenReturn(0);
-        
when(_templateStoreDao.findByTemplateZoneReady(anyLong(),anyLong())).thenReturn(_templateDataStoreMock);
+        when(_templateStoreDao.findByTemplateZoneReady(anyLong(), 
anyLong())).thenReturn(_templateDataStoreMock);
 
     }
 
-
     @Test
-    public void testValidateRootDiskResize()
-    {
+    public void testValidateRootDiskResize() {
         HypervisorType hypervisorType = HypervisorType.Any;
         Long rootDiskSize = Long.valueOf(10);
-        UserVmVO  vm = Mockito.mock(UserVmVO.class);
+        UserVmVO vm = Mockito.mock(UserVmVO.class);
         VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
         Map<String, String> customParameters = new HashMap<String, String>();
         Map<String, String> vmDetals = new HashMap<String, String>();
 
-
-        vmDetals.put("rootDiskController","ide");
+        vmDetals.put("rootDiskController", "ide");
         when(vm.getDetails()).thenReturn(vmDetals);
-        when(templateVO.getSize()).thenReturn((rootDiskSize<<30)+1);
+        when(templateVO.getSize()).thenReturn((rootDiskSize << 30) + 1);
         //Case 1: >
-        try{
+        try {
             _userVmMgr.validateRootDiskResize(hypervisorType, rootDiskSize, 
templateVO, vm, customParameters);
             Assert.fail("Function should throw 
InvalidParameterValueException");
-        }catch(Exception e){
+        } catch (Exception e) {
             assertThat(e, instanceOf(InvalidParameterValueException.class));
         }
 
         //Case 2: =
-        when(templateVO.getSize()).thenReturn((rootDiskSize<<30));
-        customParameters.put("rootdisksize","10");
+        when(templateVO.getSize()).thenReturn((rootDiskSize << 30));
+        customParameters.put("rootdisksize", "10");
         _userVmMgr.validateRootDiskResize(hypervisorType, rootDiskSize, 
templateVO, vm, customParameters);
-        assert(!customParameters.containsKey("rootdisksize"));
+        assert (!customParameters.containsKey("rootdisksize"));
 
-        when(templateVO.getSize()).thenReturn((rootDiskSize<<30)-1);
+        when(templateVO.getSize()).thenReturn((rootDiskSize << 30) - 1);
 
         //Case 3:  <
 
@@ -315,12 +283,12 @@ public void testValidateRootDiskResize()
         try {
             _userVmMgr.validateRootDiskResize(hypervisorType, rootDiskSize, 
templateVO, vm, customParameters);
             Assert.fail("Function should throw 
InvalidParameterValueException");
-        }catch(Exception e) {
+        } catch (Exception e) {
             assertThat(e, instanceOf(InvalidParameterValueException.class));
         }
 
         //Case 3.3:   1->(rootDiskController==scsi)
-        vmDetals.put("rootDiskController","scsi");
+        vmDetals.put("rootDiskController", "scsi");
         _userVmMgr.validateRootDiskResize(hypervisorType, rootDiskSize, 
templateVO, vm, customParameters);
     }
 
@@ -344,8 +312,7 @@ public void testRestoreVMF1() throws 
ResourceAllocationException, InsufficientCa
 
     // Test restoreVm when VM is in stopped state
     @Test
-    public void testRestoreVMF2() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
-        ResourceAllocationException {
+    public void testRestoreVMF2() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, 
ConcurrentOperationException, ResourceAllocationException {
 
         doReturn(VirtualMachine.State.Stopped).when(_vmMock).getState();
         when(_vmDao.findById(anyLong())).thenReturn(_vmMock);
@@ -381,8 +348,7 @@ public void testRestoreVMF2() throws 
ResourceUnavailableException, InsufficientC
 
     // Test restoreVM when VM is in running state
     @Test
-    public void testRestoreVMF3() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
-        ResourceAllocationException {
+    public void testRestoreVMF3() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, 
ConcurrentOperationException, ResourceAllocationException {
 
         doReturn(VirtualMachine.State.Running).when(_vmMock).getState();
         when(_vmDao.findById(anyLong())).thenReturn(_vmMock);
@@ -418,8 +384,7 @@ public void testRestoreVMF3() throws 
ResourceUnavailableException, InsufficientC
 
     // Test restoreVM on providing new template Id, when VM is in running state
     @Test
-    public void testRestoreVMF4() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
-        ResourceAllocationException {
+    public void testRestoreVMF4() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, 
ConcurrentOperationException, ResourceAllocationException {
         doReturn(VirtualMachine.State.Running).when(_vmMock).getState();
         when(_vmDao.findById(anyLong())).thenReturn(_vmMock);
         when(_volsDao.findByInstanceAndType(314L, 
Volume.Type.ROOT)).thenReturn(_rootVols);
@@ -438,9 +403,9 @@ public void testRestoreVMF4() throws 
ResourceUnavailableException, InsufficientC
         doNothing().when(_volsDao).attachVolume(anyLong(), anyLong(), 
anyLong());
         when(_volumeMock.getId()).thenReturn(3L);
         doNothing().when(_volsDao).detachVolume(anyLong());
-        List<VMSnapshotVO> mockList = mock(List.class);
+
+        List<VMSnapshotVO> mockList = new ArrayList<>();
         when(_vmSnapshotDao.findByVm(anyLong())).thenReturn(mockList);
-        when(mockList.size()).thenReturn(0);
         
when(_templateMock.getUuid()).thenReturn("b1a3626e-72e0-4697-8c7c-a110940cc55d");
 
         Account account = new AccountVO("testaccount", 1L, "networkdomain", 
(short)0, "uuid");
@@ -458,13 +423,12 @@ public void testRestoreVMF4() throws 
ResourceUnavailableException, InsufficientC
         } finally {
             CallContext.unregister();
         }
-
+        verify(_vmMock, times(0)).setIsoId(Mockito.anyLong());
     }
 
     // Test restoreVM on providing new ISO Id, when VM(deployed using ISO) is 
in running state
     @Test
-    public void testRestoreVMF5() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
-        ResourceAllocationException {
+    public void testRestoreVMF5() throws ResourceUnavailableException, 
InsufficientCapacityException, ServerApiException, 
ConcurrentOperationException, ResourceAllocationException {
         doReturn(VirtualMachine.State.Running).when(_vmMock).getState();
         when(_vmDao.findById(anyLong())).thenReturn(_vmMock);
         when(_volsDao.findByInstanceAndType(314L, 
Volume.Type.ROOT)).thenReturn(_rootVols);
@@ -485,9 +449,9 @@ public void testRestoreVMF5() throws 
ResourceUnavailableException, InsufficientC
         doNothing().when(_volsDao).attachVolume(anyLong(), anyLong(), 
anyLong());
         when(_volumeMock.getId()).thenReturn(3L);
         doNothing().when(_volsDao).detachVolume(anyLong());
-        List<VMSnapshotVO> mockList = mock(List.class);
+        List<VMSnapshotVO> mockList = new ArrayList<>();
         when(_vmSnapshotDao.findByVm(anyLong())).thenReturn(mockList);
-        when(mockList.size()).thenReturn(0);
+
         
when(_templateMock.getUuid()).thenReturn("b1a3626e-72e0-4697-8c7c-a110940cc55d");
 
         Account account = new AccountVO("testaccount", 1L, "networkdomain", 
(short)0, "uuid");
@@ -512,7 +476,7 @@ public void testRestoreVMF5() throws 
ResourceUnavailableException, InsufficientC
 
     // Test scaleVm on incompatible HV.
     @Test(expected = InvalidParameterValueException.class)
-    public void testScaleVMF1()  throws Exception {
+    public void testScaleVMF1() throws Exception {
 
         ScaleVMCmd cmd = new ScaleVMCmd();
         Class<?> _class = cmd.getClass();
@@ -527,7 +491,7 @@ public void testScaleVMF1()  throws Exception {
 
         when(_vmInstanceDao.findById(anyLong())).thenReturn(_vmInstance);
 
-       // UserContext.current().setEventDetails("Vm Id: "+getId());
+        // UserContext.current().setEventDetails("Vm Id: "+getId());
         Account account = new AccountVO("testaccount", 1L, "networkdomain", 
(short)0, "uuid");
         UserVO user = new UserVO(1, "testuser", "password", "firstname", 
"lastName", "email", "timezone", UUID.randomUUID().toString(), 
User.Source.UNKNOWN);
         //AccountVO(String accountName, long domainId, String networkDomain, 
short type, int regionId)
@@ -544,7 +508,7 @@ public void testScaleVMF1()  throws Exception {
 
     // Test scaleVm on equal service offerings.
     @Test(expected = InvalidParameterValueException.class)
-    public void testScaleVMF2()  throws Exception {
+    public void testScaleVMF2() throws Exception {
 
         ScaleVMCmd cmd = new ScaleVMCmd();
         Class<?> _class = cmd.getClass();
@@ -566,9 +530,7 @@ public void testScaleVMF2()  throws Exception {
 
         doNothing().when(_itMgr).checkIfCanUpgrade(_vmMock, _offeringVo);
 
-        ServiceOffering so1 =  getSvcoffering(512);
-        ServiceOffering so2 =  getSvcoffering(256);
-
+        ServiceOffering so1 = getSvcoffering(512);
         
when(_offeringDao.findById(anyLong())).thenReturn((ServiceOfferingVO)so1);
         when(_offeringDao.findByIdIncludingRemoved(anyLong(), 
anyLong())).thenReturn((ServiceOfferingVO)so1);
 
@@ -585,7 +547,7 @@ public void testScaleVMF2()  throws Exception {
 
     // Test scaleVm for Stopped vm.
     //@Test(expected=InvalidParameterValueException.class)
-    public void testScaleVMF3()  throws Exception {
+    public void testScaleVMF3() throws Exception {
 
         ScaleVMCmd cmd = new ScaleVMCmd();
         Class<?> _class = cmd.getClass();
@@ -601,8 +563,8 @@ public void testScaleVMF3()  throws Exception {
         when(_vmInstanceDao.findById(anyLong())).thenReturn(_vmInstance);
         
doReturn(Hypervisor.HypervisorType.XenServer).when(_vmInstance).getHypervisorType();
 
-        ServiceOffering so1 =  getSvcoffering(512);
-        ServiceOffering so2 =  getSvcoffering(256);
+        ServiceOffering so1 = getSvcoffering(512);
+        ServiceOffering so2 = getSvcoffering(256);
 
         when(_entityMgr.findById(eq(ServiceOffering.class), 
anyLong())).thenReturn(so2);
         when(_entityMgr.findById(ServiceOffering.class, 1L)).thenReturn(so1);
@@ -626,7 +588,7 @@ public void testScaleVMF3()  throws Exception {
     }
 
     // Test scaleVm for Running vm. Full positive test.
-    public void testScaleVMF4()  throws Exception {
+    public void testScaleVMF4() throws Exception {
 
         ScaleVMCmd cmd = new ScaleVMCmd();
         Class<?> _class = cmd.getClass();
@@ -647,8 +609,8 @@ public void testScaleVMF4()  throws Exception {
         when(_vmInstanceDao.findById(anyLong())).thenReturn(_vmInstance);
         
doReturn(Hypervisor.HypervisorType.XenServer).when(_vmInstance).getHypervisorType();
 
-        ServiceOffering so1 =  getSvcoffering(512);
-        ServiceOffering so2 =  getSvcoffering(256);
+        ServiceOffering so1 = getSvcoffering(512);
+        ServiceOffering so2 = getSvcoffering(256);
 
         when(_entityMgr.findById(eq(ServiceOffering.class), 
anyLong())).thenReturn(so2);
         when(_entityMgr.findById(ServiceOffering.class, 1L)).thenReturn(so1);
@@ -656,7 +618,7 @@ public void testScaleVMF4()  throws Exception {
         doReturn(VirtualMachine.State.Running).when(_vmInstance).getState();
 
         //when(ApiDBUtils.getCpuOverprovisioningFactor()).thenReturn(3f);
-        when(_capacityMgr.checkIfHostHasCapacity(anyLong(), anyInt(), 
anyLong(), anyBoolean(), anyFloat(), anyFloat(),  
anyBoolean())).thenReturn(false);
+        when(_capacityMgr.checkIfHostHasCapacity(anyLong(), anyInt(), 
anyLong(), anyBoolean(), anyFloat(), anyFloat(), 
anyBoolean())).thenReturn(false);
         when(_itMgr.reConfigureVm(_vmInstance.getUuid(), so1, 
false)).thenReturn(_vmInstance);
 
         doReturn(true).when(_itMgr).upgradeVmDb(anyLong(), anyLong());
@@ -675,8 +637,6 @@ public void testScaleVMF4()  throws Exception {
     }
 
     private ServiceOfferingVO getSvcoffering(int ramSize) {
-
-        long id  = 4L;
         String name = "name";
         String displayText = "displayText";
         int cpu = 1;
@@ -686,15 +646,14 @@ private ServiceOfferingVO getSvcoffering(int ramSize) {
         boolean ha = false;
         boolean useLocalStorage = false;
 
-        ServiceOfferingVO serviceOffering =
-            new ServiceOfferingVO(name, cpu, ramSize, speed, null, null, ha, 
displayText, Storage.ProvisioningType.THIN,
-                    useLocalStorage, false, null, false, null, false);
+        ServiceOfferingVO serviceOffering = new ServiceOfferingVO(name, cpu, 
ramSize, speed, null, null, ha, displayText, Storage.ProvisioningType.THIN, 
useLocalStorage, false, null, false, null,
+                false);
         return serviceOffering;
     }
 
     // Test Move VM b/w accounts where caller is not ROOT/Domain admin
     @Test(expected = InvalidParameterValueException.class)
-    public void testMoveVmToUser1()  throws Exception {
+    public void testMoveVmToUser1() throws Exception {
         AssignVMCmd cmd = new AssignVMCmd();
         Class<?> _class = cmd.getClass();
 
@@ -717,7 +676,7 @@ public void testMoveVmToUser1()  throws Exception {
         CallContext.register(user, caller);
         try {
 
-        _userVmMgr.moveVMToUser(cmd);
+            _userVmMgr.moveVMToUser(cmd);
         } finally {
             CallContext.unregister();
         }
@@ -725,7 +684,7 @@ public void testMoveVmToUser1()  throws Exception {
 
     // Test Move VM b/w accounts where caller doesn't have access to the old 
or new account
     @Test(expected = PermissionDeniedException.class)
-    public void testMoveVmToUser2()  throws Exception {
+    public void testMoveVmToUser2() throws Exception {
         AssignVMCmd cmd = new AssignVMCmd();
         Class<?> _class = cmd.getClass();
 
@@ -756,8 +715,7 @@ public void testMoveVmToUser2()  throws Exception {
 
         when(_accountMgr.finalizeOwner(any(Account.class), anyString(), 
anyLong(), anyLong())).thenReturn(newAccount);
 
-        doThrow(new PermissionDeniedException("Access check 
failed")).when(_accountMgr).checkAccess(any(Account.class), 
any(AccessType.class), any(Boolean.class),
-            any(ControlledEntity.class));
+        doThrow(new PermissionDeniedException("Access check 
failed")).when(_accountMgr).checkAccess(any(Account.class), 
any(AccessType.class), any(Boolean.class), any(ControlledEntity.class));
 
         CallContext.register(user, caller);
 
@@ -805,7 +763,8 @@ public void testUpdateVmNicIpSuccess1() throws Exception {
         when(_dcMock.getNetworkType()).thenReturn(NetworkType.Advanced);
 
         when(_ipAddrMgr.allocateGuestIP(Mockito.eq(_networkMock), 
anyString())).thenReturn("10.10.10.10");
-        
doNothing().when(_networkMgr).implementNetworkElementsAndResources(Mockito.any(DeployDestination.class),
 Mockito.any(ReservationContext.class), Mockito.eq(_networkMock), 
Mockito.eq(_networkOfferingMock));
+        
doNothing().when(_networkMgr).implementNetworkElementsAndResources(Mockito.any(DeployDestination.class),
 Mockito.any(ReservationContext.class), Mockito.eq(_networkMock),
+                Mockito.eq(_networkOfferingMock));
         when(_nicDao.persist(any(NicVO.class))).thenReturn(nic);
 
         Account caller = new AccountVO("testaccount", 1, "networkdomain", 
(short)0, UUID.randomUUID().toString());
diff --git a/server/test/com/cloud/vm/UserVmManagerImplTest.java 
b/server/test/com/cloud/vm/UserVmManagerImplTest.java
new file mode 100644
index 00000000000..bec9d7dee6a
--- /dev/null
+++ b/server/test/com/cloud/vm/UserVmManagerImplTest.java
@@ -0,0 +1,224 @@
+// 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 com.cloud.vm;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+
+import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
+import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.BDDMockito;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+import com.cloud.uservm.UserVm;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.dao.UserVmDetailsDao;
+
+@RunWith(PowerMockRunner.class)
+public class UserVmManagerImplTest {
+
+    @Spy
+    @InjectMocks
+    private UserVmManagerImpl userVmManagerImpl = new UserVmManagerImpl();
+
+    @Mock
+    private GuestOSDao guestOSDao;
+
+    @Mock
+    private UserVmDao userVmDao;
+
+    @Mock
+    private UpdateVMCmd updateVmCommand;
+
+    @Mock
+    private AccountManager accountManager;
+
+    @Mock
+    private UserVmDetailsDao userVmDetailVO;
+
+    @Mock
+    private UserVmVO userVmVoMock;
+
+    private long vmId = 1l;
+
+    @Before
+    public void beforeTest() {
+        Mockito.when(updateVmCommand.getId()).thenReturn(vmId);
+
+        
Mockito.when(userVmDao.findById(Mockito.eq(vmId))).thenReturn(userVmVoMock);
+    }
+
+    @Test
+    public void 
validateGuestOsIdForUpdateVirtualMachineCommandTestOsTypeNull() {
+        Mockito.when(updateVmCommand.getOsTypeId()).thenReturn(null);
+        
userVmManagerImpl.validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void 
validateGuestOsIdForUpdateVirtualMachineCommandTestOsTypeNotFound() {
+        Mockito.when(updateVmCommand.getOsTypeId()).thenReturn(1l);
+
+        
userVmManagerImpl.validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
+    }
+
+    @Test
+    public void 
validateGuestOsIdForUpdateVirtualMachineCommandTestOsTypeFound() {
+        Mockito.when(updateVmCommand.getOsTypeId()).thenReturn(1l);
+        
Mockito.when(guestOSDao.findById(Mockito.eq(1l))).thenReturn(Mockito.mock(GuestOSVO.class));
+
+        
userVmManagerImpl.validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void 
validateInputsAndPermissionForUpdateVirtualMachineCommandTestVmNotFound() {
+        Mockito.when(userVmDao.findById(Mockito.eq(vmId))).thenReturn(null);
+
+        
userVmManagerImpl.validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand);
+    }
+
+    @Test
+    @PrepareForTest(CallContext.class)
+    public void 
validateInputsAndPermissionForUpdateVirtualMachineCommandTest() {
+        
Mockito.doNothing().when(userVmManagerImpl).validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
+
+        Account accountMock = Mockito.mock(Account.class);
+        CallContext callContextMock = Mockito.mock(CallContext.class);
+
+        PowerMockito.mockStatic(CallContext.class);
+        BDDMockito.given(CallContext.current()).willReturn(callContextMock);
+        
Mockito.when(callContextMock.getCallingAccount()).thenReturn(accountMock);
+
+        Mockito.doNothing().when(accountManager).checkAccess(accountMock, 
null, true, userVmVoMock);
+        
userVmManagerImpl.validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand);
+
+        
Mockito.verify(userVmManagerImpl).validateGuestOsIdForUpdateVirtualMachineCommand(updateVmCommand);
+        Mockito.verify(accountManager).checkAccess(accountMock, null, true, 
userVmVoMock);
+    }
+
+    @Test
+    public void updateVirtualMachineTestDisplayChanged() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        configureDoNothingForMethodsThatWeDoNotWantToTest();
+
+        Mockito.when(userVmVoMock.isDisplay()).thenReturn(true);
+        Mockito.doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, 
vmId, userVmVoMock);
+
+        userVmManagerImpl.updateVirtualMachine(updateVmCommand);
+        verifyMethodsThatAreAlwaysExecuted();
+
+        Mockito.verify(userVmManagerImpl).updateDisplayVmFlag(false, vmId, 
userVmVoMock);
+        Mockito.verify(userVmDetailVO, Mockito.times(0)).removeDetails(vmId);
+    }
+
+    @Test
+    public void updateVirtualMachineTestCleanUpTrue() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        configureDoNothingForMethodsThatWeDoNotWantToTest();
+
+        Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(true);
+
+        Mockito.doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, 
vmId, userVmVoMock);
+        Mockito.doNothing().when(userVmDetailVO).removeDetails(vmId);
+
+        userVmManagerImpl.updateVirtualMachine(updateVmCommand);
+        verifyMethodsThatAreAlwaysExecuted();
+        Mockito.verify(userVmDetailVO).removeDetails(vmId);
+        Mockito.verify(userVmManagerImpl, 
Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
+    }
+
+    @Test
+    public void updateVirtualMachineTestCleanUpTrueAndDetailEmpty() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        prepareAndExecuteMethodDealingWithDetails(true, true);
+    }
+
+    @Test
+    public void updateVirtualMachineTestCleanUpTrueAndDetailsNotEmpty() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        prepareAndExecuteMethodDealingWithDetails(true, false);
+    }
+
+    @Test
+    public void updateVirtualMachineTestCleanUpFalseAndDetailsNotEmpty() 
throws ResourceUnavailableException, InsufficientCapacityException {
+        prepareAndExecuteMethodDealingWithDetails(false, true);
+    }
+
+    @Test
+    public void updateVirtualMachineTestCleanUpFalseAndDetailsEmpty() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        prepareAndExecuteMethodDealingWithDetails(false, false);
+    }
+
+    private void prepareAndExecuteMethodDealingWithDetails(boolean 
cleanUpDetails, boolean isDetailsEmpty) throws ResourceUnavailableException, 
InsufficientCapacityException {
+        configureDoNothingForMethodsThatWeDoNotWantToTest();
+
+        HashMap<String, String> details = new HashMap<>();
+        if(!isDetailsEmpty) {
+            details.put("", "");
+        }
+        Mockito.when(updateVmCommand.getDetails()).thenReturn(details);
+        
Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(cleanUpDetails);
+
+        configureDoNothingForDetailsMethod();
+
+        userVmManagerImpl.updateVirtualMachine(updateVmCommand);
+        verifyMethodsThatAreAlwaysExecuted();
+
+        Mockito.verify(userVmVoMock, Mockito.times(cleanUpDetails || 
isDetailsEmpty ? 0 : 1)).setDetails(details);
+        Mockito.verify(userVmDetailVO, Mockito.times(cleanUpDetails ? 1: 
0)).removeDetails(vmId);
+        Mockito.verify(userVmDao, Mockito.times(cleanUpDetails || 
isDetailsEmpty ? 0 : 1)).saveDetails(userVmVoMock);
+        Mockito.verify(userVmManagerImpl, 
Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
+    }
+
+    private void configureDoNothingForDetailsMethod() {
+        Mockito.doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, 
vmId, userVmVoMock);
+        Mockito.doNothing().when(userVmDetailVO).removeDetails(vmId);
+        Mockito.doNothing().when(userVmDao).saveDetails(userVmVoMock);
+    }
+
+    @SuppressWarnings("unchecked")
+    private void verifyMethodsThatAreAlwaysExecuted() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        
Mockito.verify(userVmManagerImpl).validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand);
+        
Mockito.verify(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand);
+        
Mockito.verify(userVmManagerImpl).updateVirtualMachine(Mockito.anyLong(), 
Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), 
Mockito.anyBoolean(), Mockito.anyLong(),
+                Mockito.anyString(), Mockito.anyBoolean(), 
Mockito.any(HTTPMethod.class), Mockito.anyString(), Mockito.anyString(), 
Mockito.anyString(), Mockito.anyListOf(Long.class),
+                Mockito.anyMap());
+
+    }
+
+    @SuppressWarnings("unchecked")
+    private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws 
ResourceUnavailableException, InsufficientCapacityException {
+        
Mockito.doNothing().when(userVmManagerImpl).validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand);
+        Mockito.doReturn(new 
ArrayList<Long>()).when(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand);
+        
Mockito.doReturn(Mockito.mock(UserVm.class)).when(userVmManagerImpl).updateVirtualMachine(Mockito.anyLong(),
 Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(),
+                Mockito.anyBoolean(), Mockito.anyLong(),
+                Mockito.anyString(), Mockito.anyBoolean(), 
Mockito.any(HTTPMethod.class), Mockito.anyString(), Mockito.anyString(), 
Mockito.anyString(), Mockito.anyListOf(Long.class),
+                Mockito.anyMap());
+    }
+}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> User is able to change to “Guest OS type” that has been removed 
> ----------------------------------------------------------------
>
>                 Key: CLOUDSTACK-10230
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10230
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>            Reporter: Rafael Weingärtner
>            Assignee: Rafael Weingärtner
>            Priority: Critical
>
> Users are able to change the OS type of VMs to “Guest OS type” that has been 
> removed. This becomes a security issue when we try to force users to use HVM 
> VMs (Meltdown/Spectre thing). A removed “guest os type” should not be usable 
> by any users in the cloud.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to