Arik Hadas has uploaded a new change for review. Change subject: core: cleanup in AddVmCommand ......................................................................
core: cleanup in AddVmCommand - No need for both tempVar & vmDynamic fields in addVmDynamic method, thus tempVar is removed - Replaced size() > 0 with !isEmpty() - addVmImages always returns true (exception is thrown in case of an error), so it is changed to void - Extract parameters construction for CreateSnapshotFromTemplate command to separate method Change-Id: Idbe9402fc4dd28ac09b014027cc01a880759f360 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java 1 file changed, 48 insertions(+), 44 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/93/32493/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 33232a7..45d44a7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -767,24 +767,24 @@ } }); - if (addVmImages()) { - TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + addVmImages(); - @Override - public Void runInTransaction() { - copyVmDevices(); - addDiskPermissions(); - if (getInstanceType() == null) { - addVmPayload(); - updateSmartCardDevices(); - addVmWatchdog(); - } - setActionReturnValue(getVm().getId()); - setSucceeded(true); - return null; + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + + @Override + public Void runInTransaction() { + copyVmDevices(); + addDiskPermissions(); + if (getInstanceType() == null) { + addVmPayload(); + updateSmartCardDevices(); + addVmWatchdog(); } - }); - } + setActionReturnValue(getVm().getId()); + setSucceeded(true); + return null; + } + }); } else { log.errorFormat("Failed to add vm . The reasons are: {0}", StringUtils.join(errorMessages, ',')); } @@ -951,16 +951,15 @@ } void addVmDynamic() { - VmDynamic tempVar = new VmDynamic(); - tempVar.setId(getVmId()); - tempVar.setStatus(VMStatus.Down); - tempVar.setVmHost(""); - tempVar.setVmIp(""); - tempVar.setVmFQDN(""); - tempVar.setDisplayType(getParameters().getVmStaticData().getDefaultDisplayType()); - tempVar.setLastStopTime(new Date()); - VmDynamic vmDynamic = tempVar; - DbFacade.getInstance().getVmDynamicDao().save(vmDynamic); + VmDynamic vmDynamic = new VmDynamic(); + vmDynamic.setId(getVmId()); + vmDynamic.setStatus(VMStatus.Down); + vmDynamic.setVmHost(""); + vmDynamic.setVmIp(""); + vmDynamic.setVmFQDN(""); + vmDynamic.setDisplayType(getParameters().getVmStaticData().getDefaultDisplayType()); + vmDynamic.setLastStopTime(new Date()); + getDbFacade().getVmDynamicDao().save(vmDynamic); getCompensationContext().snapshotNewEntity(vmDynamic); } @@ -971,26 +970,17 @@ getCompensationContext().snapshotNewEntity(stats); } - protected boolean addVmImages() { - if (vmDisksSource.getDiskTemplateMap().size() > 0) { + protected void addVmImages() { + if (!vmDisksSource.getDiskTemplateMap().isEmpty()) { if (getVm().getStatus() != VMStatus.Down) { log.error("Cannot add images. VM is not Down"); throw new VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL); } VmHandler.lockVm(getVmId()); - for (DiskImage dit : getImagesToCheckDestinationStorageDomains()) { - CreateSnapshotFromTemplateParameters tempVar = new CreateSnapshotFromTemplateParameters( - dit.getImageId(), getParameters().getVmStaticData().getId()); - tempVar.setDestStorageDomainId(diskInfoDestinationMap.get(dit.getId()).getStorageIds().get(0)); - tempVar.setDiskAlias(diskInfoDestinationMap.get(dit.getId()).getDiskAlias()); - tempVar.setStorageDomainId(dit.getStorageIds().get(0)); - tempVar.setVmSnapshotId(getVmSnapshotId()); - tempVar.setParentCommand(VdcActionType.AddVm); - tempVar.setEntityInfo(getParameters().getEntityInfo()); - tempVar.setParentParameters(getParameters()); - tempVar.setQuotaId(diskInfoDestinationMap.get(dit.getId()).getQuotaId()); - tempVar.setDiskProfileId(diskInfoDestinationMap.get(dit.getId()).getDiskProfileId()); - VdcReturnValueBase result = runInternalActionWithTasksContext(VdcActionType.CreateSnapshotFromTemplate, tempVar); + for (DiskImage image : getImagesToCheckDestinationStorageDomains()) { + VdcReturnValueBase result = runInternalActionWithTasksContext( + VdcActionType.CreateSnapshotFromTemplate, + buildCreateSnapshotFromTemplateParameters(image)); /** * if couldn't create snapshot then stop the transaction and the command @@ -1000,18 +990,32 @@ } else { getTaskIdList().addAll(result.getInternalVdsmTaskIdList()); DiskImage newImage = (DiskImage) result.getActionReturnValue(); - srcDiskIdToTargetDiskIdMapping.put(dit.getId(), newImage.getId()); + srcDiskIdToTargetDiskIdMapping.put(image.getId(), newImage.getId()); } } } - return true; } + private CreateSnapshotFromTemplateParameters buildCreateSnapshotFromTemplateParameters(DiskImage image) { + CreateSnapshotFromTemplateParameters tempVar = new CreateSnapshotFromTemplateParameters( + image.getImageId(), getParameters().getVmStaticData().getId()); + tempVar.setDestStorageDomainId(diskInfoDestinationMap.get(image.getId()).getStorageIds().get(0)); + tempVar.setDiskAlias(diskInfoDestinationMap.get(image.getId()).getDiskAlias()); + tempVar.setStorageDomainId(image.getStorageIds().get(0)); + tempVar.setVmSnapshotId(getVmSnapshotId()); + tempVar.setParentCommand(VdcActionType.AddVm); + tempVar.setEntityInfo(getParameters().getEntityInfo()); + tempVar.setParentParameters(getParameters()); + tempVar.setQuotaId(diskInfoDestinationMap.get(image.getId()).getQuotaId()); + tempVar.setDiskProfileId(diskInfoDestinationMap.get(image.getId()).getDiskProfileId()); + + return tempVar; + } @Override public AuditLogType getAuditLogTypeValue() { switch (getActionState()) { case EXECUTE: - return getSucceeded() ? (getReturnValue().getVdsmTaskIdList().size() > 0 ? AuditLogType.USER_ADD_VM_STARTED + return getSucceeded() ? (!getReturnValue().getVdsmTaskIdList().isEmpty() ? AuditLogType.USER_ADD_VM_STARTED : AuditLogType.USER_ADD_VM) : AuditLogType.USER_FAILED_ADD_VM; case END_SUCCESS: -- To view, visit http://gerrit.ovirt.org/32493 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idbe9402fc4dd28ac09b014027cc01a880759f360 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
