Gilad Chaplik has uploaded a new change for review.

Change subject: core: RunVmCommand.canDoAction clean-up (8)
......................................................................

core: RunVmCommand.canDoAction clean-up (8)

Extract validate iso path.

Change-Id: I2c10427764fa58faee54afaa89c4e5e62f5bad82
Signed-off-by: Gilad Chaplik <[email protected]>
---
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/VmPoolCommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
5 files changed, 111 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/61/13661/1

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 7438683..20eb6ab 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
@@ -694,6 +694,9 @@
                                     isInternalExecution(),
                                     messages) &&
                             validate(new 
StoragePoolValidator(getStoragePoolDAO().get(vm.getStoragePoolId())).isUp()) &&
+                            
validate(getRunVmValidator().validateIsoPath(vm.isAutoStartup(), 
vm.getStoragePoolId(),
+                                    getParameters().getDiskPath(),
+                                    getParameters().getFloppyPath())) &&
                             canRunVm(vm) &&
                             validateNetworkInterfaces();
             if (!canDoAction) {
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 b3ad6f6..528d1fe 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
@@ -269,7 +269,9 @@
                 getDiskDao().getAllForVm(vm.getId(), true),
                 runVmParams.getBootSequence(),
                 false,
-                fetchStoragePool(vm.getStoragePoolId()))
+                fetchStoragePool(vm.getStoragePoolId()),
+                runVmParams.getDiskPath(),
+                runVmParams.getFloppyPath())
                 &&
                 VmRunHandler.getInstance().canRunVm(vm,
                         messages,
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 984901e..29148a5 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
@@ -3,11 +3,9 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
-import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.bll.interfaces.BackendInternal;
 import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
 import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
@@ -16,8 +14,6 @@
 import org.ovirt.engine.core.common.action.VdcActionType;
 import org.ovirt.engine.core.common.businessentities.Disk;
 import org.ovirt.engine.core.common.businessentities.DiskImage;
-import org.ovirt.engine.core.common.businessentities.ImageType;
-import org.ovirt.engine.core.common.businessentities.RepoFileMetaData;
 import org.ovirt.engine.core.common.businessentities.StorageDomain;
 import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.businessentities.VDSStatus;
@@ -25,9 +21,6 @@
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
-import org.ovirt.engine.core.common.queries.GetImagesListParameters;
-import org.ovirt.engine.core.common.queries.VdcQueryReturnValue;
-import org.ovirt.engine.core.common.queries.VdcQueryType;
 import 
org.ovirt.engine.core.common.vdscommands.IsVmDuringInitiatingVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.compat.Guid;
@@ -60,48 +53,39 @@
             VdsSelector vdsSelector, SnapshotsValidator snapshotsValidator) {
         boolean retValue = true;
 
-        // Check if iso and floppy path exists
-        if (retValue && !vm.isAutoStartup()
-                && !validateIsoPath(getIsoDomainListSyncronizer()
-                        .findActiveISODomain(vm.getStoragePoolId()),
-                        runParams,
-                        message)) {
+        boolean isVmDuringInit = ((Boolean) getBackend()
+                .getResourceManager()
+                .RunVdsCommand(VDSCommandType.IsVmDuringInitiating,
+                        new 
IsVmDuringInitiatingVDSCommandParameters(vm.getId()))
+                .getReturnValue()).booleanValue();
+        if (vm.isRunning() || (vm.getStatus() == VMStatus.NotResponding) || 
isVmDuringInit) {
             retValue = false;
-        } else if (retValue) {
-            boolean isVmDuringInit = ((Boolean) getBackend()
-                    .getResourceManager()
-                    .RunVdsCommand(VDSCommandType.IsVmDuringInitiating,
-                            new 
IsVmDuringInitiatingVDSCommandParameters(vm.getId()))
-                    .getReturnValue()).booleanValue();
-            if (vm.isRunning() || (vm.getStatus() == VMStatus.NotResponding) 
|| isVmDuringInit) {
+            
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING.toString());
+        } else if (vm.getStatus() == VMStatus.Paused && vm.getRunOnVds() != 
null) {
+            VDS vds = DbFacade.getInstance().getVdsDao().get(
+                    new Guid(vm.getRunOnVds().toString()));
+            if (vds.getStatus() != VDSStatus.Up) {
                 retValue = false;
-                
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING.toString());
-            } else if (vm.getStatus() == VMStatus.Paused && vm.getRunOnVds() 
!= null) {
-                VDS vds = DbFacade.getInstance().getVdsDao().get(
-                        new Guid(vm.getRunOnVds().toString()));
-                if (vds.getStatus() != VDSStatus.Up) {
-                    retValue = false;
-                    
message.add(VdcBllMessages.VAR__HOST_STATUS__UP.toString());
-                    
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.toString());
-                }
+                message.add(VdcBllMessages.VAR__HOST_STATUS__UP.toString());
+                
message.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.toString());
             }
