Arik Hadas has uploaded a new change for review.

Change subject: core: [WIP] remove memory from active snapshot
......................................................................

core: [WIP] remove memory from active snapshot

Change-Id: I26665a76146568d14b687c0431aa73265f94bbb1
Signed-off-by: Arik Hadas <[email protected]>
---
M backend/manager/dbscripts/snapshots_sp.sql
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/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
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
10 files changed, 225 insertions(+), 117 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/19/15119/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/CreateAllSnapshotsFromVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
index 6c63dd0..89de8e5 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
@@ -47,7 +47,6 @@
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.common.validation.group.CreateEntity;
 import 
org.ovirt.engine.core.common.vdscommands.CreateImageVDSCommandParameters;
-import 
org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.SnapshotVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
@@ -142,9 +141,19 @@
     }
 
     private MemoryImageBuilder createMemoryImageBuilder() {
-        return 
FeatureSupported.memorySnapshot(ClusterUtils.getCompatilibilyVersion(getVm()))
-                && getParameters().isSaveMemory() && 
isLiveSnapshotApplicable() ?
-                        new DefaultMemoryImageBuilder() : new 
NullableImageBuilder();
+        if 
(!FeatureSupported.memorySnapshot(ClusterUtils.getCompatilibilyVersion(getVm())))
 {
+            return new NullableMemoryImageBuilder();
+        }
+
+        if (getParameters().getSnapshotType() == SnapshotType.STATELESS) {
+            return new StatelessSnapshotMemoryImageBuilder();
+        }
+
+        if (getParameters().isSaveMemory() && isLiveSnapshotApplicable()) {
+            return new LiveSnapshotMemoryImageBuilder();
+        }
+
+        return new NullableMemoryImageBuilder();
     }
 
     private Snapshot addSnapshotToDB(Guid snapshotId, MemoryImageBuilder 
memoryImageBuilder) {
@@ -214,25 +223,32 @@
                     if (isLiveSnapshotApplicable()) {
                         performLiveSnapshot(createdSnapshot);
                     }
-                    else if (!createdSnapshot.getMemoryVolume().isEmpty() && 
getVm() != null) {
-                        String[] strings = 
getVm().getHibernationVolHandle().split(",");
-                        Guid[] guids = new Guid[strings.length];
-                        for (int i=0; i<strings.length; ++i) {
-                            guids[i] = new Guid(strings[i]);
-                        }
-
-                        VDSReturnValue vdsRetValue1 = runVdsCommand(
-                                VDSCommandType.DeleteImageGroup,
-                                new 
DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[2],
-                                        false, false, 
getVm().getVdsGroupCompatibilityVersion().toString()));
-
-                        if (!vdsRetValue1.getSucceeded()) {
-                            // log..
+                    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();
@@ -456,11 +472,24 @@
     }
 
     private interface MemoryImageBuilder {
+
+        /**
+         * Create the images
+         */
         void build();
+
+        /**
+         * Return the string representation of the memory volumes
+         * @return string representation of the memory volumes
+         */
         String getVolumeStringRepresentation();
     }
 
-    private class DefaultMemoryImageBuilder implements MemoryImageBuilder {
+    /**
+     * This builder creates the memory images for live snapshots with memory 
operation
+     */
+    private class LiveSnapshotMemoryImageBuilder implements MemoryImageBuilder 
{
+
         private Guid storageDomainId;
         private Guid storagePoolId;
         private Guid imageGroupId;
@@ -468,7 +497,7 @@
         private Guid imageGroupId2;
         private Guid volumeId2;
 
-        private DefaultMemoryImageBuilder() {
+        private LiveSnapshotMemoryImageBuilder() {
             this.storageDomainId = getStorageDomainId().getValue();
             this.storagePoolId = getVm().getStoragePoolId();
             this.imageGroupId = Guid.NewGuid();
@@ -554,7 +583,11 @@
         }
     }
 
-    private class NullableImageBuilder implements MemoryImageBuilder {
+    /**
+     * This builder is used when no memory image should be created
+     */
+    private class NullableMemoryImageBuilder implements MemoryImageBuilder {
+
         @Override
         public void build() {
             //no op
@@ -565,4 +598,28 @@
             return StringUtils.EMPTY;
         }
     }
+
+    /**
+     * This builder is responsible to create the memory volumes for stateless 
snapshot -
+     * it just take the memory volume of the previously active snapshot
+     */
+    private class StatelessSnapshotMemoryImageBuilder implements 
MemoryImageBuilder {
+
+        private final String memoryVolumesFromActiveSnapshot;
+
+        public StatelessSnapshotMemoryImageBuilder() {
+            memoryVolumesFromActiveSnapshot =
+                    getSnapshotDao().get(getVm().getId(), 
SnapshotType.ACTIVE).getMemoryVolume();
+        }
+
+        @Override
+        public void build() {
+          //no op
+        }
+
+        @Override
+        public String getVolumeStringRepresentation() {
+            return memoryVolumesFromActiveSnapshot;
+        }
+    }
 }
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 755f066..454cd2c 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
@@ -26,7 +26,6 @@
 import org.ovirt.engine.core.common.action.RestoreFromSnapshotParameters;
 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.ImageStatus;
 import org.ovirt.engine.core.common.businessentities.Snapshot;
@@ -36,15 +35,10 @@
 import org.ovirt.engine.core.common.errors.VdcBllErrors;
 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.compat.NGuid;
 import org.ovirt.engine.core.dal.VdcBllMessages;
 import org.ovirt.engine.core.dao.SnapshotDao;
-import org.ovirt.engine.core.utils.linq.LinqUtils;
-import org.ovirt.engine.core.utils.linq.Predicate;
 
 /**
  * Restores the given snapshot, including all the VM configuration that was 
stored in it.<br>
@@ -131,47 +125,17 @@
 
     protected void removeSnapshotsFromDB() {
         for (Guid snapshotId : snapshotsToRemove) {
-            Snapshot snapshot = getSnapshotDao().get(snapshotId);
-            if (!snapshot.getMemoryVolume().isEmpty()) {
-                removeMemory(snapshot);
+            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);
         }
-    }
-
-    private void removeMemory(final Snapshot snapshot) {
-        String[] strings = snapshot.getMemoryVolume().split(",");
-        Guid[] guids = new Guid[strings.length];
-        for (int i=0; i<strings.length; ++i) {
-            guids[i]= new Guid(strings[i]);
-        }
-        // 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 vdsRetValue1 = runVdsCommand(
-                        VDSCommandType.DeleteImageGroup,
-                        new DeleteImageGroupVDSCommandParameters(guids[1], 
guids[0], guids[2],
-                                postZero, false, 
getVm().getVdsGroupCompatibilityVersion().toString()));
-
-                if (!vdsRetValue1.getSucceeded()) {
-                    // TODO
-                }
-                else {
-                    Guid guid1 =
-                            createTask(vdsRetValue1.getCreationInfo(), 
VdcActionType.RestoreAllSnapshots, VdcObjectType.Storage, guids[0]);
-                    getReturnValue().getTaskIdList().add(guid1);
-                }
     }
 
     protected void deleteOrphanedImages() {
@@ -263,7 +227,7 @@
      */
     private void restoreConfiguration(Snapshot targetSnapshot) {
         SnapshotsManager snapshotsManager = new SnapshotsManager();
-        removedSnapshotId = getSnapshotDao().getId(getVmId(), 
SnapshotType.ACTIVE);;
+        removedSnapshotId = getSnapshotDao().getId(getVmId(), 
SnapshotType.ACTIVE);
         snapshotsToRemove.add(removedSnapshotId);
         snapshotsManager.removeAllIllegalDisks(removedSnapshotId, getVmId());
 
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 f75b878..71bc6b2 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
@@ -66,6 +66,7 @@
 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;
@@ -82,8 +83,14 @@
     private String _cdImagePath = "";
     private String _floppyImagePath = "";
     private boolean mResume;
-    private boolean _isVmRunningStateless;
+    /** Note: this field should not used directly, use {@link 
#isVmRunningStateless()} instead */
+    private Boolean cachedVmIsRunningStateless;
     private boolean isFailedStatlessSnapshot;
+    // 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);
@@ -131,7 +138,25 @@
 
             // set vm disks
             VmHandler.updateDisksForVm(getVm(), 
getDiskDao().getAllForVm(getVm().getId()));
+
+            if (getVm().getStatus() != VMStatus.Suspended) {
+                // 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();
     }
 
     /**
@@ -229,9 +254,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);
@@ -249,8 +273,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()) {
@@ -269,7 +293,7 @@
                     }
                 } else if (!isInternalExecution() && !_isRerun
                         && getVm().getStatus() != VMStatus.Suspended
-                        && statelessSnapshotExistsForVm()
+                        && isStatelessSnapshotExistsForVm()
                         && !isVMPartOfManualPool()) {
                     removeVmStatlessImages();
                 } else {
@@ -281,8 +305,8 @@
         }
     }
 
-    private boolean statelessSnapshotExistsForVm() {
-        return getDbFacade().getSnapshotDao().exists(getVm().getId(), 
SnapshotType.STATELESS);
+    private boolean isStatelessSnapshotExistsForVm() {
+        return getSnapshotDao().exists(getVm().getId(), 
SnapshotType.STATELESS);
     }
 
     /**
@@ -319,7 +343,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());
@@ -439,24 +463,30 @@
 
         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) {
-            Snapshot activeSnapshotOfVmToBeCreated =
-                    
DbFacade.getInstance().getSnapshotDao().get(vmToBeCreated.getId(), 
SnapshotType.ACTIVE);
-            
vmToBeCreated.setHibernationVolHandle(activeSnapshotOfVmToBeCreated.getMemoryVolume());
+        if (!memoryFromSnapshotIrrelevant && vmToBeCreated.getStatus() != 
VMStatus.Suspended) {
+            // If the VM is not hibernated, use the hibernation volume from 
the active snapshot
+            // (might be empty)
+            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 was 
created by hibernate command)
             parameters.setClearHibernationVolumes(true);
             return parameters;
         }
@@ -474,7 +504,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 {
@@ -484,7 +514,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 ?
@@ -497,13 +527,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:
@@ -733,9 +763,7 @@
 
     @Override
     protected void endSuccessfully() {
-        setIsVmRunningStateless();
-
-        if (_isVmRunningStateless) {
+        if (isVmRunningStateless()) {
             CreateAllSnapshotsFromVmParameters createSnapshotParameters = 
buildCreateSnapshotParameters();
             
createSnapshotParameters.setImagesParameters(getParameters().getImagesParameters());
             getBackend().EndAction(VdcActionType.CreateAllSnapshotsFromVm, 
createSnapshotParameters);
@@ -768,8 +796,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);
             }
@@ -783,8 +810,7 @@
 
     @Override
     protected void endWithFailure() {
-        setIsVmRunningStateless();
-        if (_isVmRunningStateless) {
+        if (isVmRunningStateless()) {
             CreateAllSnapshotsFromVmParameters createSnapshotParameters = 
buildCreateSnapshotParameters();
             
createSnapshotParameters.setImagesParameters(getParameters().getImagesParameters());
             VdcReturnValueBase vdcReturnValue = 
getBackend().endAction(VdcActionType.CreateAllSnapshotsFromVm,
@@ -800,8 +826,33 @@
         }
     }
 
-    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 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 5ed7bc3..87471ca 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 2a0a941..af5d298 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 88c333b..0e63fc0 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 9bdb2ef..29f411a 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,7 +1,6 @@
 package org.ovirt.engine.core.bll;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 
 import org.ovirt.engine.core.bll.network.MacPoolManager;
@@ -232,15 +231,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 +252,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 +263,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/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 ffafa14..b9f3353 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
@@ -5,6 +5,7 @@
 import java.util.Date;
 import java.util.List;
 
+import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.common.businessentities.Snapshot;
 import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus;
 import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
@@ -222,4 +223,14 @@
                 getIntegerMapper(),
                 parameterSource);
     }
+
+    @Override
+    public void removeMemoryFromSnapshot(Guid snapshotId) {
+        MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource()
+                .addValue("snapshot_id", snapshotId)
+                .addValue("memory_volume", StringUtils.EMPTY);
+
+        getCallsHandler().executeModification("UpdateMemoryOfSnapshot",
+                parameterSource);
+    }
 }


--
To view, visit http://gerrit.ovirt.org/15119
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I26665a76146568d14b687c0431aa73265f94bbb1
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

Reply via email to