Federico Simoncelli has uploaded a new change for review. Change subject: backend: [wip] improve live snapshot rollback handling ......................................................................
backend: [wip] improve live snapshot rollback handling Change-Id: Id13469bace3810879dafa7f41af115bd64fea646 Signed-off-by: Federico Simoncelli <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/CreateAllSnapshotsFromVmParameters.java 2 files changed, 54 insertions(+), 24 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/81/20281/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java index 96e4e69..ac7be3b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java @@ -45,6 +45,7 @@ import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBLLException; +import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; @@ -211,37 +212,44 @@ return isInternalExecution() ? getParameters().getSnapshotType() : SnapshotType.REGULAR; } + protected void rollbackMemorySaving(Snapshot createdSnapshot) { + if (createdSnapshot.containsMemory()) { + logMemorySavingFailed(); + getSnapshotDao().removeMemoryFromSnapshot(createdSnapshot.getId()); + removeMemoryVolumesOfSnapshot(createdSnapshot); + } + } + + @Override + protected boolean overrideChildCommandSuccess() { + return getParameters().getRollbackLiveSnapshot(); + } + @Override protected void endVmCommand() { Snapshot createdSnapshot = getSnapshotDao().get(getVmId(), getParameters().getSnapshotType(), SnapshotStatus.LOCKED); - // if the snapshot was not created in the DB - // the command should also be handled as a failure + // if the snapshot was not created in the DB the command should also be handled as a failure boolean taskGroupSucceeded = createdSnapshot != null && getParameters().getTaskGroupSuccess(); if (taskGroupSucceeded) { getSnapshotDao().updateStatus(createdSnapshot.getId(), SnapshotStatus.OK); - if (isLiveSnapshotApplicable()) { performLiveSnapshot(createdSnapshot); + taskGroupSucceeded = taskGroupSucceeded & getParameters().getTaskGroupSuccess(); } else { - // If the created snapshot contains memory, remove the memory volumes as - // they are not going to be in use since no live snapshot is created - if (createdSnapshot.containsMemory()) { - logMemorySavingFailed(); + rollbackMemorySaving(createdSnapshot); + } + } - getSnapshotDao().removeMemoryFromSnapshot(createdSnapshot.getId()); - removeMemoryVolumesOfSnapshot(createdSnapshot); + if (createdSnapshot != null) { + if (!taskGroupSucceeded) { + rollbackMemorySaving(createdSnapshot); // Always revert memory snapshots + if (getParameters().getRollbackLiveSnapshot()) { + revertToActiveSnapshot(createdSnapshot.getId()); } } } else { - if (createdSnapshot != null) { - revertToActiveSnapshot(createdSnapshot.getId()); - // If the removed snapshot contained memory, remove the memory volumes - // Note that the memory volumes might not have been created - removeMemoryVolumesOfSnapshot(createdSnapshot); - } else { - log.warnFormat("No snapshot was created for VM {0} which is in LOCKED status", getVmId()); - } + log.warnFormat("No snapshot was created for VM {0} which is in LOCKED status", getVmId()); } incrementVmGeneration(); @@ -301,22 +309,35 @@ * unrecoverable then the {@link CreateAllSnapshotsFromVmParameters#getTaskGroupSuccess()} will return false - which * will indicate that rollback of snapshot command should happen. * - * @param createdSnapshotId + * @param snapshot * Snapshot to revert to being active, in case of rollback. */ protected void performLiveSnapshot(final Snapshot snapshot) { try { TransactionSupport.executeInScope(TransactionScopeOption.Suppress, new TransactionMethod<Void>() { - @Override public Void runInTransaction() { - runVdsCommand(VDSCommandType.Snapshot, buildLiveSnapshotParameters(snapshot)); return null; } }); } catch (VdcBLLException e) { - handleVdsLiveSnapshotFailure(e); + // In any case the live snapshot command failed, we need to decide whether we can safely + // rollback or not. + getParameters().setTaskGroupSuccess(false); + + // FIXME: sadly there's no difference between ConnectException and any other type of network error + if (e.getErrorCode() == VdcBllErrors.VDS_NETWORK_ERROR || e.getErrorCode() == VdcBllErrors.unexpected) { + // We have no clue on what happened on the vdsm side, preventing rollback + log.warnFormat("Wasn't able to live snapshot due to error: {0}, preventing rollback", + ExceptionUtils.getMessage(e)); + logLiveSnapshotFailure(e); + } else { + // We know for sure that the vm didn't switch to the new volumes, rollback snapshot + log.errorFormat("Wasn't able to live snapshot due to error: {0}, executing rollback", + ExceptionUtils.getMessage(e)); + getParameters().setRollbackLiveSnapshot(true); + } } } @@ -336,9 +357,7 @@ } } - private void handleVdsLiveSnapshotFailure(VdcBLLException e) { - log.warnFormat("Wasn't able to live snapshot due to error: {0}. VM will still be configured to the new created snapshot", - ExceptionUtils.getMessage(e)); + private void logLiveSnapshotFailure(VdcBLLException e) { addCustomValue("SnapshotName", getSnapshotName()); addCustomValue("VmName", getVmName()); updateCallStackFromThrowable(e); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/CreateAllSnapshotsFromVmParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/CreateAllSnapshotsFromVmParameters.java index d445c77..6f86a16 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/CreateAllSnapshotsFromVmParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/CreateAllSnapshotsFromVmParameters.java @@ -27,6 +27,9 @@ /** Used to indicate whether the memory should be saved as part of this snapshot or not */ private boolean saveMemory; + /** Used to indicate whether the live snapshot command succeeded or if it failed */ + private boolean rollbackLiveSnapshot; + public CreateAllSnapshotsFromVmParameters() { needsLocking = true; } @@ -74,4 +77,12 @@ public void setNeedsLocking(boolean needsLocking) { this.needsLocking = needsLocking; } + + public boolean getRollbackLiveSnapshot() { + return rollbackLiveSnapshot; + } + + public void setRollbackLiveSnapshot(boolean rollbackLiveSnapshot) { + this.rollbackLiveSnapshot = rollbackLiveSnapshot; + } } -- To view, visit http://gerrit.ovirt.org/20281 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id13469bace3810879dafa7f41af115bd64fea646 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
