Allon Mureinik has uploaded a new change for review.

Change subject: core: Remove VM preview check from ImagesHandler
......................................................................

core: Remove VM preview check from ImagesHandler

Removed the logic to check if a VM is in preview from ImagesHandler.
This meant removing the isVmInPreview(VM) method and replacing any
explicit calls to it from other classes with a SnapshotValidator and
removing the reference to it from PerformImagesChecks.

Change-Id: I34f5793ff5c8436f488dd4ca507a001023095b9f
Signed-off-by: Allon Mureinik <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.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/ExportVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.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/TryBackToAllSnapshotsOfVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
15 files changed, 14 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/88/11188/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
index 32e65b8..a0b2d3a 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
@@ -207,7 +207,6 @@
                 false,
                 false,
                 false,
-                false,
                 true,
                 Collections.<Disk> emptyList());
     }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
index eb4fc20..83d5dcb 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
@@ -208,8 +208,9 @@
                     true,
                     true,
                     true,
-                    false,
-                    true, checkIsValid, 
sourceImageDomainsImageMap.get(srcStorageDomainId))) {
+                    true,
+                    checkIsValid,
+                    sourceImageDomainsImageMap.get(srcStorageDomainId))) {
                 return false;
             }
             checkIsValid = false;
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
index 984753a..a966e44 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
@@ -5,6 +5,7 @@
 
 import org.apache.commons.lang.NotImplementedException;
 import org.ovirt.engine.core.bll.context.CompensationContext;
+import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
 import org.ovirt.engine.core.bll.storage.StorageDomainCommandBase;
 import org.ovirt.engine.core.common.action.ImagesActionsParametersBase;
 import org.ovirt.engine.core.common.action.ImagesContainterParametersBase;
@@ -71,10 +72,12 @@
         return mImage;
     }
 
+    @Override
     protected ImageDao getImageDao() {
         return getDbFacade().getImageDao();
     }
 
+    @Override
     protected SnapshotDao getSnapshotDao() {
         return getDbFacade().getSnapshotDao();
     }
@@ -196,7 +199,7 @@
      */
     // TODO: Should be moved to another class in the hierarchy
     protected boolean canCreateSnapshot() {
-        if (ImagesHandler.isVmInPreview(getVmId())) {
+        if (!new 
SnapshotsValidator().vmNotDuringSnapshot(getVmId()).isValid()) {
             log.error("Cannot create snapshot. Vm is in preview status");
             return false;
         }
@@ -326,6 +329,7 @@
         return false;
     }
 
+    @Override
     protected BaseDiskDao getBaseDiskDao() {
         return getDbFacade().getBaseDiskDao();
     }
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 4593e60..db8b1ce 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
@@ -301,7 +301,6 @@
                             true,
                             true,
                             true,
-                            false,
                             true,
                             true,
                             disksList);
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java
index 51267e9..d5a722f 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java
@@ -165,7 +165,6 @@
                         true,
                         false,
                         false,
-                        false,
                         true,
                         true,
                         getDisksBasedOnImage()))) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
index d656559..2822deb 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
@@ -23,7 +23,6 @@
 import org.ovirt.engine.core.common.businessentities.ImageStatus;
 import org.ovirt.engine.core.common.businessentities.LUNs;
 import org.ovirt.engine.core.common.businessentities.LunDisk;
-import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus;
 import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
 import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
 import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
@@ -413,10 +412,6 @@
         return fromIrs;
     }
 
-    public static boolean isVmInPreview(Guid vmId) {
-        return DbFacade.getInstance().getSnapshotDao().exists(vmId, 
SnapshotStatus.IN_PREVIEW);
-    }
-
     public static boolean CheckImageConfiguration(StorageDomainStatic 
storageDomain,
             DiskImageBase diskInfo, List<String> messages) {
         boolean result = true;
@@ -464,9 +459,9 @@
             boolean checkImagesLocked,
             boolean checkImagesIllegal,
             boolean checkImagesExist,
-            boolean checkVmInPreview,
             boolean checkStorageDomain,
-            boolean checkIsValid, Collection<? extends Disk> diskImageList) {
+            boolean checkIsValid,
+            Collection<? extends Disk> diskImageList) {
 
         boolean returnValue = true;
 
@@ -478,11 +473,6 @@
         List<DiskImage> images = getImages(vm, diskImageList);
         if (returnValue && checkImagesLocked) {
             returnValue = checkImagesLocked(vm, messages, images);
-        }
-
-        if (returnValue && checkVmInPreview && isVmInPreview(vm.getId())) {
-            returnValue = false;
-            ListUtils.nullSafeAdd(messages, 
VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW.toString());
         }
 
         if (returnValue && checkIsValid) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
index 0cb264a..c0ef58f 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
@@ -85,7 +85,6 @@
                                 true,
                                 true,
                                 false,
-                                false,
                                 true,
                                 diskImages);
         setStoragePoolId(getVm().getStoragePoolId());
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
index 7a2aa29..ef35983 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
@@ -238,7 +238,6 @@
                             false,
                             false,
                             false,
