Liron Ar has uploaded a new change for review. Change subject: wip: core: remove as sync verb. ......................................................................
wip: core: remove as sync verb. When performing DeleteImageGroup, the engine can treat the call as synchronous call as afterwards it the image is considered to be non exist on the storage side - when receiving an ImageDoesNotExist error from vdsm - proceed with removing the image. Change-Id: Ic822a92acb8231d2ca84ae092bd98659507925d8 Bug-Url: https://bugzilla.redhat.com/884635 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java M backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties 7 files changed, 56 insertions(+), 94 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/11/13611/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java index 2c2393c..e2133e8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java @@ -274,8 +274,9 @@ p, ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); if (vdcReturnValue.getSucceeded()) { + incrementVmsGeneration(); getReturnValue().getTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList()); - setSucceeded(vdcReturnValue.getSucceeded()); + setSucceeded(true); } } else { removeLunDisk(); @@ -295,39 +296,29 @@ @Override protected void endSuccessfully() { - endCommand(); + setSucceeded(true); } @Override protected void endWithFailure() { - endCommand(); + setSucceeded(true); } - private void endCommand() { + private void incrementVmsGeneration() { List<VM> listVms = getVmsForDiskId(); for (VM vm : listVms) { getVmStaticDAO().incrementDbGeneration(vm.getId()); } - // Get the disk before it is being deleted to use the disk alias in the audit log. - getDisk(); - Backend.getInstance().EndAction(VdcActionType.RemoveImage, getParameters().getImagesParameters().get(0)); - setSucceeded(true); } @Override public AuditLogType getAuditLogTypeValue() { switch (getActionState()) { case EXECUTE: - if (getDisk().getDiskStorageType() == DiskStorageType.IMAGE) { - return getSucceeded() ? AuditLogType.USER_REMOVE_DISK : AuditLogType.USER_FAILED_REMOVE_DISK; - } else { - return getSucceeded() ? AuditLogType.USER_FINISHED_REMOVE_DISK - : AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK; - } - case END_SUCCESS: - return AuditLogType.USER_FINISHED_REMOVE_DISK; + return getSucceeded() ? AuditLogType.USER_FINISHED_REMOVE_DISK + : AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK; default: - return AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK; + return AuditLogType.UNASSIGNED; } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java index 47a4f99..9e29344 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java @@ -19,6 +19,8 @@ import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.businessentities.ImageStorageDomainMapId; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; +import org.ovirt.engine.core.common.errors.VdcBLLException; +import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; @@ -76,17 +78,27 @@ @Override protected void executeCommand() { if (getDiskImage() != null) { - VDSReturnValue vdsReturnValue = performImageVdsmOperation(); - getReturnValue().getInternalTaskIdList().add( - createTask(vdsReturnValue.getCreationInfo(), - getParameters().getParentCommand(), - VdcObjectType.Storage, - getParameters().getStorageDomainId())); + try { + VDSReturnValue vdsReturnValue = performImageVdsmOperation(); + getReturnValue().getInternalTaskIdList().add( + createTask(vdsReturnValue.getCreationInfo(), + getParameters().getParentCommand(), + VdcObjectType.Storage, + getParameters().getStorageDomainId())); + } catch (VdcBLLException e) { + if (e.getErrorCode() != VdcBllErrors.ImageDoesNotExistInDomainError) { + throw e; + } + } - if (getParameters().isRemoveDuringExecution() - && getParameters().getParentCommand() != VdcActionType.RemoveVmFromImportExport + if (getParameters().getParentCommand() != VdcActionType.RemoveVmFromImportExport && getParameters().getParentCommand() != VdcActionType.RemoveVmTemplateFromImportExport) { - removeImageFromDB(false); + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + performImageDbOperations(); + return null; + }}); } } else { log.warn("DiskImage is null, nothing to remove"); @@ -264,27 +276,23 @@ @Override protected void endSuccessfully() { - endCommand(); + setSucceeded(true); } @Override protected void endWithFailure() { - endCommand(); + setSucceeded(true); } - private void endCommand() { + private void performImageDbOperations() { if (getParameters().getRemoveFromDB()) { - if (!getParameters().isRemoveDuringExecution()) { removeImageFromDB(true); - } } else { getImageStorageDomainMapDao().remove( new ImageStorageDomainMapId(getParameters().getImageId(), getParameters().getStorageDomainId())); unLockImage(); } - - setSucceeded(true); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java index 1d33026..44402bf 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java @@ -23,7 +23,6 @@ import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; -import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; @@ -76,18 +75,18 @@ if (getParameters().isRemoveDisks() && hasImages) { if (!removeVmImages(null)) { - return false; + prepareAuditLogMessageForDisksLeftInVm(); } } - else { - TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { - @Override - public Void runInTransaction() { - removeVmFromDb(); - return null; - } - }); - } + + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + removeVmFromDb(); + return null; + } + }); + return true; } @@ -230,16 +229,9 @@ public AuditLogType getAuditLogTypeValue() { switch (getActionState()) { case EXECUTE: - if (hasImages) { - return getSucceeded() ? AuditLogType.USER_REMOVE_VM : AuditLogType.USER_FAILED_REMOVE_VM; - } else { - return getSucceeded() ? AuditLogType.USER_REMOVE_VM_FINISHED : AuditLogType.USER_FAILED_REMOVE_VM; - } - case END_FAILURE: - case END_SUCCESS: + return getSucceeded() ? AuditLogType.USER_REMOVE_VM_FINISHED : hasImages ? AuditLogType.USER_REMOVE_VM_FINISHED_WITH_ILLEGAL_DISKS : AuditLogType.USER_FAILED_REMOVE_VM; default: - return disksLeftInVm.isEmpty() ? AuditLogType.USER_REMOVE_VM_FINISHED - : AuditLogType.USER_REMOVE_VM_FINISHED_WITH_ILLEGAL_DISKS; + return AuditLogType.UNASSIGNED; } } @@ -271,29 +263,13 @@ @Override protected void endVmCommand() { - try { - if (acquireLock()) { - // Ensures the lock on the VM guid can be acquired. This prevents a race - // between executeVmCommand (for example, of a first multiple VMs removal that includes VM A, - // and a second multiple VMs removal that include the same VM). - setVm(DbFacade.getInstance().getVmDao().get(getVmId())); - if (getVm() != null) { - updateDisksAfterVmRemoved(); - - // Remove VM from DB. - removeVmFromDb(); - } - } - setSucceeded(true); - } finally { - freeLock(); - } + setSucceeded(true); } /** * Update disks for VM after disks were removed. */ - private void updateDisksAfterVmRemoved() { + private void prepareAuditLogMessageForDisksLeftInVm() { VmHandler.updateDisksFromDb(getVm()); // Get all disk images for VM (VM should not have any image disk associated with it). @@ -301,16 +277,8 @@ true, false); - // If the VM still has disk images related to it, change their status to Illegal. if (!diskImages.isEmpty()) { for (DiskImage diskImage : diskImages) { - if (diskImage.getImageStatus() != ImageStatus.ILLEGAL) { - log.errorFormat("Disk {0} which is part of VM {1} was not at ILLEGAL state.", - diskImage.getDiskAlias(), - getVm().getName()); - ImagesHandler.updateImageStatus(diskImage.getImage().getId(), ImageStatus.ILLEGAL); - } - disksLeftInVm.add(diskImage.getDiskAlias()); } addCustomValue("DisksNames", StringUtils.join(disksLeftInVm, ",")); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java index 90a329c..1a2aa43 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java @@ -123,25 +123,29 @@ } else { getReturnValue().setFault(vdcRetValue.getFault()); } - } else { - EndRemoveTemplate(); } + EndRemoveTemplate(); } @Override public AuditLogType getAuditLogTypeValue() { - return getSucceeded() ? AuditLogType.IMPORTEXPORT_REMOVE_TEMPLATE - : AuditLogType.IMPORTEXPORT_REMOVE_TEMPLATE_FAILED; + switch (getActionState()) { + case EXECUTE: + return getSucceeded() ? AuditLogType.IMPORTEXPORT_REMOVE_TEMPLATE + : AuditLogType.IMPORTEXPORT_REMOVE_TEMPLATE_FAILED; + default: + return AuditLogType.UNASSIGNED; + } } @Override protected void endSuccessfully() { - EndRemoveTemplate(); + setSucceeded(true); } @Override protected void endWithFailure() { - EndRemoveTemplate(); + setSucceeded(true); } protected void EndRemoveTemplate() { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java index cc135ec..f611209 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java @@ -109,7 +109,6 @@ USER_UPDATE_VM(35), USER_FAILED_UPDATE_VM(58), USER_UPDATE_VM_CLUSTER_DEFAULT_HOST_CLEARED(250), - USER_REMOVE_VM(36), USER_REMOVE_VM_FINISHED(113), USER_REMOVE_VM_FINISHED_WITH_ILLEGAL_DISKS(172), USER_ADD(149), @@ -167,8 +166,6 @@ USER_FAILED_MOVED_VM_DISK(2009), USER_MOVED_VM_DISK_FINISHED_SUCCESS(2010), USER_MOVED_VM_DISK_FINISHED_FAILURE(2011), - USER_REMOVE_DISK(2012), - USER_FAILED_REMOVE_DISK(2013), USER_FINISHED_REMOVE_DISK(2014), USER_FINISHED_FAILED_REMOVE_DISK(2015), USER_ATTACH_DISK_TO_VM(2016), diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java index c820e33..9e5902e 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java @@ -371,8 +371,6 @@ severities.put(AuditLogType.USER_FAILED_MOVED_VM_DISK, AuditLogSeverity.ERROR); severities.put(AuditLogType.USER_MOVED_VM_DISK_FINISHED_SUCCESS, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_MOVED_VM_DISK_FINISHED_FAILURE, AuditLogSeverity.ERROR); - severities.put(AuditLogType.USER_REMOVE_DISK, AuditLogSeverity.NORMAL); - severities.put(AuditLogType.USER_FAILED_REMOVE_DISK, AuditLogSeverity.ERROR); severities.put(AuditLogType.USER_FINISHED_REMOVE_DISK, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK, AuditLogSeverity.WARNING); severities.put(AuditLogType.IRS_FAILURE, AuditLogSeverity.ERROR); @@ -473,7 +471,6 @@ severities.put(AuditLogType.USER_FAILED_ADD_VM, AuditLogSeverity.ERROR); severities.put(AuditLogType.USER_UPDATE_VM, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_FAILED_UPDATE_VM, AuditLogSeverity.ERROR); - severities.put(AuditLogType.USER_REMOVE_VM, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_REMOVE_VM_FINISHED, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_REMOVE_VM_FINISHED_WITH_ILLEGAL_DISKS, AuditLogSeverity.WARNING); severities.put(AuditLogType.USER_FAILED_REMOVE_VM, AuditLogSeverity.ERROR); diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties index 0aff7b0..850772b 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties @@ -92,8 +92,6 @@ USER_FAILED_MOVED_VM_DISK=User ${UserName} failed to move disk ${DiskAlias} to domain ${StorageDomainName}. USER_MOVED_VM_DISK_FINISHED_SUCCESS=User ${UserName} finished moving disk ${DiskAlias} to domain ${StorageDomainName}. USER_MOVED_VM_DISK_FINISHED_FAILURE=User ${UserName} have failed to move disk ${DiskAlias} to domain ${StorageDomainName}. -USER_REMOVE_DISK=User ${UserName} initiate removing of disk ${DiskAlias} from domain ${StorageDomainName}. -USER_FAILED_REMOVE_DISK=User ${UserName} failed to initiate removing of disk ${DiskAlias} from domain ${StorageDomainName}. USER_FINISHED_REMOVE_DISK=User ${UserName} finished removing disk ${DiskAlias} from domain ${StorageDomainName}. USER_FINISHED_FAILED_REMOVE_DISK=User ${UserName} finished removing disk ${DiskAlias} with storage failure in domain ${StorageDomainName}. USER_ATTACH_DISK_TO_VM=Disk ${DiskAlias} was successfully attached to VM ${VmName} by ${UserName}. @@ -138,7 +136,6 @@ USER_REMOVE_BOOKMARK=Bookmark ${BookmarkName} was removed by ${UserName}. USER_REMOVE_BOOKMARK_FAILED=Failed to remove bookmark ${BookmarkName} (User: ${UserName}) USER_REMOVE_VDS=Host ${VdsName} was removed by ${UserName}. -USER_REMOVE_VM=Removal of VM ${VmName} was initiated by ${UserName}. USER_REMOVE_VM_FINISHED=VM ${VmName} was successfully removed. USER_REMOVE_VM_FINISHED_WITH_ILLEGAL_DISKS=VM ${VmName} has been removed, but the following disks could not be removed: ${DisksNames}. These disks will appear in the main disks tab in illegal state, please remove manually when possible. USER_REMOVE_VM_FROM_POOL=VM ${VmName} was removed from VM Pool ${VmPoolName} by ${UserName}. -- To view, visit http://gerrit.ovirt.org/13611 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic822a92acb8231d2ca84ae092bd98659507925d8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
