Revert "parallel nic adding (#5541)" (#5665)

This reverts commit 3574d8d.

Author: nvazquez

Diff for: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -42,7 +42,6 @@
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
-import javax.persistence.EntityExistsException;
 
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
 import org.apache.cloudstack.annotation.AnnotationService;
@@ -4005,7 +4004,7 @@ public NicProfile addVmToNetwork(final VirtualMachine vm, final Network network,
         if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
             // avoid re-entrance
             VmWorkJobVO placeHolder = null;
-            placeHolder = createPlaceHolderWork(vm.getId(), network.getUuid());
+            placeHolder = createPlaceHolderWork(vm.getId());
             try {
                 return orchestrateAddVmToNetwork(vm, network, requested);
             } finally {
@@ -4045,23 +4044,10 @@ public NicProfile addVmToNetwork(final VirtualMachine vm, final Network network,
         }
     }
 
-    /**
-     * duplicated in {@see UserVmManagerImpl} for a {@see UserVmVO}
-     */
-    private void checkIfNetworkExistsForVM(VirtualMachine virtualMachine, Network network) {
-        List<NicVO> allNics = _nicsDao.listByVmId(virtualMachine.getId());
-        for (NicVO nic : allNics) {
-            if (nic.getNetworkId() == network.getId()) {
-                throw new CloudRuntimeException("A NIC already exists for VM:" + virtualMachine.getInstanceName() + " in network: " + network.getUuid());
-            }
-        }
-    }
-
     private NicProfile orchestrateAddVmToNetwork(final VirtualMachine vm, final Network network, final NicProfile requested) throws ConcurrentOperationException, ResourceUnavailableException,
     InsufficientCapacityException {
         final CallContext cctx = CallContext.current();
 
-        checkIfNetworkExistsForVM(vm, network);
         s_logger.debug("Adding vm " + vm + " to network " + network + "; requested nic profile " + requested);
         final VMInstanceVO vmVO = _vmDao.findById(vm.getId());
         final ReservationContext context = new ReservationContextImpl(null, null, cctx.getCallingUser(), cctx.getCallingAccount());
@@ -5412,7 +5398,7 @@ public Outcome<VirtualMachine> migrateVmThroughJobQueue(final String vmUuid, fin
         Map<Volume, StoragePool> volumeStorageMap = dest.getStorageForDisks();
         if (volumeStorageMap != null) {
             for (Volume vol : volumeStorageMap.keySet()) {
-                checkConcurrentJobsPerDatastoreThreshold(volumeStorageMap.get(vol));
+                checkConcurrentJobsPerDatastoreThreshhold(volumeStorageMap.get(vol));
             }
         }
 
@@ -5577,7 +5563,7 @@ public Outcome<VirtualMachine> migrateVmForScaleThroughJobQueue(
         return new VmJobVirtualMachineOutcome(workJob, vm.getId());
     }
 
-    private void checkConcurrentJobsPerDatastoreThreshold(final StoragePool destPool) {
+    private void checkConcurrentJobsPerDatastoreThreshhold(final StoragePool destPool) {
         final Long threshold = VolumeApiService.ConcurrentMigrationsThresholdPerDatastore.value();
         if (threshold != null && threshold > 0) {
             long count = _jobMgr.countPendingJobs("\"storageid\":\"" + destPool.getUuid() + "\"", MigrateVMCmd.class.getName(), MigrateVolumeCmd.class.getName(), MigrateVolumeCmdByAdmin.class.getName());
@@ -5598,7 +5584,7 @@ public Outcome<VirtualMachine> migrateVmStorageThroughJobQueue(
         Set<Long> uniquePoolIds = new HashSet<>(poolIds);
         for (Long poolId : uniquePoolIds) {
             StoragePoolVO pool = _storagePoolDao.findById(poolId);
-            checkConcurrentJobsPerDatastoreThreshold(pool);
+            checkConcurrentJobsPerDatastoreThreshhold(pool);
         }
 
         final VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
@@ -5645,61 +5631,35 @@ public Outcome<VirtualMachine> addVmToNetworkThroughJobQueue(
 
         final List<VmWorkJobVO> pendingWorkJobs = _workJobDao.listPendingWorkJobs(
                 VirtualMachine.Type.Instance, vm.getId(),
-                VmWorkAddVmToNetwork.class.getName(), network.getUuid());
+                VmWorkAddVmToNetwork.class.getName());
 
         VmWorkJobVO workJob = null;
         if (pendingWorkJobs != null && pendingWorkJobs.size() > 0) {
-            if (pendingWorkJobs.size() > 1) {
-                s_logger.warn(String.format("The number of jobs to add network %s to vm %s are %d", network.getUuid(), vm.getInstanceName(), pendingWorkJobs.size()));
-            }
+            assert pendingWorkJobs.size() == 1;
             workJob = pendingWorkJobs.get(0);
         } else {
-            if (s_logger.isTraceEnabled()) {
-                s_logger.trace(String.format("no jobs to add network %s for vm %s yet", network, vm));
-            }
 
-            workJob = createVmWorkJobToAddNetwork(vm, network, requested, context, user, account);
-        }
-        AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
-
-        return new VmJobVirtualMachineOutcome(workJob, vm.getId());
-    }
-
-    private VmWorkJobVO createVmWorkJobToAddNetwork(
-            VirtualMachine vm,
-            Network network,
-            NicProfile requested,
-            CallContext context,
-            User user,
-            Account account) {
-        VmWorkJobVO workJob;
-        workJob = new VmWorkJobVO(context.getContextId());
+            workJob = new VmWorkJobVO(context.getContextId());
 
-        workJob.setDispatcher(VmWorkConstants.VM_WORK_JOB_DISPATCHER);
-        workJob.setCmd(VmWorkAddVmToNetwork.class.getName());
+            workJob.setDispatcher(VmWorkConstants.VM_WORK_JOB_DISPATCHER);
+            workJob.setCmd(VmWorkAddVmToNetwork.class.getName());
 
-        workJob.setAccountId(account.getId());
-        workJob.setUserId(user.getId());
-        workJob.setVmType(VirtualMachine.Type.Instance);
-        workJob.setVmInstanceId(vm.getId());
-        workJob.setRelated(AsyncJobExecutionContext.getOriginJobId());
-        workJob.setSecondaryObjectIdentifier(network.getUuid());
+            workJob.setAccountId(account.getId());
+            workJob.setUserId(user.getId());
+            workJob.setVmType(VirtualMachine.Type.Instance);
+            workJob.setVmInstanceId(vm.getId());
+            workJob.setRelated(AsyncJobExecutionContext.getOriginJobId());
 
-        // save work context info (there are some duplications)
-        final VmWorkAddVmToNetwork workInfo = new VmWorkAddVmToNetwork(user.getId(), account.getId(), vm.getId(),
-                VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER, network.getId(), requested);
-        workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
+            // save work context info (there are some duplications)
+            final VmWorkAddVmToNetwork workInfo = new VmWorkAddVmToNetwork(user.getId(), account.getId(), vm.getId(),
+                    VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER, network.getId(), requested);
+            workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
 
-        try {
             _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId());
-        } catch (CloudRuntimeException e) {
-            if (e.getCause() instanceof EntityExistsException) {
-                String msg = String.format("A job to add a nic for network %s to vm %s already exists", network.getUuid(), vm.getUuid());
-                s_logger.warn(msg, e);
-            }
-            throw e;
         }
-        return workJob;
+        AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
+
+        return new VmJobVirtualMachineOutcome(workJob, vm.getId());
     }
 
     public Outcome<VirtualMachine> removeNicFromVmThroughJobQueue(
@@ -6008,10 +5968,6 @@ public Pair<JobInfo.Status, String> handleVmWorkJob(final VmWork work) throws Ex
     }
 
     private VmWorkJobVO createPlaceHolderWork(final long instanceId) {
-        return createPlaceHolderWork(instanceId, null);
-    }
-
-    private VmWorkJobVO createPlaceHolderWork(final long instanceId, String secondaryObjectIdentifier) {
         final VmWorkJobVO workJob = new VmWorkJobVO("");
 
         workJob.setDispatcher(VmWorkConstants.VM_WORK_JOB_PLACEHOLDER);
@@ -6023,9 +5979,6 @@ private VmWorkJobVO createPlaceHolderWork(final long instanceId, String secondar
         workJob.setStep(VmWorkJobVO.Step.Starting);
         workJob.setVmType(VirtualMachine.Type.Instance);
         workJob.setVmInstanceId(instanceId);
-        if(StringUtils.isNotBlank(secondaryObjectIdentifier)) {
-            workJob.setSecondaryObjectIdentifier(secondaryObjectIdentifier);
-        }
         workJob.setInitMsid(ManagementServerNode.getManagementServerId());
 
         _workJobDao.persist(workJob);

Diff for: engine/schema/src/main/resources/META-INF/db/schema-41520to41600.sql

@@ -805,6 +805,3 @@ ALTER TABLE cloud.user_vm_details MODIFY value varchar(5120) NOT NULL;
 ALTER TABLE cloud_usage.usage_network DROP PRIMARY KEY, ADD PRIMARY KEY (`account_id`,`zone_id`,`host_id`,`network_id`,`event_time_millis`);
 ALTER TABLE `cloud`.`user_statistics` DROP INDEX `account_id`, ADD UNIQUE KEY `account_id`  (`account_id`,`data_center_id`,`public_ip_address`,`device_id`,`device_type`, `network_id`);
 ALTER TABLE `cloud_usage`.`user_statistics` DROP INDEX `account_id`, ADD UNIQUE KEY `account_id`  (`account_id`,`data_center_id`,`public_ip_address`,`device_id`,`device_type`, `network_id`);
-
-ALTER TABLE `cloud`.`vm_work_job` ADD COLUMN `secondary_object` char(100) COMMENT 'any additional item that must be checked during queueing' AFTER `vm_instance_id`;
-ALTER TABLE cloud.vm_work_job ADD CONSTRAINT vm_work_job_step_and_objects UNIQUE KEY (step,vm_instance_id,secondary_object);

Diff for: framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java

@@ -32,8 +32,6 @@ public interface VmWorkJobDao extends GenericDao<VmWorkJobVO, Long> {
 
     List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type, long instanceId, String jobCmd);
 
-    List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type, long instanceId, String jobCmd, String secondaryObjectIdentifier);
-
     void updateStep(long workJobId, Step step);
 
     void expungeCompletedWorkJobs(Date cutDate);

Diff for: framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java

@@ -67,7 +67,6 @@ public void init() {
         PendingWorkJobByCommandSearch.and("jobStatus", PendingWorkJobByCommandSearch.entity().getStatus(), Op.EQ);
         PendingWorkJobByCommandSearch.and("vmType", PendingWorkJobByCommandSearch.entity().getVmType(), Op.EQ);
         PendingWorkJobByCommandSearch.and("vmInstanceId", PendingWorkJobByCommandSearch.entity().getVmInstanceId(), Op.EQ);
-        PendingWorkJobByCommandSearch.and("secondaryObjectIdentifier", PendingWorkJobByCommandSearch.entity().getSecondaryObjectIdentifier(), Op.EQ);
         PendingWorkJobByCommandSearch.and("step", PendingWorkJobByCommandSearch.entity().getStep(), Op.NEQ);
         PendingWorkJobByCommandSearch.and("cmd", PendingWorkJobByCommandSearch.entity().getCmd(), Op.EQ);
         PendingWorkJobByCommandSearch.done();
@@ -120,20 +119,6 @@ public List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type, long inst
         return this.listBy(sc, filter);
     }
 
-    @Override
-    public List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type, long instanceId, String jobCmd, String secondaryObjectIdentifier) {
-
-        SearchCriteria<VmWorkJobVO> sc = PendingWorkJobByCommandSearch.create();
-        sc.setParameters("jobStatus", JobInfo.Status.IN_PROGRESS);
-        sc.setParameters("vmType", type);
-        sc.setParameters("vmInstanceId", instanceId);
-        sc.setParameters("secondaryObjectIdentifier", secondaryObjectIdentifier);
-        sc.setParameters("cmd", jobCmd);
-
-        Filter filter = new Filter(VmWorkJobVO.class, "created", true, null, null);
-        return this.listBy(sc, filter);
-    }
-
     @Override
     public void updateStep(long workJobId, Step step) {
         VmWorkJobVO jobVo = findById(workJobId);

Diff for: framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java

@@ -384,7 +384,7 @@ public void setRemoved(final Date removed) {
     @Override
     public String toString() {
         StringBuffer sb = new StringBuffer();
-        sb.append("AsyncJobVO : {id:").append(getId());
+        sb.append("AsyncJobVO {id:").append(getId());
         sb.append(", userId: ").append(getUserId());
         sb.append(", accountId: ").append(getAccountId());
         sb.append(", instanceType: ").append(getInstanceType());

Diff for: framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java

@@ -58,9 +58,6 @@ boolean updateState() {
     @Column(name = "vm_instance_id")
     long vmInstanceId;
 
-    @Column(name = "secondary_object")
-    String secondaryObjectIdentifier;
-
     protected VmWorkJobVO() {
     }
 
@@ -92,25 +89,4 @@ public long getVmInstanceId() {
     public void setVmInstanceId(long vmInstanceId) {
         this.vmInstanceId = vmInstanceId;
     }
-
-    public String getSecondaryObjectIdentifier() {
-        return secondaryObjectIdentifier;
-    }
-
-    public void setSecondaryObjectIdentifier(String secondaryObjectIdentifier) {
-        this.secondaryObjectIdentifier = secondaryObjectIdentifier;
-    }
-    @Override
-    public String toString() {
-        StringBuffer sb = new StringBuffer();
-        sb.append("VmWorkJobVO : {").
-                append(", step: ").append(getStep()).
-                append(", vmType: ").append(getVmType()).
-                append(", vmInstanceId: ").append(getVmInstanceId()).
-                append(", secondaryObjectIdentifier: ").append(getSecondaryObjectIdentifier()).
-                append(super.toString()).
-                append("}");
-        return sb.toString();
-    }
-
 }

Diff for: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -1385,7 +1385,12 @@ public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) throws InvalidParameterV
         Account vmOwner = _accountMgr.getAccount(vmInstance.getAccountId());
         _networkModel.checkNetworkPermissions(vmOwner, network);
 
-        checkIfNetExistsForVM(vmInstance, network);
+        List<NicVO> allNics = _nicDao.listByVmId(vmInstance.getId());
+        for (NicVO nic : allNics) {
+            if (nic.getNetworkId() == network.getId()) {
+                throw new CloudRuntimeException("A NIC already exists for VM:" + vmInstance.getInstanceName() + " in network: " + network.getUuid());
+            }
+        }
 
         macAddress = validateOrReplaceMacAddress(macAddress, network.getId());
 
@@ -1452,22 +1457,10 @@ public UserVm addNicToVirtualMachine(AddNicToVMCmd cmd) throws InvalidParameterV
             }
         }
         CallContext.current().putContextParameter(Nic.class, guestNic.getUuid());
-        s_logger.debug(String.format("Successful addition of %s from %s through %s", network, vmInstance, guestNic));
+        s_logger.debug("Successful addition of " + network + " from " + vmInstance);
         return _vmDao.findById(vmInstance.getId());
     }
 
-    /**
-     * duplicated in {@see VirtualMachineManagerImpl} for a {@see VMInstanceVO}
-     */
-    private void checkIfNetExistsForVM(VirtualMachine virtualMachine, Network network) {
-        List<NicVO> allNics = _nicDao.listByVmId(virtualMachine.getId());
-        for (NicVO nic : allNics) {
-            if (nic.getNetworkId() == network.getId()) {
-                throw new CloudRuntimeException("A NIC already exists for VM:" + virtualMachine.getInstanceName() + " in network: " + network.getUuid());
-            }
-        }
-    }
-
     /**
      * If the given MAC address is invalid it replaces the given MAC with the next available MAC address
      */