Arik Hadas has uploaded a new change for review. Change subject: core: remove memory snapshot when its not needed ......................................................................
core: remove memory snapshot when its not needed This patch adds the removal of memory snapshot volumes in case it's not needed anymore: 1. when removing VM that has snapshots with memory 2. when snapshots with memory are removed because we revert (commit) to previous snapshot 3. on run VM when the VM reach to a point where the disks might not be synchronous with the memory state that was restored anymore (and the active snapshot is the only one that points to its memory) This patch also treat the restoration of snapshot's memory when running a VM that is previewing/being committed to a snapshot with memory in stateless mode. Change-Id: Ic087580475cd4fcfac969ddf0f69929fa247f86d Bug-Url: https://bugzilla.redhat.com/960931 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/dbscripts/snapshots_sp.sql M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java 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/RunVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java R backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDao.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDaoDbFacadeImpl.java 16 files changed, 251 insertions(+), 126 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/82/15682/1 diff --git a/backend/manager/dbscripts/snapshots_sp.sql b/backend/manager/dbscripts/snapshots_sp.sql index 30026b2..6a5878b 100644 --- a/backend/manager/dbscripts/snapshots_sp.sql +++ b/backend/manager/dbscripts/snapshots_sp.sql @@ -327,3 +327,15 @@ END; $procedure$ LANGUAGE plpgsql; +Create or replace FUNCTION UpdateMemoryOfSnapshot( + v_snapshot_id UUID, + v_memory_volume VARCHAR(255)) +RETURNS VOID +AS $procedure$ +BEGIN + UPDATE snapshots + SET memory_volume = v_memory_volume + WHERE snapshot_id = v_snapshot_id; +END; $procedure$ +LANGUAGE plpgsql; + diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index c89f5b1..c70a1a8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -716,7 +716,7 @@ protected void removeVmRelatedEntitiesFromDb() { removeVmUsers(); removeVmNetwork(); - new SnapshotsManager().removeSnapshots(getVmId()); + removeVmSnapshots(); removeVmStatic(); } 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 10f6832..9a7031e 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 @@ -9,9 +9,10 @@ import org.apache.commons.lang.exception.ExceptionUtils; import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.job.ExecutionHandler; -import org.ovirt.engine.core.bll.memory.DefaultMemoryImageBuilder; +import org.ovirt.engine.core.bll.memory.LiveSnapshotMemoryImageBuilder; import org.ovirt.engine.core.bll.memory.MemoryImageBuilder; import org.ovirt.engine.core.bll.memory.NullableMemoryImageBuilder; +import org.ovirt.engine.core.bll.memory.StatelessSnapshotMemoryImageBuilder; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; @@ -146,14 +147,20 @@ } private MemoryImageBuilder createMemoryImageBuilder() { - if (FeatureSupported.memorySnapshot(getVm().getVdsGroupCompatibilityVersion()) - && getParameters().isSaveMemory() && isLiveSnapshotApplicable()) { - prepareForMemorySnapshot = true; - return new DefaultMemoryImageBuilder(getVm(), getStorageDomainIdForVmMemory(), getStoragePool(), this); - } - else { + if (!FeatureSupported.memorySnapshot(getVm().getVdsGroupCompatibilityVersion())) { return new NullableMemoryImageBuilder(); } + + if (getParameters().getSnapshotType() == SnapshotType.STATELESS) { + return new StatelessSnapshotMemoryImageBuilder(getVm()); + } + + if (getParameters().isSaveMemory() && isLiveSnapshotApplicable()) { + prepareForMemorySnapshot = true; + return new LiveSnapshotMemoryImageBuilder(getVm(), getStorageDomainIdForVmMemory(), getStoragePool(), this); + } + + return new NullableMemoryImageBuilder(); } private Snapshot addSnapshotToDB(Guid snapshotId, MemoryImageBuilder memoryImageBuilder) { @@ -223,10 +230,32 @@ if (isLiveSnapshotApplicable()) { performLiveSnapshot(createdSnapshot); } - // TODO: else remove the memory image if exists + else { + // If the created snapshot contains memory, remove the memory volumes as + // they are not in use since no live snapshot was created + String memoryVolume = createdSnapshot.getMemoryVolume(); + if (!memoryVolume.isEmpty() && + getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1) { + boolean succeed = removeMemoryVolumes(memoryVolume, getActionType(), false); + if (!succeed) { + log.errorFormat("Failed to remove unused memory {0} of snapshot {1}", + memoryVolume, createdSnapshot.getId()); + } + } + } } else { revertToActiveSnapshot(createdSnapshot.getId()); - // TODO: remove the memory image if exists + // If the removed snapshot contained memory, remove the memory volumes + // Note that the memory volumes might not have been created + String memoryVolume = createdSnapshot.getMemoryVolume(); + if (!memoryVolume.isEmpty() && + getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1) { + boolean succeed = removeMemoryVolumes(memoryVolume, getActionType(), false); + if (!succeed) { + log.warnFormat("Failed to remove memory {0} of snapshot {1}", + memoryVolume, createdSnapshot.getId()); + } + } } incrementVmGeneration(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java index f61dfac..4b7a6f9 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java @@ -23,7 +23,6 @@ import org.ovirt.engine.core.common.action.RemoveSnapshotParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; -import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus; @@ -31,18 +30,12 @@ import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; -import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; -import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; -import org.ovirt.engine.core.common.vdscommands.VDSCommandType; -import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.SnapshotDao; -import org.ovirt.engine.core.utils.GuidUtils; import org.ovirt.engine.core.utils.collections.MultiValueMapUtils; -import org.ovirt.engine.core.utils.linq.LinqUtils; -import org.ovirt.engine.core.utils.linq.Predicate; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; @@ -123,60 +116,10 @@ setSucceeded(true); } - /** - * There is a one to many relation between memory volumes and snapshots, so memory - * volumes should be removed only if the only snapshot that points to them is removed - */ - private boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume) { - return !memoryVolume.isEmpty() && - getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1; - } - private void removeMemory(final Snapshot snapshot) { - List<Guid> guids = GuidUtils.getGuidListFromString(snapshot.getMemoryVolume()); - - // get all vm disks in order to check post zero - if one of the - // disks is marked with wipe_after_delete - boolean postZero = - LinqUtils.filter(getDiskDao().getAllForVm(getVm().getId()), - new Predicate<Disk>() { - @Override - public boolean eval(Disk disk) { - return disk.isWipeAfterDelete(); - } - }).size() > 0; - - // delete first image - // the next 'DeleteImageGroup' command should also take care of the - // image removal: - VDSReturnValue vdsRetValue = runVdsCommand( - VDSCommandType.DeleteImageGroup, - new DeleteImageGroupVDSCommandParameters(guids.get(1), guids.get(0), guids.get(2), - postZero, false, getVm().getVdsGroupCompatibilityVersion().toString())); - - if (!vdsRetValue.getSucceeded()) { - log.errorFormat("Cannot remove memory dump volume for snapshot {0}", snapshot.getId()); - } - else { - Guid guid = - createTask(vdsRetValue.getCreationInfo(), VdcActionType.RemoveSnapshot, VdcObjectType.Storage, guids.get(0)); - getReturnValue().getTaskIdList().add(guid); - } - - // delete second image - // the next 'DeleteImageGroup' command should also take care of the image removal: - vdsRetValue = runVdsCommand( - VDSCommandType.DeleteImageGroup, - new DeleteImageGroupVDSCommandParameters(guids.get(1), guids.get(0), guids.get(4), - postZero, false, getVm().getVdsGroupCompatibilityVersion().toString())); - - if (!vdsRetValue.getSucceeded()) { - log.errorFormat("Cannot remove memory metadata volume for snapshot {0}", snapshot.getId()); - } - else { - Guid guid = - createTask(vdsRetValue.getCreationInfo(), VdcActionType.RemoveSnapshot, VdcObjectType.Storage, guids.get(0)); - getReturnValue().getTaskIdList().add(guid); + boolean success = removeMemoryVolumes(snapshot.getMemoryVolume(), getActionType(), false); + if (!success) { + log.errorFormat("Cannot remove memory volumes for snapshot {0}", snapshot.getId()); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java index b696746..cb6539d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java @@ -12,7 +12,6 @@ import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; -import org.ovirt.engine.core.bll.snapshots.SnapshotsManager; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.utils.PermissionSubject; @@ -249,7 +248,7 @@ removeLunDisks(); removeVmUsers(); removeVmNetwork(); - new SnapshotsManager().removeSnapshots(getVmId()); + removeVmSnapshots(); removeVmStatic(); } 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 3a85d1a..b9f1508 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 @@ -125,6 +125,15 @@ protected void removeSnapshotsFromDB() { for (Guid snapshotId : snapshotsToRemove) { + String memoryVolume = getSnapshotDao().get(snapshotId).getMemoryVolume(); + if (!memoryVolume.isEmpty() && + getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1) { + boolean succeed = removeMemoryVolumes(memoryVolume, getActionType(), false); + if (!succeed) { + log.errorFormat("Failed to remove memory {0} of snapshot {1}", + memoryVolume, snapshotId); + } + } getSnapshotDao().remove(snapshotId); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java index 1706e50..82a7301 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java @@ -63,8 +63,10 @@ import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; import org.ovirt.engine.core.dal.job.ExecutionMessageDirector; +import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.utils.NetworkUtils; import org.ovirt.engine.core.utils.linq.LinqUtils; import org.ovirt.engine.core.utils.linq.Predicate; @@ -81,10 +83,16 @@ private String _cdImagePath = ""; private String _floppyImagePath = ""; private boolean mResume; - private boolean _isVmRunningStateless; + /** Note: this field should not be used directly, use {@link #isVmRunningStateless()} instead */ + private Boolean cachedVmIsRunningStateless; private boolean isFailedStatlessSnapshot; /** Indicates whether restoration of memory from snapshot is supported for the VM */ private boolean memorySnapshotSupported; + /** The memory volume which is stored in the active snapshot of the VM */ + private String memoryVolumeFromSnapshot = StringUtils.EMPTY; + /** This flag is used to indicate that the disks might be dirty since the memory + * from the active snapshot was restored so the memory should not be used */ + private boolean memoryFromSnapshotIrrelevant; protected RunVmCommand(Guid commandId) { super(commandId); @@ -135,8 +143,23 @@ if (getVm().getStatus() != VMStatus.Suspended) { memorySnapshotSupported = FeatureSupported.memorySnapshot(getVm().getVdsGroupCompatibilityVersion()); + // If the VM is not hibernated, save the hibernation volume from the baseline snapshot + memoryVolumeFromSnapshot = getBaselineSnapshot().getMemoryVolume(); } } + } + + /** + * Returns the snapshot the vm is based on - either the active snapshot (usually) or + * the stateless snapshot in case the VM is running in stateless mode + */ + private Snapshot getBaselineSnapshot() { + return getSnapshotDao().get(getVm().getId(), + isVmRunningStateless() ? SnapshotType.STATELESS : SnapshotType.ACTIVE); + } + + private SnapshotDao getSnapshotDao() { + return DbFacade.getInstance().getSnapshotDao(); } /** @@ -234,9 +257,8 @@ if (status != null && (status.isRunning() || status == VMStatus.RestoringState)) { setSucceeded(true); } else { - // Try to rerun Vm on different vds - // no need to log the command because it is being logged inside - // the rerun + // Try to rerun Vm on different vds no need to log the command because it is + // being logged inside the rerun log.infoFormat("Failed to run desktop {0}, rerun", getVm().getName()); setCommandShouldBeLogged(false); setSucceeded(true); @@ -254,8 +276,8 @@ @Override protected void executeVmCommand() { - // Before running the VM we update its devices, as they may need to be changed due to configuration option - // change + // Before running the VM we update its devices, as they may need to be changed due to + // configuration option change VmDeviceUtils.updateVmDevices(getVm().getStaticData()); setActionReturnValue(VMStatus.Down); if (initVm()) { @@ -274,7 +296,7 @@ } } else if (!isInternalExecution() && !_isRerun && getVm().getStatus() != VMStatus.Suspended - && statelessSnapshotExistsForVm() + && isStatelessSnapshotExistsForVm() && !isVMPartOfManualPool()) { removeVmStatlessImages(); } else { @@ -286,8 +308,8 @@ } } - private boolean statelessSnapshotExistsForVm() { - return getDbFacade().getSnapshotDao().exists(getVm().getId(), SnapshotType.STATELESS); + private boolean isStatelessSnapshotExistsForVm() { + return getSnapshotDao().exists(getVm().getId(), SnapshotType.STATELESS); } /** @@ -324,7 +346,7 @@ private void statelessVmTreatment() { warnIfNotAllDisksPermitSnapshots(); - if (statelessSnapshotExistsForVm()) { + if (isStatelessSnapshotExistsForVm()) { log.errorFormat( "RunVmAsStateless - {0} - found existing vm images in stateless_vm_image_map table - skipped creating snapshots.", getVm().getName()); @@ -444,34 +466,38 @@ VMStatus vmStatus = (VMStatus) getBackend() .getResourceManager() - .RunAsyncVdsCommand(VDSCommandType.CreateVm, initVdsCreateVmParams(), this).getReturnValue(); + .RunAsyncVdsCommand(VDSCommandType.CreateVm, initCreateVmParams(), this).getReturnValue(); + + // Don't use the memory from the active snapshot anymore if there's a chance that disks were changed + memoryFromSnapshotIrrelevant = vmStatus.isRunning() || vmStatus == VMStatus.RestoringState; // After VM was create (or not), we can remove the quota vds group memory. return vmStatus; } + /** - * Initial the parameters for the VDSM command of VM creation + * Initialize the parameters for the VDSM command of VM creation * @return the VDS create VM parameters */ - protected CreateVmVDSCommandParameters initVdsCreateVmParams() { + protected CreateVmVDSCommandParameters initCreateVmParams() { VM vmToBeCreated = getVm(); if (vmToBeCreated.getStatus() == VMStatus.Suspended) { return new CreateVmVDSCommandParameters(getVdsId(), vmToBeCreated); } - if (!memorySnapshotSupported) { + if (!memorySnapshotSupported || memoryFromSnapshotIrrelevant) { vmToBeCreated.setHibernationVolHandle(StringUtils.EMPTY); return new CreateVmVDSCommandParameters(getVdsId(), vmToBeCreated); } // otherwise, use the memory that is saved on the active snapshot (might be empty) - Snapshot activeSnapshotOfVmToBeCreated = - getDbFacade().getSnapshotDao().get(vmToBeCreated.getId(), SnapshotType.ACTIVE); - vmToBeCreated.setHibernationVolHandle(activeSnapshotOfVmToBeCreated.getMemoryVolume()); + vmToBeCreated.setHibernationVolHandle(memoryVolumeFromSnapshot); CreateVmVDSCommandParameters parameters = new CreateVmVDSCommandParameters(getVdsId(), vmToBeCreated); + // Mark that the hibernation volume should be cleared from the VM right after the sync part of + // the create verb is finished (unlike hibernation volume that is created by hibernate command) parameters.setClearHibernationVolumes(true); return parameters; } @@ -487,7 +513,7 @@ return getSucceeded() ? AuditLogType.USER_RESUME_VM : AuditLogType.USER_FAILED_RESUME_VM; } else if (isInternalExecution()) { return getSucceeded() ? - (statelessSnapshotExistsForVm() ? AuditLogType.VDS_INITIATED_RUN_VM_AS_STATELESS + (isStatelessSnapshotExistsForVm() ? AuditLogType.VDS_INITIATED_RUN_VM_AS_STATELESS : AuditLogType.VDS_INITIATED_RUN_VM) : AuditLogType.VDS_INITIATED_RUN_VM_FAILED; } else { @@ -497,7 +523,7 @@ && getVm().getDedicatedVmForVds() != null && !getVm().getRunOnVds().equals(getVm().getDedicatedVmForVds()) ? AuditLogType.USER_RUN_VM_ON_NON_DEFAULT_VDS : - (statelessSnapshotExistsForVm() ? AuditLogType.USER_RUN_VM_AS_STATELESS : AuditLogType.USER_RUN_VM) + (isStatelessSnapshotExistsForVm() ? AuditLogType.USER_RUN_VM_AS_STATELESS : AuditLogType.USER_RUN_VM) : _isRerun ? AuditLogType.VDS_INITIATED_RUN_VM : getTaskIdList().size() > 0 ? @@ -510,13 +536,13 @@ // if not running as stateless, or if succeeded running as // stateless, // command should be with 'CommandShouldBeLogged = false': - return _isVmRunningStateless && !getSucceeded() ? AuditLogType.USER_RUN_VM_AS_STATELESS_FINISHED_FAILURE + return isVmRunningStateless() && !getSucceeded() ? AuditLogType.USER_RUN_VM_AS_STATELESS_FINISHED_FAILURE : AuditLogType.UNASSIGNED; case END_FAILURE: // if not running as stateless, command should // be with 'CommandShouldBeLogged = false': - return _isVmRunningStateless ? AuditLogType.USER_RUN_VM_AS_STATELESS_FINISHED_FAILURE + return isVmRunningStateless() ? AuditLogType.USER_RUN_VM_AS_STATELESS_FINISHED_FAILURE : AuditLogType.UNASSIGNED; default: @@ -746,9 +772,7 @@ @Override protected void endSuccessfully() { - setIsVmRunningStateless(); - - if (_isVmRunningStateless) { + if (isVmRunningStateless()) { CreateAllSnapshotsFromVmParameters createSnapshotParameters = buildCreateSnapshotParameters(); createSnapshotParameters.setImagesParameters(getParameters().getImagesParameters()); getBackend().EndAction(VdcActionType.CreateAllSnapshotsFromVm, createSnapshotParameters); @@ -781,8 +805,7 @@ .runInternalAction(getActionType(), getParameters(), new CommandContext(runStatelessVmCtx)) .getSucceeded()); if (!getSucceeded()) { - // could not run the vm don't try to run the end action - // again + // could not run the vm don't try to run the end action again log.warnFormat("Could not run the vm {0} on RunVm.EndSuccessfully", getVm().getName()); getReturnValue().setEndActionTryAgain(false); } @@ -796,8 +819,7 @@ @Override protected void endWithFailure() { - setIsVmRunningStateless(); - if (_isVmRunningStateless) { + if (isVmRunningStateless()) { CreateAllSnapshotsFromVmParameters createSnapshotParameters = buildCreateSnapshotParameters(); createSnapshotParameters.setImagesParameters(getParameters().getImagesParameters()); VdcReturnValueBase vdcReturnValue = getBackend().endAction(VdcActionType.CreateAllSnapshotsFromVm, @@ -813,8 +835,37 @@ } } - private void setIsVmRunningStateless() { - _isVmRunningStateless = statelessSnapshotExistsForVm(); + @Override + public void runningSucceded() { + removeMemoryFromSnapshot(); + super.runningSucceded(); + } + + @Override + protected void failedToRunVm() { + if (memoryFromSnapshotIrrelevant) { + removeMemoryFromSnapshot(); + } + super.failedToRunVm(); + } + + private void removeMemoryFromSnapshot() { + if (memoryVolumeFromSnapshot.isEmpty()) { + return; + } + + // If the active snapshot is the only one that points to the memory volume we can remove it + if (getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolumeFromSnapshot) == 1) { + removeMemoryVolumes(memoryVolumeFromSnapshot, getActionType(), true); + } + getSnapshotDao().removeMemoryFromSnapshot(getBaselineSnapshot().getId()); + } + + private boolean isVmRunningStateless() { + if (cachedVmIsRunningStateless == null) { + cachedVmIsRunningStateless = isStatelessSnapshotExistsForVm(); + } + return cachedVmIsRunningStateless; } /** diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java index 62478a7..baa8fe7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java @@ -9,6 +9,7 @@ import java.util.Map; import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; + import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.job.ExecutionContext.ExecutionMethod; @@ -223,7 +224,7 @@ getVm().setLastVdsRunOn(getCurrentVdsId()); } if (StringUtils.isNotEmpty(getVm().getHibernationVolHandle())) { - handleHibernatedVm(getActionType(), true); + removeMemoryVolumes(getVm().getHibernationVolHandle(), getActionType(), true); // In order to prevent a race where VdsUpdateRuntimeInfo saves the Vm Dynamic as UP prior to execution of // this method (which is a part of the cached VM command, // so the state this method is aware to is RESTORING, in case of RunVmCommand after the VM got suspended. diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java index 7abb99c..230fdb7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java @@ -6,8 +6,8 @@ import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; -import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; +import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; import org.ovirt.engine.core.common.action.RunVmOnceParams; import org.ovirt.engine.core.common.action.RunVmParams; import org.ovirt.engine.core.common.action.SysPrepParams; @@ -57,9 +57,9 @@ } @Override - protected CreateVmVDSCommandParameters initVdsCreateVmParams() { + protected CreateVmVDSCommandParameters initCreateVmParams() { getVm().setRunOnce(true); - CreateVmVDSCommandParameters createVmParams = super.initVdsCreateVmParams(); + CreateVmVDSCommandParameters createVmParams = super.initCreateVmParams(); SysPrepParams sysPrepParams = new SysPrepParams(); RunVmOnceParams runOnceParams = getParameters(); sysPrepParams.setSysPrepDomainName(runOnceParams.getSysPrepDomainName()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java index a2e06e3..9462562 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java @@ -113,7 +113,7 @@ // Set the VM to image locked to decrease race condition. updateVmStatus(VMStatus.ImageLocked); if (StringUtils.isNotEmpty(getVm().getHibernationVolHandle()) - && handleHibernatedVm(getActionType(), false)) { + && removeMemoryVolumes(getVm().getHibernationVolHandle(), getActionType(), false)) { returnVal = true; } else { updateVmStatus(vmStatus); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java index de8f451..b9f4080 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java @@ -1,10 +1,11 @@ package org.ovirt.engine.core.bll; import java.util.ArrayList; -import java.util.LinkedList; +import java.util.Collection; import java.util.List; import org.ovirt.engine.core.bll.network.MacPoolManager; +import org.ovirt.engine.core.bll.snapshots.SnapshotsManager; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.VdcActionParametersBase; @@ -165,6 +166,25 @@ } } + protected void removeVmSnapshots() { + Collection<String> memoriesOfRemovedSnapshots = + new SnapshotsManager().removeSnapshots(getVmId()); + for (String memoryVolumes : memoriesOfRemovedSnapshots) { + if (shouldRemoveMemorySnapshotVolumes(memoryVolumes)) { + removeMemoryVolumes(memoryVolumes, getActionType(), false); + } + } + } + + /** + * There is a one to many relation between memory volumes and snapshots, so memory + * volumes should be removed only if the only snapshot that points to them is removed + */ + protected boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume) { + return !memoryVolume.isEmpty() && + getDbFacade().getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1; + } + protected void removeVmUsers() { List<TagsVmMap> all = getTagDao().getTagVmMapByVmIdAndDefaultTag(getVmId()); for (TagsVmMap tagVm : all) { @@ -232,15 +252,15 @@ return VdcActionType.Unknown; } - protected boolean handleHibernatedVm(VdcActionType parentCommand, boolean startPollingTasks) { + protected boolean removeMemoryVolumes(String memVols, VdcActionType parentCommand, boolean startPollingTasks) { // this is temp code until it will be implemented in SPM - String[] strings = getVm().getHibernationVolHandle().split(","); - List<Guid> guids = new LinkedList<Guid>(); - for (String string : strings) { - guids.add(new Guid(string)); + String[] strings = memVols.split(","); + Guid[] guids = new Guid[strings.length]; + for (int i=0; i<strings.length; ++i) { + guids[i] = new Guid(strings[i]); } - Guid[] imagesList = guids.toArray(new Guid[0]); - if (imagesList.length == 6) { + + if (guids.length == 6) { // get all vm disks in order to check post zero - if one of the // disks is marked with wipe_after_delete boolean postZero = @@ -253,11 +273,10 @@ }).size() > 0; // delete first image - // the next 'DeleteImageGroup' command should also take care of the - // image removal: + // the next 'DeleteImageGroup' command should also take care of the image removal: VDSReturnValue vdsRetValue1 = runVdsCommand( VDSCommandType.DeleteImageGroup, - new DeleteImageGroupVDSCommandParameters(imagesList[1], imagesList[0], imagesList[2], + new DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[2], postZero, false, getVm().getVdsGroupCompatibilityVersion().toString())); if (!vdsRetValue1.getSucceeded()) { @@ -265,15 +284,14 @@ } Guid guid1 = - createTask(vdsRetValue1.getCreationInfo(), parentCommand, VdcObjectType.Storage, imagesList[0]); + createTask(vdsRetValue1.getCreationInfo(), parentCommand, VdcObjectType.Storage, guids[0]); getTaskIdList().add(guid1); // delete second image - // the next 'DeleteImageGroup' command should also take care of the - // image removal: + // the next 'DeleteImageGroup' command should also take care of the image removal: VDSReturnValue vdsRetValue2 = runVdsCommand( VDSCommandType.DeleteImageGroup, - new DeleteImageGroupVDSCommandParameters(imagesList[1], imagesList[0], imagesList[4], + new DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[4], postZero, false, getVm().getVdsGroupCompatibilityVersion().toString())); if (!vdsRetValue2.getSucceeded()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/DefaultMemoryImageBuilder.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java similarity index 96% rename from backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/DefaultMemoryImageBuilder.java rename to backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java index add296c..eb8ff90 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/DefaultMemoryImageBuilder.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java @@ -18,7 +18,7 @@ /** * This builder creates the memory images for live snapshots with memory operation */ -public class DefaultMemoryImageBuilder implements MemoryImageBuilder { +public class LiveSnapshotMemoryImageBuilder implements MemoryImageBuilder { private Guid storageDomainId; private Guid memoryDumpImageGroupId; private Guid memoryDumpVolumeId; @@ -28,7 +28,7 @@ private TaskHandlerCommand<?> enclosingCommand; private StoragePool storagePool; - public DefaultMemoryImageBuilder(VM vm, Guid storageDomainId, + public LiveSnapshotMemoryImageBuilder(VM vm, Guid storageDomainId, StoragePool storagePool, TaskHandlerCommand<?> enclosingCommand) { this.vm = vm; this.enclosingCommand = enclosingCommand; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java new file mode 100644 index 0000000..ae349f1 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java @@ -0,0 +1,32 @@ +package org.ovirt.engine.core.bll.memory; + +import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; + +/** + * This builder is responsible to create the memory volumes for stateless snapshot - + * it just take the memory volume of the previously active snapshot + */ +public class StatelessSnapshotMemoryImageBuilder implements MemoryImageBuilder { + + private final String memoryVolumesFromActiveSnapshot; + + public StatelessSnapshotMemoryImageBuilder(VM vm) { + memoryVolumesFromActiveSnapshot = + DbFacade.getInstance().getSnapshotDao().get(vm.getId(),SnapshotType.ACTIVE) + .getMemoryVolume(); + } + + public void build() { + //no op + } + + public String getVolumeStringRepresentation() { + return memoryVolumesFromActiveSnapshot; + } + + public boolean isCreateTasks() { + return false; + } +} 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 9af2216..797c635 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 @@ -2,7 +2,9 @@ import java.util.ArrayList; import java.util.Date; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.ImagesHandler; @@ -257,11 +259,15 @@ * * @param vmId * The ID of the VM. + * @return Set of memoryVolumes of the removed snapshots */ - public void removeSnapshots(Guid vmId) { + public Set<String> removeSnapshots(Guid vmId) { + Set<String> memoryVolumes = new HashSet<String>(); for (Snapshot snapshot : getSnapshotDao().getAll(vmId)) { + memoryVolumes.add(snapshot.getMemoryVolume()); getSnapshotDao().remove(snapshot.getId()); } + return memoryVolumes; } /** diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDao.java index 33bde37..74ece58 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDao.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDao.java @@ -163,5 +163,20 @@ */ boolean exists(Guid vmId, Guid snapshotId); + /** + * Get the number of snapshots that contain the given memory + * + * @param memoryVolume + * The memory that should be used to filter the snapshots + * @return Number of snapshots containing the given memory + */ int getNumOfSnapshotsByMemory(String memoryVolume); + + /** + * Clear the memory from the snapshot with the given id + * + * @param snapshotId + * The id of the snapshot that its memory should be cleared + */ + void removeMemoryFromSnapshot(Guid snapshotId); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDaoDbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDaoDbFacadeImpl.java index d3e7972..96b9987 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDaoDbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/SnapshotDaoDbFacadeImpl.java @@ -223,6 +223,16 @@ parameterSource); } + @Override + public void removeMemoryFromSnapshot(Guid snapshotId) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() + .addValue("snapshot_id", snapshotId) + .addValue("memory_volume", null); + + getCallsHandler().executeModification("UpdateMemoryOfSnapshot", + parameterSource); + } + private String getNullableRepresentation(String memoryVolume) { return memoryVolume.isEmpty() ? null : memoryVolume; } -- To view, visit http://gerrit.ovirt.org/15682 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic087580475cd4fcfac969ddf0f69929fa247f86d 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