+        }
 
-            boolean isStatelessVm = shouldVmRunAsStateless(runParams, vm);
+        boolean isStatelessVm = shouldVmRunAsStateless(runParams, vm);
 
-            if (retValue && isStatelessVm && 
!snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) {
-                retValue = false;
-                
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString());
-            }
+        if (retValue && isStatelessVm && 
!snapshotsValidator.vmNotInPreview(vm.getId()).isValid()) {
+            retValue = false;
+            
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_WHILE_IN_PREVIEW.toString());
+        }
 
-            // if the VM itself is stateless or run once as stateless
-            if (retValue && isStatelessVm && vm.isAutoStartup()) {
-                retValue = false;
-                
message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA.toString());
-            }
+        // if the VM itself is stateless or run once as stateless
+        if (retValue && isStatelessVm && vm.isAutoStartup()) {
+            retValue = false;
+            message.add(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA.toString());
+        }
 
-            if (retValue && isStatelessVm && !hasSpaceForSnapshots(vm, 
message)) {
-                return false;
-            }
+        if (retValue && isStatelessVm && !hasSpaceForSnapshots(vm, message)) {
+            return false;
         }
 
         retValue = retValue == false ? retValue : 
vdsSelector.canFindVdsToRunOn(message, false);
@@ -170,62 +154,6 @@
             }
         }
         return map;
-    }
-
-    @SuppressWarnings("unchecked")
-    private boolean validateIsoPath(Guid storageDomainId,
-            RunVmParams runParams,
-            ArrayList<String> messages) {
-        if (!StringUtils.isEmpty(runParams.getDiskPath())) {
-            if (storageDomainId == null) {
-                
messages.add(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO.toString());
-                return false;
-            }
-            boolean retValForIso = false;
-            VdcQueryReturnValue ret =
-                    getBackend().runInternalQuery(VdcQueryType.GetImagesList,
-                            new GetImagesListParameters(storageDomainId, 
ImageType.ISO));
-            if (ret != null && ret.getReturnValue() != null && 
ret.getSucceeded()) {
-                List<RepoFileMetaData> repoFileNameList = 
(List<RepoFileMetaData>) ret.getReturnValue();
-                if (repoFileNameList != null) {
-                    for (RepoFileMetaData isoFileMetaData : 
(List<RepoFileMetaData>) ret.getReturnValue()) {
-                        if 
(isoFileMetaData.getRepoFileName().equals(runParams.getDiskPath())) {
-                            retValForIso = true;
-                            break;
-                        }
-                    }
-                }
-            }
-            if (!retValForIso) {
-                
messages.add(VdcBllMessages.ERROR_CANNOT_FIND_ISO_IMAGE_PATH.toString());
-                return false;
-            }
-        }
-
-        if (!StringUtils.isEmpty(runParams.getFloppyPath())) {
-            boolean retValForFloppy = false;
-            VdcQueryReturnValue ret =
-                    getBackend().runInternalQuery(VdcQueryType.GetImagesList,
-                            new GetImagesListParameters(storageDomainId, 
ImageType.Floppy));
-            if (ret != null && ret.getReturnValue() != null && 
ret.getSucceeded()) {
-                List<RepoFileMetaData> repoFileNameList = 
(List<RepoFileMetaData>) ret.getReturnValue();
-                if (repoFileNameList != null) {
-
-                    for (RepoFileMetaData isoFileMetaData : 
(List<RepoFileMetaData>) ret.getReturnValue()) {
-                        if 
(isoFileMetaData.getRepoFileName().equals(runParams.getFloppyPath())) {
-                            retValForFloppy = true;
-                            break;
-                        }
-                    }
-                }
-            }
-            if (!retValForFloppy) {
-                
messages.add(VdcBllMessages.ERROR_CANNOT_FIND_FLOPPY_IMAGE_PATH.toString());
-                return false;
-            }
-        }
-
-        return true;
     }
 
     public boolean shouldVmRunAsStateless(RunVmParams param, VM vm) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
index 1f479a8..be9b365 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
@@ -2,17 +2,25 @@
 
 import java.util.List;
 
+import org.apache.commons.lang.StringUtils;
+import org.ovirt.engine.core.bll.Backend;
 import org.ovirt.engine.core.bll.ImagesHandler;
 import org.ovirt.engine.core.bll.IsoDomainListSyncronizer;
 import org.ovirt.engine.core.bll.ValidationResult;
 import org.ovirt.engine.core.bll.VmHandler;