-                            false,
                             firstTime,
                             diskList)) {
                 return false;
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 4681672..4f3cbac 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
@@ -213,7 +213,7 @@
     protected boolean validateImagesAndVMStates() {
         return ImagesHandler.PerformImagesChecks(getVm(), 
getReturnValue().getCanDoActionMessages(),
                 getVm().getStoragePoolId(), Guid.Empty,
-                hasImages(), hasImages(), hasImages(), hasImages(), false, 
true, true, null);
+                hasImages(), hasImages(), hasImages(), hasImages(), true, 
true, null);
     }
 
     protected boolean validateImageNotInTemplate() {
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 adcb51e..47f2df0 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
@@ -153,7 +153,6 @@
                 !getParameters().getForce(),
                 false,
                 false,
-                false,
                 !getVm().getDiskMap().values().isEmpty(),
                 true,
                 getVm().getDiskMap().values())) {
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 ace8e27..8513502 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
@@ -334,7 +334,6 @@
                         true,
                         false,
                         false,
-                        false,
                         true,
                         true,
                         getImagesList());
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
index fe8dced..69b2286 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
@@ -225,7 +225,6 @@
                                     true,
                                     false,
                                     false,
-                                    false,
                                     true,
                                     true,
                                     getVm().getDiskMap().values());
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
index bed879d..1f1682c 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
@@ -210,7 +210,6 @@
                                 true,
                                 false,
                                 false,
-                                false,
                                 !Guid.Empty.equals(storageDomainId),
                                 true,
                                 disks);
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
index 3f8517a..2047fa0 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
@@ -136,7 +136,7 @@
 
                             boolean isStatelessVm = 
shouldVmRunAsStateless(runParams, vm);
 
-                            if (retValue && isStatelessVm && 
isVmInPreview(vm)) {
+                            if (retValue && isStatelessVm && 
!snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) {
                                 retValue = false;
                                 
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString());
                             }
@@ -215,14 +215,10 @@
             (VM vm, List<String> message, RunVmParams runParams, List<Disk> 
vmDisks) {
         return ImagesHandler.PerformImagesChecks(vm, message,
                 vm.getStoragePoolId(), Guid.Empty, !vm.isAutoStartup(),
-                true, false, false, false,
+                true, false, false,
                 !vm.isAutoStartup() || !runParams.getIsInternal() && 
vm.isAutoStartup(),
                 !vm.isAutoStartup() || !runParams.getIsInternal() && 
vm.isAutoStartup(),
                 vmDisks);
-    }
-
-    protected boolean isVmInPreview(VM vm) {
-        return ImagesHandler.isVmInPreview(vm.getId());
     }
 
     /**
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
index 8585103..377228c 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
@@ -327,7 +327,6 @@
                 anyListOf(String.class),
                 any(RunVmParams.class),
                 anyListOf(Disk.class));
-        doReturn(false).when(vmRunHandler).isVmInPreview(any(VM.class));
     }
 
     @Test
@@ -484,6 +483,7 @@
     private SnapshotsValidator mockSuccessfulSnapshotValidator() {
         SnapshotsValidator snapshotsValidator = mock(SnapshotsValidator.class);
         
when(snapshotsValidator.vmNotDuringSnapshot(any(Guid.class))).thenReturn(ValidationResult.VALID);
+        
when(snapshotsValidator.vmNotInPreview(any(Guid.class))).thenReturn(ValidationResult.VALID);
         doReturn(snapshotsValidator).when(command).getSnapshotsValidator();
         return snapshotsValidator;
     }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I34f5793ff5c8436f488dd4ca507a001023095b9f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to