Arik Hadas has uploaded a new change for review. Change subject: core: memory state handling on preview/commit/undo ......................................................................
core: memory state handling on preview/commit/undo On preview snapshot, add the memory from the previewed snapshot to the active snapshot that is created, if the 'restoreMemory' flag is set in the parameters. On commit/undo, add the memory from the restored snapshot to the active snapshot that is created. Change-Id: I42c1b7dd4fcdf12fba1943792a1442984796cf45 Bug-Url: https://bugzilla.redhat.com/960931 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/TryBackToAllSnapshotsOfVmParameters.java 4 files changed, 124 insertions(+), 26 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/79/15679/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java index 43b4b21..3a85d1a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java @@ -228,7 +228,11 @@ getCompensationContext(), getVm().getVdsGroupCompatibilityVersion()); getSnapshotDao().remove(targetSnapshot.getId()); // add active snapshot with status locked, so that other commands that depend on the VM's snapshots won't run in parallel - snapshotsManager.addActiveSnapshot(targetSnapshot.getId(), getVm(), SnapshotStatus.LOCKED, getCompensationContext()); + snapshotsManager.addActiveSnapshot(targetSnapshot.getId(), + getVm(), + SnapshotStatus.LOCKED, + targetSnapshot.getMemoryVolume(), + getCompensationContext()); } /** diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java index f31bcb7..79e7422 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java @@ -5,6 +5,7 @@ import java.util.Map; import java.util.Set; +import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.snapshots.SnapshotsManager; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; @@ -13,6 +14,7 @@ import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; +import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.ImagesContainterParametersBase; import org.ovirt.engine.core.common.action.TryBackToAllSnapshotsOfVmParameters; @@ -40,6 +42,7 @@ public class TryBackToAllSnapshotsOfVmCommand<T extends TryBackToAllSnapshotsOfVmParameters> extends VmCommand<T> { private final SnapshotsManager snapshotsManager = new SnapshotsManager(); + private Snapshot cachedSnapshot; /** * Constructor for command creation when compensation is applied on startup @@ -67,12 +70,13 @@ @Override protected void endWithFailure() { - Guid previouslyActiveSnapshotId = - getSnapshotDao().getId(getVmId(), SnapshotType.PREVIEW, SnapshotStatus.LOCKED); - getSnapshotDao().remove(previouslyActiveSnapshotId); + Snapshot previouslyActiveSnapshot = + getSnapshotDao().get(getVmId(), SnapshotType.PREVIEW, SnapshotStatus.LOCKED); + getSnapshotDao().remove(previouslyActiveSnapshot.getId()); getSnapshotDao().remove(getSnapshotDao().getId(getVmId(), SnapshotType.ACTIVE)); - snapshotsManager.addActiveSnapshot(previouslyActiveSnapshotId, getVm(), getCompensationContext()); + snapshotsManager.addActiveSnapshot(previouslyActiveSnapshot.getId(), getVm(), + previouslyActiveSnapshot.getMemoryVolume(), getCompensationContext()); super.endWithFailure(); } @@ -105,7 +109,7 @@ SnapshotStatus.OK); snapshotsManager.attempToRestoreVmConfigurationFromSnapshot(getVm(), - getSnapshotDao().get(getParameters().getDstSnapshotId()), + getDstSnapshot(), getSnapshotDao().getId(getVm().getId(), SnapshotType.ACTIVE), getCompensationContext(), getVm().getVdsGroupCompatibilityVersion()); } @@ -114,10 +118,13 @@ protected void executeVmCommand() { final Guid newActiveSnapshotId = Guid.NewGuid(); + final Snapshot snapshotToBePreviewed = getDstSnapshot(); final List<DiskImage> images = DbFacade .getInstance() .getDiskImageDao() - .getAllSnapshotsForVmSnapshot(getParameters().getDstSnapshotId()); + .getAllSnapshotsForVmSnapshot(snapshotToBePreviewed.getId()); + final boolean restoreMemory = getParameters().isRestoreMemory() && + FeatureSupported.memorySnapshot(getVm().getVdsGroupCompatibilityVersion()); TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override @@ -130,10 +137,14 @@ "Active VM before the preview", SnapshotType.PREVIEW, getVm(), + previousActiveSnapshot.getMemoryVolume(), getCompensationContext()); - snapshotsManager.addActiveSnapshot(newActiveSnapshotId, getVm(), getCompensationContext()); - //if there are no images there's no reason the save the compensation data to DB as - //the update is being executed in the same transaction so we can restore the vm config and end the command. + snapshotsManager.addActiveSnapshot(newActiveSnapshotId, + getVm(), + restoreMemory ? snapshotToBePreviewed.getMemoryVolume() : StringUtils.EMPTY, + getCompensationContext()); + // if there are no images there's no reason to save the compensation data to DB as the update is + // being executed in the same transaction so we can restore the vm config and end the command. if (!images.isEmpty()) { getCompensationContext().stateChanged(); } else { @@ -144,37 +155,41 @@ } }); - if (images.size() > 0) { + if (!images.isEmpty()) { VmHandler.LockVm(getVm().getDynamicData(), getCompensationContext()); freeLock(); TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { for (DiskImage image : images) { - ImagesContainterParametersBase tempVar = new ImagesContainterParametersBase(image.getImageId()); - tempVar.setParentCommand(VdcActionType.TryBackToAllSnapshotsOfVm); - tempVar.setVmSnapshotId(newActiveSnapshotId); - tempVar.setEntityId(getParameters().getEntityId()); - tempVar.setParentParameters(getParameters()); - tempVar.setQuotaId(image.getQuotaId()); - ImagesContainterParametersBase p = tempVar; VdcReturnValueBase vdcReturnValue = Backend.getInstance().runInternalAction(VdcActionType.TryBackToSnapshot, - p, + buildTryBackToSnapshotParameters(newActiveSnapshotId, image), ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); if (vdcReturnValue.getSucceeded()) { getTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList()); } else if (vdcReturnValue.getFault() != null) { // if we have a fault, forward it to the user - throw new VdcBLLException(vdcReturnValue.getFault().getError(), vdcReturnValue.getFault() - .getMessage()); + throw new VdcBLLException(vdcReturnValue.getFault().getError(), + vdcReturnValue.getFault().getMessage()); } else { log.error("Cannot create snapshot"); throw new VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL); } } return null; + } + + private ImagesContainterParametersBase buildTryBackToSnapshotParameters( + final Guid newActiveSnapshotId, DiskImage image) { + ImagesContainterParametersBase params = new ImagesContainterParametersBase(image.getImageId()); + params.setParentCommand(VdcActionType.TryBackToAllSnapshotsOfVm); + params.setVmSnapshotId(newActiveSnapshotId); + params.setEntityId(getParameters().getEntityId()); + params.setParentParameters(getParameters()); + params.setQuotaId(image.getQuotaId()); + return params; } }); } @@ -199,7 +214,7 @@ @Override protected boolean canDoAction() { - Snapshot snapshot = getSnapshotDao().get(getParameters().getDstSnapshotId()); + Snapshot snapshot = getDstSnapshot(); SnapshotsValidator snapshotsValidator = new SnapshotsValidator(); VmValidator vmValidator = new VmValidator(getVm()); if (!validate(vmValidator.vmDown()) @@ -248,6 +263,13 @@ return true; } + private Snapshot getDstSnapshot() { + if (cachedSnapshot == null) { + cachedSnapshot = getSnapshotDao().get(getParameters().getDstSnapshotId()); + } + return cachedSnapshot; + } + @Override protected void setActionMessageParameters() { addCanDoActionMessage(VdcBllMessages.VAR__ACTION__PREVIEW); @@ -272,7 +294,7 @@ @Override public String getSnapshotName() { if (super.getSnapshotName() == null) { - final Snapshot snapshot = getSnapshotDao().get(getParameters().getDstSnapshotId()); + final Snapshot snapshot = getDstSnapshot(); if (snapshot != null) { setSnapshotName(snapshot.getDescription()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java index 5428bfe..9af2216 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java @@ -62,6 +62,7 @@ * The VM to save the snapshot for. * @param compensationContext * Context for saving compensation details. + * @return the newly created snapshot */ public Snapshot addActiveSnapshot(Guid snapshotId, VM vm, @@ -69,6 +70,59 @@ return addActiveSnapshot(snapshotId, vm, SnapshotStatus.OK, + "", + compensationContext); + } + + /** + * Save an active snapshot for the VM, without saving the configuration.<br> + * The snapshot is created in status {@link SnapshotStatus#OK} by default. + * + * @see #addActiveSnapshot(Guid, VM, SnapshotStatus, CompensationContext) + * @param snapshotId + * The ID for the snapshot. + * @param vm + * The VM to save the snapshot for. + * @param snapshotStatus + * The initial status of the created snapshot + * @param compensationContext + * Context for saving compensation details. + * @return the newly created snapshot + */ + public Snapshot addActiveSnapshot(Guid snapshotId, + VM vm, + SnapshotStatus snapshotStatus, + final CompensationContext compensationContext) { + return addActiveSnapshot(snapshotId, + vm, + snapshotStatus, + "", + compensationContext); + } + + /** + * Save an active snapshot for the VM, without saving the configuration.<br> + * The snapshot is created in status {@link SnapshotStatus#OK} by default. + * + * @see #addActiveSnapshot(Guid, VM, SnapshotStatus, CompensationContext) + * @param snapshotId + * The ID for the snapshot. + * @param vm + * The VM to save the snapshot for. + * @param memoryVolume + * The memory state for the created snapshot + * @param compensationContext + * Context for saving compensation details. + * @return the newly created snapshot + */ + public Snapshot addActiveSnapshot(Guid snapshotId, + VM vm, + String memoryVolume, + final CompensationContext compensationContext) { + return addActiveSnapshot(snapshotId, + vm, + SnapshotStatus.OK, + memoryVolume, compensationContext); } @@ -88,6 +142,7 @@ public Snapshot addActiveSnapshot(Guid snapshotId, VM vm, SnapshotStatus snapshotStatus, + String memoryVolume, final CompensationContext compensationContext) { return addSnapshot(snapshotId, "Active VM", @@ -95,7 +150,7 @@ SnapshotType.ACTIVE, vm, false, - StringUtils.EMPTY, + memoryVolume, compensationContext); } @@ -112,6 +167,8 @@ * The snapshot type. * @param vm * The VM to save in configuration. + * @param memoryVolume + * the volume in which the snapshot's memory is stored * @param compensationContext * Context for saving compensation details. * @return the added snapshot @@ -120,9 +177,10 @@ String description, SnapshotType snapshotType, VM vm, + String memoryVolume, final CompensationContext compensationContext) { return addSnapshot(snapshotId, description, SnapshotStatus.LOCKED, - snapshotType, vm, true, StringUtils.EMPTY, compensationContext); + snapshotType, vm, true, memoryVolume, compensationContext); } /** diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/TryBackToAllSnapshotsOfVmParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/TryBackToAllSnapshotsOfVmParameters.java index ada761e..7fe21ae 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/TryBackToAllSnapshotsOfVmParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/TryBackToAllSnapshotsOfVmParameters.java @@ -5,16 +5,30 @@ public class TryBackToAllSnapshotsOfVmParameters extends VmOperationParameterBase implements java.io.Serializable { private static final long serialVersionUID = 1862924807826485840L; private Guid dstSnapshotId = Guid.Empty; + private boolean restoreMemory; + + public TryBackToAllSnapshotsOfVmParameters() { + } public TryBackToAllSnapshotsOfVmParameters(Guid vmId, Guid dstSnapshotId) { super(vmId); this.dstSnapshotId = dstSnapshotId; } + public TryBackToAllSnapshotsOfVmParameters(Guid vmId, Guid dstSnapshotId, boolean restoreMemory) { + this(vmId, dstSnapshotId); + this.restoreMemory = restoreMemory; + } + public Guid getDstSnapshotId() { return dstSnapshotId; } - public TryBackToAllSnapshotsOfVmParameters() { + public boolean isRestoreMemory() { + return restoreMemory; + } + + public void setRestoreMemory(boolean restoreMemory) { + this.restoreMemory = restoreMemory; } } -- To view, visit http://gerrit.ovirt.org/15679 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I42c1b7dd4fcdf12fba1943792a1442984796cf45 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
