Fix execution of RemoveImage

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/RemoveDiskCommand.java

@@ -516,7 +516,7 @@ private RemoveImageParameters buildRemoveImageParameters(DiskImage diskImage) {
         RemoveImageParameters result = new RemoveImageParameters(diskImage.getImageId());
         result.setTransactionScopeOption(TransactionScopeOption.Suppress);
         result.setDiskImage(diskImage);
-        result.setParentCommand(ActionType.RemoveDisk);
+        result.setParentCommand(getActionType());
         result.setEntityInfo(new EntityInfo(VdcObjectType.Disk, getParameters().getDiskId()));
         result.setParentParameters(getParameters());
         result.setRemoveFromSnapshots(true);

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/RemoveImageCommand.java

@@ -9,16 +9,13 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import javax.enterprise.inject.Instance;
-import javax.enterprise.inject.Typed;
 import javax.inject.Inject;
 
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.bll.InternalCommandAttribute;
 import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
 import org.ovirt.engine.core.bll.context.CommandContext;
 import org.ovirt.engine.core.bll.storage.domain.PostDeleteActionHandler;
-import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.VdcObjectType;
 import org.ovirt.engine.core.common.action.ActionType;
@@ -88,10 +85,6 @@ public class RemoveImageCommand<T extends RemoveImageParameters> extends BaseIma
     @Inject
     private VmDao vmDao;
 
-    @Inject
-    @Typed(RemoveImageCommandCallback.class)
-    private Instance<RemoveImageCommandCallback> callbackProvider;
-
 
     public RemoveImageCommand(T parameters, CommandContext cmdContext) {
         super(parameters, cmdContext);
@@ -136,7 +129,7 @@ protected void executeCommand() {
                 VDSReturnValue vdsReturnValue = performDeleteImageVdsmOperation();
                 Guid vdsmTaskId = createTask(taskId,
                         vdsReturnValue.getCreationInfo(),
-                        getParameters().getParentCommand(),
+                        ActionType.RemoveImage,
                         VdcObjectType.Storage,
                         getStorageDomainId());
                 getTaskIdList().add(vdsmTaskId);
@@ -145,12 +138,14 @@ protected void executeCommand() {
                 if (e.getErrorCode() == EngineError.ImageDoesNotExistInDomainError) {
                     log.info("Disk '{}' doesn't exist on storage domain '{}', rolling forward",
                             getDiskImage().getId(), getStorageDomainId());
+                    endSuccessfully();
                 } else if (e.getErrorCode() == EngineError.ImageDeleteError && isImageRemovedFromStorage()) {
                     // VDSM renames the image before deleting it, so technically the image doesn't exist after renaming,
                     // but the actual delete can still fail with ImageDeleteError.
                     // In this case, Engine has to check whether image still exists on the storage or not.
-                    log.info("Disk '{}' was deleted from storage domain '{}'", getDiskImage().getId(),
-                            getStorageDomainId());
+                    log.info("Failed to delete Disk '{}' from storage domain '{}', changing to illegal",
+                            getDiskImage().getId(), getStorageDomainId());
+                    endWithFailure();
                 } else {
                     throw e;
                 }
@@ -286,11 +281,21 @@ protected List<Snapshot> prepareSnapshotConfigWithoutImage(Guid imageGroupToRemo
 
     @Override
     protected void endSuccessfully() {
+        performImageDbOperations();
         setSucceeded(true);
     }
 
     @Override
     protected void endWithFailure() {
+        List<Guid> snapshotIds = diskImageDao
+                .getAllSnapshotsForImageGroup(getImageGroupId()).stream()
+                .map(DiskImage::getImageId)
+                .collect(Collectors.toList());
+        addCustomValue("DiskId", getImageGroupId().toString());
+        addCustomValue("VolumeIds", StringUtils.join(snapshotIds, ','));
+        auditLog(this, AuditLogType.DELETE_IMAGE_GROUP_TASK_FAILED);
+
+        imageDao.updateStatusOfImagesByImageGroupId(getImageGroupId(), ImageStatus.ILLEGAL);
         setSucceeded(true);
     }
 
@@ -337,33 +342,4 @@ protected VDSReturnValue performDeleteImageVdsmOperation() {
                                 getDiskImage().isWipeAfterDelete(), getStorageDomain().getDiscardAfterDelete(),
                                 getParameters().isForceDelete())));
     }
-
-    @Override
-    public CommandCallback getCallback() {
-        return callbackProvider.get();
-    }
-
-    public void onSucceeded() {
-        performImageDbOperations();
-    }
-
-    public void onFailed() {
-        if (getParameters().getAsyncTaskErrorMessage() != null) {
-            Guid diskId = getParameters().getDiskImage().getId();
-            List<Guid> snapshotIds = diskImageDao
-                    .getAllSnapshotsForImageGroup(diskId).stream()
-                    .map(DiskImage::getImageId)
-                    .collect(Collectors.toList());
-            addCustomValue("DiskId", diskId.toString());
-            addCustomValue("VolumeIds", StringUtils.join(snapshotIds, ','));
-            addCustomValue("ErrorMessage", getParameters().getAsyncTaskErrorMessage());
-            auditLog(this, AuditLogType.DELETE_IMAGE_GROUP_TASK_FAILED);
-        }
-        imageDao.updateStatusOfImagesByImageGroupId(getImageGroupId(), ImageStatus.ILLEGAL);
-    }
-
-    @Override
-    protected void setSucceeded(boolean value) {
-        super.setSucceeded(value);
-    }
 }

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java

@@ -13,13 +13,13 @@ public class RemoveImageParameters extends ImagesContainterParametersBase implem
     private ImageDbOperationScope dbOperationScope;
     private boolean removeFromSnapshots;
     private boolean shouldLockImage;
-    private String asyncTaskErrorMessage;
 
     public RemoveImageParameters(Guid imageId) {
         super(imageId, null);
         setForceDelete(false);
         dbOperationScope = ImageDbOperationScope.IMAGE;
         shouldLockImage= true;
+        setEndProcedure(EndProcedure.COMMAND_MANAGED);
     }
 
     public RemoveImageParameters() {
@@ -56,12 +56,4 @@ public boolean isRemoveFromSnapshots() {
     public void setRemoveFromSnapshots(boolean removeFromSnapshots) {
         this.removeFromSnapshots = removeFromSnapshots;
     }
-
-    public String getAsyncTaskErrorMessage() {
-        return asyncTaskErrorMessage;
-    }
-
-    public void setAsyncTaskErrorMessage(String asyncTaskErrorMessage) {
-        this.asyncTaskErrorMessage = asyncTaskErrorMessage;
-    }
 }

backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties

@@ -1360,4 +1360,4 @@ VNICS_OUT_OF_SYNC_ON_NETWORK_UPDATE=Update to network ${NetworkName} was not app
 VNICS_OUT_OF_SYNC_ON_PROFILE_UPDATE=Update to network interface profile ${ProfileName} was not applied to virtual network interfaces [${VnicNames}]. The actual configuration on the interfaces may differ from the displayed one.
 VM_BACKUP_SNAPSHOT_POSSIBLE_LEFTOVER=Possible failure while removing a snapshot that was generated automatically during backup of VM '${VmName}'. The snapshot may be removed manually
 VM_BACKUP_FAILED_UNMONITORED=Backup ${BackupId} for VM ${VmName} was marked as failed since it is no longer monitored, cleanup may be required.
-DELETE_IMAGE_GROUP_TASK_FAILED=Image group deletion task for disk ${DiskId} with volume(s) [${VolumeIds}] failed. Manual volume removal might be required. Underlying error message: ${ErrorMessage}
+DELETE_IMAGE_GROUP_TASK_FAILED=Image group deletion task for disk ${DiskId} with volume(s) [${VolumeIds}] failed. Manual volume removal might be required.