+import org.ovirt.engine.core.bll.interfaces.BackendInternal;
 import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
 import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
 import org.ovirt.engine.core.common.businessentities.BootSequence;
 import org.ovirt.engine.core.common.businessentities.Disk;
 import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.ImageType;
+import org.ovirt.engine.core.common.businessentities.RepoFileMetaData;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.storage_pool;
+import org.ovirt.engine.core.common.queries.GetImagesListParameters;
+import org.ovirt.engine.core.common.queries.VdcQueryReturnValue;
+import org.ovirt.engine.core.common.queries.VdcQueryType;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.VdcBllMessages;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
@@ -80,6 +88,67 @@
         return true;
     }
 
+    @SuppressWarnings("unchecked")
+    public ValidationResult validateIsoPath(boolean isAutoStartup,
+            Guid storageDomainId,
+            String diskPath,
+            String floppyPath) {
+        if (isAutoStartup) {
+            return ValidationResult.VALID;
+        }
+        if (!StringUtils.isEmpty(diskPath)) {
+            if (storageDomainId == null) {
+                return new 
ValidationResult(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO);
+            }
+            boolean retValForIso = false;
+            VdcQueryReturnValue ret =
+                    getBackend().runInternalQuery(VdcQueryType.GetImagesList,
+                            new GetImagesListParameters(storageDomainId, 
ImageType.ISO));
+            if (ret != null && ret.getReturnValue() != null && 
ret.getSucceeded()) {
+                List<RepoFileMetaData> repoFileNameList = 
(List<RepoFileMetaData>) ret.getReturnValue();
+                if (repoFileNameList != null) {
+                    for (RepoFileMetaData isoFileMetaData : 
(List<RepoFileMetaData>) ret.getReturnValue()) {
+                        if 
(isoFileMetaData.getRepoFileName().equals(diskPath)) {
+                            retValForIso = true;
+                            break;
+                        }
+                    }
+                }
+            }
+            if (!retValForIso) {
+                return new 
ValidationResult(VdcBllMessages.ERROR_CANNOT_FIND_ISO_IMAGE_PATH);
+            }
+        }
+
+        if (!StringUtils.isEmpty(floppyPath)) {
+            boolean retValForFloppy = false;
+            VdcQueryReturnValue ret =
+                    getBackend().runInternalQuery(VdcQueryType.GetImagesList,
+                            new GetImagesListParameters(storageDomainId, 
ImageType.Floppy));
+            if (ret != null && ret.getReturnValue() != null && 
ret.getSucceeded()) {
+                List<RepoFileMetaData> repoFileNameList = 
(List<RepoFileMetaData>) ret.getReturnValue();
+                if (repoFileNameList != null) {
+
+                    for (RepoFileMetaData isoFileMetaData : 
(List<RepoFileMetaData>) ret.getReturnValue()) {
+                        if 
(isoFileMetaData.getRepoFileName().equals(floppyPath)) {
+                            retValForFloppy = true;
+                            break;
+                        }
+                    }
+                }
+            }
+            if (!retValForFloppy) {
+                return new 
ValidationResult(VdcBllMessages.ERROR_CANNOT_FIND_FLOPPY_IMAGE_PATH);
+            }
+        }
+
+        return ValidationResult.VALID;
+    }
+
+    private BackendInternal getBackend() {
+        return Backend.getInstance();
+    }
+
     protected VmNetworkInterfaceDao getVmNetworkInterfaceDao() {
         return DbFacade.getInstance().getVmNetworkInterfaceDao();
     }
@@ -99,7 +168,9 @@
             List<Disk> vmDisks,
             BootSequence bootSequence,
             boolean isInternalExecution,
-            storage_pool storagePool) {
+            storage_pool storagePool,
+            String diskPath,
+            String floppyPath) {
         if (!validateVmProperties(vm, messages)) {
             return false;
         }
@@ -126,6 +197,12 @@
             messages.add(result.getMessage().toString());
             return false;
         }
+        result = validateIsoPath(vm.isAutoStartup(), vm.getStoragePoolId(), 
diskPath, floppyPath);
+        if (!result.isValid()) {
+            messages.add(result.getMessage().toString());
+            return false;
+        }
+
         return true;
     }
 
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 4bcf6d7..1973e32 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
@@ -466,6 +466,7 @@
         sp.setstatus(StoragePoolStatus.Up);
         when(spDao.get(any(Guid.class))).thenReturn(sp);
         doReturn(spDao).when(command).getStoragePoolDAO();
+        when(runVmValidator.validateIsoPath(anyBoolean(), any(Guid.class), 
any(String.class), any(String.class))).thenReturn(ValidationResult.VALID);
         doReturn(runVmValidator).when(command).getRunVmValidator();
         return runVmValidator;
     }


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

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

Reply via email to