You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by nv...@apache.org on 2021/11/04 12:23:44 UTC
[cloudstack] branch revert-5541-synchronous-nic-adding created (now
32485ca)
This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a change to branch revert-5541-synchronous-nic-adding
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.
at 32485ca Revert "parallel nic adding (#5541)"
This branch includes the following new commits:
new 32485ca Revert "parallel nic adding (#5541)"
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[cloudstack] 01/01: Revert "parallel nic adding (#5541)"
Posted by nv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch revert-5541-synchronous-nic-adding
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 32485ca765a7511beb0b84c50fe28d01e9c24190
Author: Nicolas Vazquez <ni...@gmail.com>
AuthorDate: Thu Nov 4 09:22:54 2021 -0300
Revert "parallel nic adding (#5541)"
This reverts commit 3574d8d20be01b90ca09e58ab4b492604c1a8d1f.
---
.../com/cloud/vm/VirtualMachineManagerImpl.java | 89 +++++-----------------
.../resources/META-INF/db/schema-41520to41600.sql | 3 -
.../framework/jobs/dao/VmWorkJobDao.java | 2 -
.../framework/jobs/dao/VmWorkJobDaoImpl.java | 15 ----
.../cloudstack/framework/jobs/impl/AsyncJobVO.java | 2 +-
.../framework/jobs/impl/VmWorkJobVO.java | 24 ------
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 21 ++---
7 files changed, 29 insertions(+), 127 deletions(-)
diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
index 2604298..1bba582 100755
--- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
+++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -42,7 +42,6 @@ import java.util.concurrent.TimeUnit;
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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
}
}
- /**
- * 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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
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 class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
}
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 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
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 --git a/engine/schema/src/main/resources/META-INF/db/schema-41520to41600.sql b/engine/schema/src/main/resources/META-INF/db/schema-41520to41600.sql
index 16a0dd7..d7c0641 100644
--- a/engine/schema/src/main/resources/META-INF/db/schema-41520to41600.sql
+++ b/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 --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java
index 89601e6..44e39e4 100644
--- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java
+++ b/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 --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
index 497f12d..e81ab1e 100644
--- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
+++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
@@ -67,7 +67,6 @@ public class VmWorkJobDaoImpl extends GenericDaoBase<VmWorkJobVO, Long> implemen
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();
@@ -121,20 +120,6 @@ public class VmWorkJobDaoImpl extends GenericDaoBase<VmWorkJobVO, Long> implemen
}
@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);
jobVo.setStep(step);
diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
index 777fcba..9d30c2c 100644
--- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
+++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
@@ -384,7 +384,7 @@ public class AsyncJobVO implements AsyncJob, JobInfo {
@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 --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
index a8a05d4..ef0ac7d 100644
--- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
+++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
@@ -58,9 +58,6 @@ public class VmWorkJobVO extends AsyncJobVO {
@Column(name = "vm_instance_id")
long vmInstanceId;
- @Column(name = "secondary_object")
- String secondaryObjectIdentifier;
-
protected VmWorkJobVO() {
}
@@ -92,25 +89,4 @@ public class VmWorkJobVO extends AsyncJobVO {
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 --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index efb8f49..d7c90b3 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -1385,7 +1385,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
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,23 +1457,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
}
}
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
*/
protected String validateOrReplaceMacAddress(String macAddress, long networkId) {