Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set VMs that preview memory snapshots to suspended #516

Merged
merged 3 commits into from
Jul 13, 2022
Merged

Set VMs that preview memory snapshots to suspended #516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
010d09f
core: drop IsMemorySnapshotSupported since it is always 'true'
ahadas Jun 23, 2022
2de1544
core: drop TryBackToAllSnapshotsOfVmCommand#isRestoreMemory
ahadas Jul 5, 2022
6ce8656
core: set vms that preview memory snapshots to suspended
ahadas Jul 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ...modules/bll/src/main/java/org/ovirt/engine/core/bll/GetArchitectureCapabilitiesQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ private static boolean isSupported(ArchCapabilitiesVerb archCapabilitiesVerb,
switch (archCapabilitiesVerb) {
case GetMigrationSupport:
return FeatureSupported.isMigrationSupported(architecture, version);
case GetMemorySnapshotSupport:
return FeatureSupported.isMemorySnapshotSupportedByArchitecture(architecture, version);
case GetSuspendSupport:
return FeatureSupported.isSuspendSupportedByArchitecture(architecture, version);
case GetMemoryHotUnplugSupport:
Expand Down
9 changes: 9 additions & 0 deletions backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.ovirt.engine.core.bll.memory.MemoryDisks;
import org.ovirt.engine.core.bll.memory.MemoryStorageHandler;
import org.ovirt.engine.core.bll.memory.MemoryUtils;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.bll.storage.disk.image.DisksFilter;
import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil;
import org.ovirt.engine.core.bll.validator.VmValidator;
Expand Down Expand Up @@ -60,6 +61,8 @@ public class HibernateVmCommand<T extends VmOperationParameterBase> extends VmOp
private SnapshotDao snapshotDao;
@Inject
private CommandCoordinatorUtil commandCoordinatorUtil;
@Inject
protected SnapshotsValidator snapshotsValidator;

/**
* Constructor for command creation when compensation is applied on startup
Expand Down Expand Up @@ -231,6 +234,12 @@ protected boolean validate() {
return false;
}

// we block hibernating a vm in preview mode because there is a bug that
// would result in keeping the hibernation metadata volume
if (!validate(snapshotsValidator.vmNotInPreview(getVm().getId()))) {
return false;
}

return true;
}

Expand Down
7 changes: 1 addition & 6 deletions backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,7 @@ protected List<Guid> getPredefinedVdsIdListToRunOn() {
protected boolean shouldRestoreMemory() {
// If the memory from the snapshot could have been restored already, the disks might be
// non coherent with the memory, thus we don't want to try to restore the memory again
return !memoryFromSnapshotUsed &&
(getFlow() == RunVmFlow.RESUME_HIBERNATE ||
FeatureSupported.isMemorySnapshotSupportedByArchitecture(
getVm().getClusterArch(),
getVm().getCompatibilityVersion())) &&
getActiveSnapshot().containsMemory();
return !memoryFromSnapshotUsed && getActiveSnapshot().containsMemory();
ahadas marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
2 changes: 1 addition & 1 deletion ...bll/src/main/java/org/ovirt/engine/core/bll/snapshots/CreateLiveSnapshotForVmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ private SnapshotVDSCommandParameters buildLiveSnapshotParameters(Snapshot snapsh
List<Disk> pluggedDisksForVm = diskDao.getAllForVm(getVm().getId(), true);
List<DiskImage> filteredPluggedDisksForVm = DisksFilter.filterImageDisks(pluggedDisksForVm,
ONLY_SNAPABLE, ONLY_ACTIVE);
boolean memoryDump = getParameters().isMemorySnapshotSupported() && snapshot.containsMemory();
boolean memoryDump = snapshot.containsMemory();

// 'filteredPluggedDisks' should contain only disks from 'getDisksList()' that are plugged to the VM.
List<DiskImage> filteredPluggedDisks = ImagesHandler.imagesIntersection(filteredPluggedDisksForVm, getParameters().getCachedSelectedActiveDisks());
Expand Down
17 changes: 1 addition & 16 deletions ...les/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/CreateSnapshotForVmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ public class CreateSnapshotForVmCommand<T extends CreateSnapshotForVmParameters>
private List<DiskImage> cachedSelectedActiveDisks;
private List<DiskImage> cachedImagesDisks;
private boolean isLiveSnapshotApplicable;
private boolean isMemorySnapshotSupported;
private boolean shouldFreezeOrThaw;

@Inject
Expand Down Expand Up @@ -129,7 +128,6 @@ public void init() {
getParameters().setEntityInfo(new EntityInfo(VdcObjectType.VM, getVmId()));
setSnapshotName(getParameters().getDescription());
setStoragePoolId(getVm() != null ? getVm().getStoragePoolId() : null);
isMemorySnapshotSupported = isMemorySnapshotSupported();
isLiveSnapshotApplicable = isLiveSnapshotApplicable();
shouldFreezeOrThaw = shouldFreezeOrThawVm();
}
Expand Down Expand Up @@ -410,7 +408,6 @@ private CreateSnapshotForVmParameters createLiveSnapshotParameters(final Snapsho
params.setLegacyFlow(true);
}
params.setCachedSelectedActiveDisks(cachedSelectedActiveDisks);
params.setMemorySnapshotSupported(isMemorySnapshotSupported);
params.setParentLiveMigrateDisk(getParameters().getParentCommand() == ActionType.LiveMigrateDisk);
return params;
}
Expand Down Expand Up @@ -679,7 +676,7 @@ private boolean shouldFreezeOrThawVm() {
return false; // only relevant for live snapshots
}

if (isMemorySnapshotSupported && getParameters().isSaveMemory()) {
if (getParameters().isSaveMemory()) {
return false; // irrelevant for snapshots that contain memory
}

Expand All @@ -704,10 +701,6 @@ private boolean diskOfTypeExists(DiskStorageType type) {
}

private MemoryImageBuilder createMemoryImageBuilder() {
if (!isMemorySnapshotSupported) {
return new NullableMemoryImageBuilder();
}

if (getParameters().getSnapshotType() == Snapshot.SnapshotType.STATELESS) {
return new StatelessSnapshotMemoryImageBuilder(getVm());
}
Expand All @@ -721,14 +714,6 @@ private MemoryImageBuilder createMemoryImageBuilder() {
return new NullableMemoryImageBuilder();
}

/**
* Check if Memory Snapshot is supported
*/
private boolean isMemorySnapshotSupported() {
return FeatureSupported.isMemorySnapshotSupportedByArchitecture(
getVm().getClusterArch(), getVm().getCompatibilityVersion());
}

private Guid updateActiveSnapshotId() {
final Snapshot activeSnapshot = snapshotDao.get(getVmId(), Snapshot.SnapshotType.ACTIVE);
final Guid activeSnapshotId = activeSnapshot.getId();
Expand Down
10 changes: 6 additions & 4 deletions ...les/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/RestoreAllSnapshotsCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
import org.ovirt.engine.core.common.businessentities.SnapshotActionEnum;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmCheckpoint;
import org.ovirt.engine.core.common.businessentities.storage.CinderDisk;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
Expand Down Expand Up @@ -142,13 +143,14 @@ public RestoreAllSnapshotsCommand(T parameters, CommandContext commandContext) {

@Override
protected LockProperties applyLockProperties(LockProperties lockProperties) {
return lockProperties.withScope(Scope.Execution);
return lockProperties.withScope(Scope.Command);
}

@Override
protected void executeVmCommand() {

if (!getImagesList().isEmpty()) {
// we can probably get rid of the following check getVm().getStatus() != VMStatus.Suspended
// and rely on memory locks instead of db locks but that requires more testing
if (!getImagesList().isEmpty() && getVm().getStatus() != VMStatus.Suspended) {
lockVmWithCompensationIfNeeded();
if (!isInternalExecution()) {
freeLock();
Expand Down Expand Up @@ -843,7 +845,7 @@ protected boolean validate() {
MultipleStorageDomainsValidator storageValidator = createStorageDomainValidator();
if (!validate(storageValidator.allDomainsExistAndActive()) ||
!performImagesChecks() ||
!validate(vmValidator.vmDown()) ||
!validate(vmValidator.validateVmStatusUsingMatrix(getActionType())) ||
!validate(storageValidator.isSupportedByManagedBlockStorageDomains(getActionType())) ||
// if the user choose to commit a snapshot the vm can't have disk snapshots attached to other vms.
getSnapshot().getType() == SnapshotType.REGULAR && !validate(vmValidator.vmNotHavingDeviceSnapshotsAttachedToOtherVms(false))) {
Expand Down
6 changes: 5 additions & 1 deletion ...nager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus;
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmDevice;
import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
import org.ovirt.engine.core.common.businessentities.VmDeviceId;
import org.ovirt.engine.core.common.businessentities.VmDynamic;
import org.ovirt.engine.core.common.businessentities.VmStatic;
import org.ovirt.engine.core.common.businessentities.VmTemplate;
import org.ovirt.engine.core.common.businessentities.aaa.DbUser;
Expand Down Expand Up @@ -455,7 +457,9 @@ public void attemptToRestoreVmConfigurationFromSnapshot(VM vm,
}

vm.setAppList(snapshot.getAppList());
vmDynamicDao.update(vm.getDynamicData());
VmDynamic vmDynamic = vm.getDynamicData();
vmDynamic.setStatus(withMemory ? VMStatus.Suspended : VMStatus.Down);
vmDynamicDao.update(vmDynamic);

List<DiskImage> imagesToExclude = diskImageDao.getAttachedDiskSnapshotsToVm(vm.getId(), Boolean.TRUE);

Expand Down
15 changes: 4 additions & 11 deletions ...l/src/main/java/org/ovirt/engine/core/bll/snapshots/TryBackToAllSnapshotsOfVmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.ovirt.engine.core.bll.validator.storage.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.storage.StoragePoolValidator;
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.ActionReturnValue;
import org.ovirt.engine.core.common.action.ActionType;
Expand Down Expand Up @@ -219,7 +218,7 @@ private void restoreVmConfigFromSnapshot() {
getCompensationContext(),
getCurrentUser(),
new VmInterfaceManager(getMacPool()),
isRestoreMemory());
getParameters().isRestoreMemory());

// custom preview - without leases
if (!getParameters().isRestoreLease()) {
Expand All @@ -243,7 +242,7 @@ private void restoreVmConfigFromSnapshot() {

@Override
protected void executeVmCommand() {
final boolean restoreMemory = isRestoreMemory();
final boolean restoreMemory = getParameters().isRestoreMemory();

final Guid newActiveSnapshotId = Guid.newGuid();
final Snapshot snapshotToBePreviewed = getDstSnapshot();
Expand Down Expand Up @@ -432,15 +431,9 @@ private LeaseAction determineLeaseAction(Guid srcLeaseDomainId, Guid dstLeaseDom
return dstLeaseDomainId != null ? LeaseAction.CREATE_NEW_LEASE : LeaseAction.DO_NOTHING;
}

private boolean isRestoreMemory() {
return getParameters().isRestoreMemory() &&
FeatureSupported.isMemorySnapshotSupportedByArchitecture(
getVm().getClusterArch(), getVm().getCompatibilityVersion());
}

private boolean updateClusterCompatibilityVersionToOldCluster(boolean disableLock) {
Version oldClusterVersion = getVm().getClusterCompatibilityVersionOrigin();
if (isRestoreMemory() && getVm().getCustomCompatibilityVersion() == null &&
if (getParameters().isRestoreMemory() && getVm().getCustomCompatibilityVersion() == null &&
oldClusterVersion.less(getVm().getClusterCompatibilityVersion())) {
// the snapshot was taken before cluster version change, call the UpdateVmCommand

Expand Down Expand Up @@ -639,7 +632,7 @@ protected boolean validate() {
return false;
}

if (isRestoreMemory() && !validateMemoryTakenInSupportedVersion()) {
if (getParameters().isRestoreMemory() && !validateMemoryTakenInSupportedVersion()) {
return false;
}

Expand Down
23 changes: 12 additions & 11 deletions backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/ActionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public final class ActionUtils {
EnumSet.of(ActionType.HibernateVm, ActionType.RunVm,
ActionType.RunVmOnce, ActionType.AddVmTemplate, ActionType.RemoveVm,
ActionType.ExportVm, ActionType.ImportVm, ActionType.ChangeDisk,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.MigrateVm, ActionType.CancelMigrateVm,
ActionType.ExtendImageSize, ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
Expand All @@ -173,23 +173,23 @@ public final class ActionUtils {
ActionType.AddVmTemplate, ActionType.RemoveVm, ActionType.MigrateVm,
ActionType.ExportVm, ActionType.ImportVm,
ActionType.ChangeDisk, ActionType.AddVmInterface,
ActionType.UpdateVmInterface,
ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm,
ActionType.ExtendImageSize, ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
VMStatus.PoweringUp,
EnumSet.of(ActionType.HibernateVm, ActionType.RunVm,
ActionType.RunVmOnce, ActionType.AddVmTemplate, ActionType.RemoveVm,
ActionType.ExportVm, ActionType.ImportVm, ActionType.ChangeDisk,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm,
ActionType.ExtendImageSize));
vmMatrix.put(
VMStatus.RebootInProgress,
EnumSet.of(ActionType.HibernateVm, ActionType.RunVm,
ActionType.RunVmOnce, ActionType.AddVmTemplate, ActionType.RemoveVm,
ActionType.ExportVm, ActionType.ImportVm, ActionType.ChangeDisk,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm,
ActionType.ExtendImageSize, ActionType.RebootVm));
vmMatrix.put(
Expand All @@ -198,7 +198,7 @@ public final class ActionUtils {
ActionType.RunVmOnce, ActionType.AddVmTemplate, ActionType.RemoveVm,
ActionType.HibernateVm, ActionType.MigrateVm, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ChangeDisk,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CreateSnapshotForVm,
ActionType.ExtendImageSize, ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
Expand All @@ -214,7 +214,7 @@ public final class ActionUtils {
EnumSet.of(ActionType.RemoveVm, ActionType.HibernateVm,
ActionType.AddVmTemplate, ActionType.RunVmOnce, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ExtendImageSize,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm,
ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
Expand All @@ -224,7 +224,7 @@ public final class ActionUtils {
ActionType.HibernateVm, ActionType.MigrateVm, ActionType.RemoveVm,
ActionType.AddVmTemplate, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ChangeDisk,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm,
ActionType.ExtendImageSize, ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
Expand All @@ -234,7 +234,7 @@ public final class ActionUtils {
ActionType.HibernateVm, ActionType.MigrateVm, ActionType.RemoveVm,
ActionType.AddVmTemplate, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ChangeDisk,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm, ActionType.ExtendImageSize,
ActionType.RebootVm, ActionType.ResetVm));

Expand All @@ -257,6 +257,7 @@ public final class ActionUtils {
ActionType.ChangeDisk,
ActionType.AddVmInterface,
ActionType.UpdateVmInterface,
ActionType.RestoreAllSnapshots,
ActionType.CreateSnapshotForVm,
ActionType.RemoveVmInterface,
ActionType.CancelMigrateVm,
Expand All @@ -271,7 +272,7 @@ public final class ActionUtils {
ActionType.HibernateVm, ActionType.MigrateVm, ActionType.RemoveVm,
ActionType.AddVmTemplate, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ChangeDisk, ActionType.CreateSnapshotForVm,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm, ActionType.ExtendImageSize,
ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
Expand All @@ -280,7 +281,7 @@ public final class ActionUtils {
ActionType.RunVmOnce, ActionType.HibernateVm, ActionType.MigrateVm,
ActionType.RemoveVm, ActionType.AddVmTemplate, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ChangeDisk, ActionType.CreateSnapshotForVm,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm, ActionType.ExtendImageSize,
ActionType.RebootVm, ActionType.ResetVm));

Expand All @@ -291,7 +292,7 @@ public final class ActionUtils {
ActionType.HibernateVm, ActionType.MigrateVm, ActionType.RemoveVm,
ActionType.AddVmTemplate, ActionType.ExportVm,
ActionType.ImportVm, ActionType.ChangeDisk, ActionType.CreateSnapshotForVm,
ActionType.AddVmInterface, ActionType.UpdateVmInterface,
ActionType.AddVmInterface, ActionType.UpdateVmInterface, ActionType.RestoreAllSnapshots,
ActionType.RemoveVmInterface, ActionType.CancelMigrateVm, ActionType.ExtendImageSize,
ActionType.RebootVm, ActionType.ResetVm));
vmMatrix.put(
Expand Down
Loading