Arik Hadas has uploaded a new change for review. Change subject: core: lock VM for whole live disk migration process ......................................................................
core: lock VM for whole live disk migration process In order to prevent a race between VM migration and storage commands, the storage commands need to lock the VM for their entire execution. This patch takes shared lock for the VM for the entire live storage migration process. Change-Id: I432e9230da697a1a67c5d40e926e16215c2583ef Bug-Url: https://bugzilla.redhat.com/952147 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java 3 files changed, 30 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/45/18545/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java index 4816b1f..f3b0544 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java @@ -498,7 +498,7 @@ } catch (TransactionRolledbackLocalException e) { log.infoFormat("EndAction: Transaction was aborted in {0}", this.getClass().getName()); } finally { - freeLock(); + freeLockEndAction(); if (getCommandShouldBeLogged()) { logCommand(); } @@ -570,7 +570,7 @@ exceptionOccurred = true; throw e; } finally { - freeLock(); + freeLockEndAction(); if (TransactionSupport.current() == null) { // In the unusual case that we have no current transaction, try to cleanup after yourself and if the @@ -1845,6 +1845,17 @@ } } + /** + * If the command has more than one task handler, we can reach the end action + * phase and in that phase execute the next task handler. in that case, we + * don't want to release the locks, so we ask whether we're not in execute state. + */ + private void freeLockEndAction() { + if (getActionState() != CommandActionState.EXECUTE) { + freeLock(); + } + } + protected void freeLock() { if (commandLock != null) { getLockManager().releaseLock(commandLock); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java index 49ca9bc..ffc1d42 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java @@ -22,7 +22,7 @@ import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; @NonTransactiveCommandAttribute -@LockIdNameAttribute +@LockIdNameAttribute(isReleaseAtEndOfExecute = false) public class LiveMigrateDiskCommand<T extends LiveMigrateDiskParameters> extends MoveOrCopyDiskCommand<T> implements TaskHandlerCommand<LiveMigrateDiskParameters> { public LiveMigrateDiskCommand(T parameters) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java index 6c3582a..31534a3 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java @@ -26,6 +26,7 @@ import org.ovirt.engine.core.common.action.LiveMigrateDiskParameters; import org.ovirt.engine.core.common.action.LiveMigrateVmDisksParameters; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo; import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.DiskImage; @@ -40,7 +41,7 @@ import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.utils.collections.MultiValueMapUtils; -@LockIdNameAttribute +@LockIdNameAttribute(isReleaseAtEndOfExecute = false) @NonTransactiveCommandAttribute @InternalCommandAttribute public class LiveMigrateVmDisksCommand<T extends LiveMigrateVmDisksParameters> extends CommandBase<T> @@ -62,6 +63,19 @@ new LiveSnapshotTaskHandler(this), new LiveMigrateDisksTaskHandler(this) ); + } + + /** + * Ugly hack, but it is needed as the endAction method in this command is called only + * once and in that case it executes the next task handler, so we have no other option + * but to release the lock right after executing the next task handler, assuming it'll + * take the VM lock by himself so we won't end up in a state where the VM is not locked. + */ + @Override + public VdcReturnValueBase endAction() { + VdcReturnValueBase vdcReturnValueBase = super.endAction(); + freeLock(); + return vdcReturnValueBase; } /* Overridden stubs declared as public in order to implement ITaskHandlerCommand */ @@ -131,7 +145,7 @@ } @Override - protected Map<String, Pair<String, String>> getExclusiveLocks() { + protected Map<String, Pair<String, String>> getSharedLocks() { return Collections.singletonMap(getVmId().toString(), LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM, VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); } -- To view, visit http://gerrit.ovirt.org/18545 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I432e9230da697a1a67c5d40e926e16215c2583ef 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
