Implement the correct semantics of returning a VM instance ID in deploy vm async-call

core/src/com/cloud/vm/UserVmVO.java

@@ -144,7 +144,7 @@ public class UserVmVO extends VMInstanceVO implements UserVm {
                     String externalMacAddress,
                     Long vlanDbId,
                     Long routerId,
-                    long podId,
+                    Long podId,
                     long dcId,
                     boolean haEnabled,
                     String displayName,

core/src/com/cloud/vm/VMInstanceVO.java

@@ -99,8 +99,8 @@ public class VMInstanceVO implements VirtualMachine {
     @Column(name="last_host_id", updatable=true, nullable=true)
     private Long lastHostId;
 
-    @Column(name="pod_id", updatable=true, nullable=false)
+    @Column(name="pod_id", updatable=true, nullable=true)
-    private long podId;
+    private Long podId;
 
     @Column(name="private_mac_address", updatable=true, nullable=true)
     String privateMacAddress;
@@ -143,7 +143,7 @@ public class VMInstanceVO implements VirtualMachine {
                         String privateIpAddress,
                         String privateNetmask,
                         long dataCenterId,
-                        long podId,
+                        Long podId,
                         boolean haEnabled,
                         Long hostId,
                         String displayName,
@@ -162,7 +162,7 @@ public class VMInstanceVO implements VirtualMachine {
                         String privateIpAddress,
                         String privateNetmask,
                         long dataCenterId,
-                        long podId,
+                        Long podId,
                         boolean haEnabled,
                         Long hostId,
                         String displayName,
@@ -363,7 +363,7 @@ public class VMInstanceVO implements VirtualMachine {
     }
 
     @Override
-    public long getPodId() {
+    public Long getPodId() {
         return podId;
     }
     

core/src/com/cloud/vm/VirtualMachine.java

@@ -107,7 +107,7 @@ public interface VirtualMachine {
     /**
      * @return pod id.
      */
-    public long getPodId();
+    public Long getPodId();
     
     /**
      * @return data center id.

server/src/com/cloud/server/ManagementServerImpl.java

@@ -264,6 +264,7 @@ import com.cloud.vm.UserVmManager;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachineName;
 import com.cloud.vm.VmStats;
 import com.cloud.vm.dao.ConsoleProxyDao;
 import com.cloud.vm.dao.DomainRouterDao;
@@ -356,6 +357,7 @@ public class ManagementServerImpl implements ManagementServer {
     private final int _routerRamSize;
     private final int _proxyRamSize;
     private final int _ssRamSize;
+    private String _instance;
 
     private final int _maxVolumeSizeInGb;
     private final Map<String, Boolean> _availableIdsMap;
@@ -492,6 +494,11 @@ public class ManagementServerImpl implements ManagementServer {
  		
 		String hypervisorType = _configDao.getValue("hypervisor.type");
         _isHypervisorSnapshotCapable  = hypervisorType.equals(Hypervisor.Type.XenServer.name());
+        
+        _instance = _configs.get("instance.name");
+        if (_instance == null) {
+            _instance = "DEFAULT";
+        }
     }
 
     protected Map<String, String> getConfigs() {
@@ -2237,6 +2244,14 @@ public class ManagementServerImpl implements ManagementServer {
         if (password == null || password.equals("") || (!validPassword(password))) {
             throw new InvalidParameterValueException("A valid password for this virtual machine was not provided.");
         }
+        
+        long guestOSId;
+        if (template != null) {
+        	guestOSId = template.getGuestOSId();
+        } else {
+        	throw new InternalErrorException("No template or ISO was specified for the VM.");
+        }
+        
         List<NetworkGroupVO> networkGroupVOs = new ArrayList<NetworkGroupVO>();
         if (networkGroups != null) {
         	for (String groupName: networkGroups) {
@@ -2253,32 +2268,43 @@ public class ManagementServerImpl implements ManagementServer {
             stats = new UserStatisticsVO(account.getId(), dataCenterId);
             _userStatsDao.persist(stats);
         }
-        
-    	Long vmId = _vmDao.getNextInSequence(Long.class, "id");
-    	
-        // check if we are within context of async-execution
-        AsyncJobExecutor asyncExecutor = BaseAsyncJobExecutor.getCurrentExecutor();
-        if (asyncExecutor != null) {
-            AsyncJobVO job = asyncExecutor.getJob();
-
-            if (s_logger.isInfoEnabled())
-                s_logger.info("DeployVM acquired a new instance " + vmId + ", update async job-" + job.getId() + " progress status");
-
-            _asyncMgr.updateAsyncJobAttachment(job.getId(), "vm_instance", vmId);
-            _asyncMgr.updateAsyncJobStatus(job.getId(), BaseCmd.PROGRESS_INSTANCE_CREATED, vmId);
-        }
 
         HashMap<Long, StoragePoolVO> avoids = new HashMap<Long, StoragePoolVO>();
 
-        // Pod allocator now allocate VM based on a reservation style allocation, disable retry here for now
+        // Pod allocator now allocate VM based on a reservation style allocation, disable retry here
         for (int retry = 0; retry < 1; retry++) {
             String externalIp = null;
             UserVmVO created = null;
+            
+        	Long vmId = _vmDao.getNextInSequence(Long.class, "id");
+            String name = VirtualMachineName.getVmName(vmId, accountId, _instance);
+        	
+            // router id and router mask 
+            UserVmVO initial = new UserVmVO(vmId, name, template.getId(), guestOSId, accountId, account.getDomainId().longValue(),
+            		serviceOfferingId, null, null, null,
+            		null,null,null,
+            		null, null, dataCenterId,
+            		offering.getOfferHA(), displayName, group, userData);
+            if (diskOffering != null) {
+            	initial.setMirroredVols(diskOffering.isMirrored());
+            }
+            initial = _vmDao.persist(initial);
+            
+            AsyncJobExecutor asyncExecutor = BaseAsyncJobExecutor.getCurrentExecutor();
+            if (asyncExecutor != null) {
+                AsyncJobVO job = asyncExecutor.getJob();
+
+                if (s_logger.isInfoEnabled())
+                    s_logger.info("DeployVM acquired a new instance " + vmId + ", update async job-" + job.getId() + " progress status");
+
+                _asyncMgr.updateAsyncJobAttachment(job.getId(), "vm_instance", vmId);
+                _asyncMgr.updateAsyncJobStatus(job.getId(), BaseCmd.PROGRESS_INSTANCE_CREATED, vmId);
+            }
 
             ArrayList<StoragePoolVO> a = new ArrayList<StoragePoolVO>(avoids.values());
             if (_directAttachNetworkExternalIpAllocator) {
             	try {
-            		created = _vmMgr.createDirectlyAttachedVMExternal(vmId, userId, account, dc, offering, template, diskOffering, displayName, group, userData, a, networkGroupVOs, startEventId);
+            		created = _vmMgr.createDirectlyAttachedVMExternal(initial, userId, account, dc, offering, template, diskOffering, displayName, group, userData, a, networkGroupVOs, startEventId);
             	} catch (ResourceAllocationException rae) {
             		throw rae;
             	}
@@ -2299,7 +2325,7 @@ public class ManagementServerImpl implements ManagementServer {
             		}
 
             		try {
-            			created = _vmMgr.createVirtualMachine(vmId, userId, account, dc, offering, template, diskOffering, displayName, group, userData, a, startEventId);
+            			created = _vmMgr.createVirtualMachine(initial, userId, account, dc, offering, template, diskOffering, displayName, group, userData, a, startEventId);
             		} catch (ResourceAllocationException rae) {
             			throw rae;
             		} catch (InternalErrorException iee){
@@ -2307,7 +2333,7 @@ public class ManagementServerImpl implements ManagementServer {
             		}
             	} else {
             		try {
-            			created = _vmMgr.createDirectlyAttachedVM(vmId, userId, account, dc, offering, template, diskOffering, displayName, group, userData, a, networkGroupVOs, startEventId);
+            			created = _vmMgr.createDirectlyAttachedVM(initial, userId, account, dc, offering, template, diskOffering, displayName, group, userData, a, networkGroupVOs, startEventId);
             		} catch (ResourceAllocationException rae) {
             			throw rae;
             		}
@@ -2375,10 +2401,16 @@ public class ManagementServerImpl implements ManagementServer {
 
             }
             if (started == null) {
-                List<Pair<VolumeVO, StoragePoolVO>> disks = _storageMgr.isStoredOn(created);
+                List<Pair<VolumeVO, StoragePoolVO>> disks = new ArrayList<Pair<VolumeVO, StoragePoolVO>>();
                 // NOTE: We now destroy a VM if the deploy process fails at any step. We now
                 // have a lazy delete so there is still some time to figure out what's wrong.
-                _vmMgr.destroyVirtualMachine(userId, created.getId());
+                if(created != null) {
+                	disks = _storageMgr.isStoredOn(created);
+                	_vmMgr.destroyVirtualMachine(userId, created.getId());
+                } else {
+                	if(initial != null)
+                		_vmMgr.destroyVirtualMachine(userId, initial.getId());
+                }
 
                 boolean retryCreate = true;
                 for (Pair<VolumeVO, StoragePoolVO> disk : disks) {

server/src/com/cloud/vm/UserVmManager.java

@@ -77,11 +77,11 @@ public interface UserVmManager extends Manager, VirtualMachineManager<UserVmVO>
      * @param diskOffering the disk offering for the root disk (deploying from ISO) or the data disk (deploying from a normal template)
      * @return UserVmVO if created; null if not.
      */
-    UserVmVO createVirtualMachine(Long vmId, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> avoids, long startEventId) throws InsufficientStorageCapacityException, InternalErrorException, ResourceAllocationException;
+    UserVmVO createVirtualMachine(UserVmVO vm, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> avoids, long startEventId) throws InsufficientStorageCapacityException, InternalErrorException, ResourceAllocationException;
     
-	UserVmVO createDirectlyAttachedVM(Long vmId, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO> networkGroupVO, long startEventId) throws InternalErrorException, ResourceAllocationException;
+	UserVmVO createDirectlyAttachedVM(UserVmVO vm, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO> networkGroupVO, long startEventId) throws InternalErrorException, ResourceAllocationException;
 
-	UserVmVO createDirectlyAttachedVMExternal(Long vmId, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO> networkGroupVO, long startEventId) throws InternalErrorException, ResourceAllocationException;
+	UserVmVO createDirectlyAttachedVMExternal(UserVmVO vm, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO> networkGroupVO, long startEventId) throws InternalErrorException, ResourceAllocationException;
 
     /**
      * Destroys one virtual machine

server/src/com/cloud/vm/UserVmManagerImpl.java

@@ -1188,11 +1188,10 @@ public class UserVmManagerImpl implements UserVmManager {
     }
 
     @Override @DB
-    public UserVmVO createVirtualMachine(Long vmId, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> avoids, long startEventId) throws InternalErrorException, ResourceAllocationException {
+    public UserVmVO createVirtualMachine(UserVmVO vm, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> avoids, long startEventId) throws InternalErrorException, ResourceAllocationException {
         long accountId = account.getId();
         long dataCenterId = dc.getId();
         long serviceOfferingId = offering.getId();
-        UserVmVO vm = null;
         
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("Creating vm for account id=" + account.getId() +
@@ -1238,7 +1237,8 @@ public class UserVmManagerImpl implements UserVmManager {
         _accountMgr.incrementResourceCount(account.getId(), ResourceType.user_vm);
         _accountMgr.incrementResourceCount(account.getId(), ResourceType.volume, numVolumes);
         txn.commit();
-        
+
+    	Long vmId = vm.getId();
         name = VirtualMachineName.getVmName(vmId, accountId, _instance);
 
         String diskOfferingIdentifier = (diskOffering != null) ? String.valueOf(diskOffering.getId()) : "-1";
@@ -1257,25 +1257,12 @@ public class UserVmManagerImpl implements UserVmManager {
             Set<Long> podsToAvoid = new HashSet<Long>();
 
             while ((pod = _agentMgr.findPod(template, offering, dc, account.getId(), podsToAvoid)) != null) {
-                if (vm == null) {
+            	vm.setDomainRouterId(routerId);
-                    vm = new UserVmVO(vmId, name, template.getId(), guestOSId, accountId, account.getDomainId().longValue(),
+            	vm.setGuestNetmask(router.getGuestNetmask());
-                    		serviceOfferingId, null, null, router.getGuestNetmask(),
+            	vm.setPodId(pod.first().getId());
-                    		null,null,null,
+            	vm.setLastHostId(pod.second());
-                    		routerId, pod.first().getId(), dataCenterId,
+                _vmDao.update(vm.getId(), vm);
-                    		offering.getOfferHA(), displayName, group, userData);
+            	
-                    
-                    if (diskOffering != null) {
-                    	vm.setMirroredVols(diskOffering.isMirrored());
-                    }
-
-                    vm.setLastHostId(pod.second());
-                    
-                    vm = _vmDao.persist(vm);
-                } else {
-                    vm.setPodId(pod.first().getId());
-                    _vmDao.updateIf(vm, Event.OperationRetry, null);
-                }
-                
                 String ipAddressStr = acquireGuestIpAddress(dataCenterId, accountId, vm);
                 if (ipAddressStr == null) {
                 	s_logger.warn("Failed user vm creation : no guest ip address available");
@@ -2347,8 +2334,7 @@ public class UserVmManagerImpl implements UserVmManager {
     
     @DB
     @Override
-	public UserVmVO createDirectlyAttachedVM(Long vmId, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO>  networkGroups, long startEventId) throws InternalErrorException, ResourceAllocationException {
+	public UserVmVO createDirectlyAttachedVM(UserVmVO vm, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO>  networkGroups, long startEventId) throws InternalErrorException, ResourceAllocationException {
-    	
     	long accountId = account.getId();
 	    long dataCenterId = dc.getId();
 	    long serviceOfferingId = offering.getId();
@@ -2392,8 +2378,9 @@ public class UserVmManagerImpl implements UserVmManager {
         txn.commit();
         
 	    try {
-	        UserVmVO vm = null;
 	    	boolean forZone = false;
+	    	
+	    	Long vmId = vm.getId();
 	    	final String name = VirtualMachineName.getVmName(vmId, accountId, _instance);
 	
 	        final String[] macAddresses = _dcDao.getNextAvailableMacAddressPair(dc.getId());
@@ -2524,7 +2511,8 @@ public class UserVmManagerImpl implements UserVmManager {
                 String guestMacAddress = macAddresses[0];
                 String externalMacAddress = macAddresses[1];
                 Long externalVlanDbId = null;
-            
+
+/*                
 	            vm = new UserVmVO(vmId, name, templateId, guestOSId, accountId, account.getDomainId().longValue(),
 	            		serviceOfferingId, guestMacAddress, guestIp, guestVlan.getVlanNetmask(),
 	            		null, externalMacAddress, externalVlanDbId,
@@ -2537,6 +2525,17 @@ public class UserVmManagerImpl implements UserVmManager {
 	
 	            vm.setLastHostId(pod.second());
 	            vm = _vmDao.persist(vm);
+*/
+                vm.setDomainRouterId(routerId);
+                vm.setGuestNetmask(guestVlan.getVlanNetmask());
+                vm.setGuestIpAddress(guestIp);
+                vm.setGuestMacAddress(guestMacAddress);
+                vm.setPodId(pod.first().getId());
+                vm.setLastHostId(pod.second());
+                vm.setExternalMacAddress(externalMacAddress);
+                vm.setExternalVlanDbId(externalVlanDbId);
+                _vmDao.update(vm.getId(), vm);
+	            
 	            boolean addedToGroups = _networkGroupManager.addInstanceToGroups(vmId, networkGroups);
 	            if (!addedToGroups) {
 	            	s_logger.warn("Not all specified network groups can be found");
@@ -2626,7 +2625,7 @@ public class UserVmManagerImpl implements UserVmManager {
     
     @DB
     @Override
-	public UserVmVO createDirectlyAttachedVMExternal(Long vmId, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO>  networkGroups, long startEventId) throws InternalErrorException, ResourceAllocationException {
+	public UserVmVO createDirectlyAttachedVMExternal(UserVmVO vm, long userId, AccountVO account, DataCenterVO dc, ServiceOfferingVO offering, VMTemplateVO template, DiskOfferingVO diskOffering, String displayName, String group, String userData, List<StoragePoolVO> a, List<NetworkGroupVO>  networkGroups, long startEventId) throws InternalErrorException, ResourceAllocationException {
 	    long accountId = account.getId();
 	    long dataCenterId = dc.getId();
 	    long serviceOfferingId = offering.getId();
@@ -2654,7 +2653,8 @@ public class UserVmManagerImpl implements UserVmManager {
         
 	    Transaction txn = Transaction.currentTxn();
 	    try {
-	        UserVmVO vm = null;
+	    	Long vmId = vm.getId();
+	        
 	        txn.start();
 	        
 	    	account = _accountDao.lock(accountId, true);
@@ -2693,6 +2693,8 @@ public class UserVmManagerImpl implements UserVmManager {
                 	publicIpAddr = publicIp.ipaddr;
                 	publicIpNetMask = publicIp.netMask;
                 }
+                
+                /*
 	            vm = new UserVmVO(vmId, name, templateId, guestOSId, accountId, account.getDomainId().longValue(),
 	            		serviceOfferingId, guestMacAddress, publicIpAddr, publicIpNetMask,
 	            		null, externalMacAddress, null,
@@ -2705,6 +2707,18 @@ public class UserVmManagerImpl implements UserVmManager {
 	
 	            vm.setLastHostId(pod.second());
 	            _vmDao.persist(vm);
+	            */
+                
+                vm.setDomainRouterId(routerId);
+                vm.setGuestMacAddress(guestMacAddress);
+                vm.setGuestIpAddress(publicIpAddr);
+                vm.setGuestNetmask(publicIpNetMask);
+                vm.setExternalMacAddress(externalMacAddress);
+                vm.setPodId(pod.first().getId());
+                vm.setLastHostId(pod.second());
+                
+                _vmDao.update(vm.getId(), vm);
+	            
 	            _networkGroupManager.addInstanceToGroups(vmId, networkGroups);
 	            
 	            _accountMgr.incrementResourceCount(account.getId(), ResourceType.user_vm);

setup/db/create-schema.sql

@@ -498,7 +498,7 @@ CREATE TABLE  `cloud`.`vm_instance` (
   `private_mac_address` varchar(17),
   `private_ip_address` varchar(15),
   `private_netmask` varchar(15),
-  `pod_id` bigint unsigned NOT NULL,
+  `pod_id` bigint unsigned,
   `storage_ip` varchar(15),
   `data_center_id` bigint unsigned NOT NULL COMMENT 'Data Center the instance belongs to',
   `host_id` bigint unsigned,

setup/db/schema-213to214.sql

@@ -0,0 +1 @@
+ALTER TABLE `cloud`.`vm_instance` MODIFY COLUMN `pod_id` bigint unsigned;	-- remove NOT NULL constraint to allow creating DB record in early time