Liron Aravot has uploaded a new change for review. Change subject: core: RemoveImageCommand - avoid image staying LOCKED, revert to ILLEGAL ......................................................................
core: RemoveImageCommand - avoid image staying LOCKED, revert to ILLEGAL When running remove image command, a DeleteImageGroup vdsm task is being initiated by the engine. In case of failure or engine crash, the engine can't tell whether the task has been initiated on vdsm side or not - therefore the image status should be set to ILLEGAL, in the current situation the image just stayed LOCKED. NOTE: the lockImage() with no compensation was left in BaseImagesCommand as it's used by commands that are being executed within transaction, so until they will changed to be non transactive, there's no need to start new transaction within their execution. Change-Id: Ib5ec91a5f8b8495b8cfd11b19016fee5993644ba Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=840525 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java 2 files changed, 30 insertions(+), 16 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/75/11075/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java index 4e25ab4..984753a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java @@ -24,6 +24,8 @@ import org.ovirt.engine.core.dao.BaseDiskDao; import org.ovirt.engine.core.dao.ImageDao; import org.ovirt.engine.core.dao.SnapshotDao; +import org.ovirt.engine.core.utils.transaction.TransactionMethod; +import org.ovirt.engine.core.utils.transaction.TransactionSupport; /** * Base class for all image handling commands @@ -332,6 +334,18 @@ setImageStatus(ImageStatus.LOCKED); } + protected void lockImageWithCompensation() { + final DiskImage diskImage = getRelevantDiskImage(); + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + getCompensationContext().snapshotEntityStatus(diskImage.getImage(), diskImage.getimageStatus()); + getCompensationContext().stateChanged(); + setImageStatus(ImageStatus.LOCKED); + return null; + }}); + } + protected void unLockImage() { setImageStatus(ImageStatus.OK); } 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 19785c5..157c2da 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 @@ -12,12 +12,12 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.VM; 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.locks.LockingGroup; import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; @@ -37,7 +37,7 @@ */ @SuppressWarnings("serial") @InternalCommandAttribute -@NonTransactiveCommandAttribute +@NonTransactiveCommandAttribute(forceCompensation=true) public class RemoveImageCommand<T extends RemoveImageParameters> extends BaseImagesCommand<T> { private EngineLock snapshotsEngineLock; @@ -46,6 +46,10 @@ initImage(); initStoragePoolId(); initStorageDomainId(); + } + + protected RemoveImageCommand(Guid commandId) { + super(commandId); } protected void initImage() { @@ -280,24 +284,20 @@ boolean isShouldBeLocked = getParameters().getParentCommand() != VdcActionType.RemoveVmFromImportExport && getParameters().getParentCommand() != VdcActionType.RemoveVmTemplateFromImportExport; if (isShouldBeLocked) { - lockImage(); + // the image status should be set to ILLEGAL, so that in case compensation runs the image status will + // be revert to ILLEGAL, as we can't tell were the task started on vdsm side or not. + getDiskImage().setimageStatus(ImageStatus.ILLEGAL); + lockImageWithCompensation(); } // Releasing the lock for cases it was set by the parent command. The lock can be released because the image // status was already changed to lock. freeLock(); - try { - VDSReturnValue returnValue = runVdsCommand(VDSCommandType.DeleteImageGroup, - new DeleteImageGroupVDSCommandParameters(getDiskImage().getstorage_pool_id().getValue(), - getStorageDomainId().getValue(), getDiskImage().getId() - .getValue(), getDiskImage().isWipeAfterDelete(), getParameters() - .getForceDelete(), getStoragePool().getcompatibility_version().toString())); - return returnValue; - } catch (VdcBLLException e) { - if (isShouldBeLocked) { - unLockImage(); - } - throw e; - } + VDSReturnValue returnValue = runVdsCommand(VDSCommandType.DeleteImageGroup, + new DeleteImageGroupVDSCommandParameters(getDiskImage().getstorage_pool_id().getValue(), + getStorageDomainId().getValue(), getDiskImage().getId() + .getValue(), getDiskImage().isWipeAfterDelete(), getParameters() + .getForceDelete(), getStoragePool().getcompatibility_version().toString())); + return returnValue; } protected VmDeviceDAO getVmDeviceDAO() { -- To view, visit http://gerrit.ovirt.org/11075 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib5ec91a5f8b8495b8cfd11b19016fee5993644ba Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
