Allon Mureinik has uploaded a new change for review.
Change subject: core: Refactor SP validation out of ImagesHandler
......................................................................
core: Refactor SP validation out of ImagesHandler
Moved the validation on the SP's status from
ImagesHandler.PerformImagesChecks to StoragePoolValidator, where it
belongs.
In all the cases where this validation was performed by an ImagesHandler
call a call to StoragePoolValidator was added in order to keep backwards
consistency, even though it is probably useless in many (most?) cases.
E.g., there is little point in checking if the SP is UP if we already
know we have an UP domain on it.
However, these changes are left for future patches in order not to "rock
the boat" too much in a single patch.
Change-Id: I5caa3724fb73c8fa8932c8071f6d27a78d681bb3
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
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/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/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java
19 files changed, 141 insertions(+), 25 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/23/11323/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 5648e7c..7ecee77 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
@@ -13,6 +13,7 @@
import org.ovirt.engine.core.bll.quota.QuotaStorageDependent;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.bll.storage.StorageDomainCommandBase;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
import org.ovirt.engine.core.bll.utils.WipeAfterDeleteUtils;
@@ -41,6 +42,7 @@
import org.ovirt.engine.core.common.businessentities.VolumeType;
import org.ovirt.engine.core.common.businessentities.permissions;
import org.ovirt.engine.core.common.businessentities.storage_domains;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.config.Config;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.locks.LockingGroup;
@@ -140,7 +142,10 @@
canAddShareableDisk();
if (returnValue && vm != null) {
- returnValue = isStoragePoolMatching(vm) &&
+ storage_pool sp = getStoragePool(); // Note this is done according
to the VM's spId.
+ returnValue =
+ validate(new StoragePoolValidator(sp).isUp()) &&
+ isStoragePoolMatching(vm) &&
performImagesChecks(vm.getStoragePoolId()) &&
validate(getSnapshotValidator().vmNotDuringSnapshot(vm.getId())) &&
validate(getSnapshotValidator().vmNotInPreview(vm.getId())) &&
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 2d0a7ac..c657148 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
@@ -14,6 +14,7 @@
import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter;
import org.ovirt.engine.core.bll.quota.QuotaStorageDependent;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
@@ -198,6 +199,11 @@
}
}
+ // Check storage pool (spId was set in the constructor
+ if (!validate(new StoragePoolValidator(getStoragePool()).isUp())) {
+ return false;
+ }
+
for (Guid srcStorageDomainId : sourceImageDomainsImageMap.keySet()) {
boolean checkIsValid = true;
if (!ImagesHandler.PerformImagesChecks(
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 736219f..786f3bf 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
@@ -12,6 +12,7 @@
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.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -286,9 +287,11 @@
}
List<DiskImage> disksList = getDisksList();
if (disksList.size() > 0) {
+ StoragePoolValidator spValidator = new
StoragePoolValidator(getStoragePool());
VmValidator vmValidator = new VmValidator(getVm());
SnapshotsValidator snapshotValidator = new SnapshotsValidator();
- result = validate(snapshotValidator.vmNotDuringSnapshot(getVmId()))
+ result = validate(spValidator.isUp())
+ &&
validate(snapshotValidator.vmNotDuringSnapshot(getVmId()))
&& validate(snapshotValidator.vmNotInPreview(getVmId()))
&& validate(vmValidator.vmNotDuringMigration())
&& validate(vmValidator.vmNotRunningStateless())
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 bcd11f5..0c2b66d 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
@@ -12,6 +12,7 @@
import org.ovirt.engine.core.bll.command.utils.StorageDomainSpaceChecker;
import org.ovirt.engine.core.bll.job.ExecutionHandler;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
@@ -154,6 +155,7 @@
SnapshotsValidator snapshotValidator = new SnapshotsValidator();
if (!(checkVmInStorageDomain()
+ && validate(new StoragePoolValidator(getStoragePool()).isUp())
&& validate(snapshotValidator.vmNotDuringSnapshot(getVmId()))
&& validate(snapshotValidator.vmNotInPreview(getVmId()))
&& validate(new VmValidator(getVm()).vmDown())
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 a5b0430..df786e1 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
@@ -24,7 +24,6 @@
import org.ovirt.engine.core.common.businessentities.LUNs;
import org.ovirt.engine.core.common.businessentities.LunDisk;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
-import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
import org.ovirt.engine.core.common.businessentities.StorageType;
import org.ovirt.engine.core.common.businessentities.VM;
@@ -34,7 +33,6 @@
import org.ovirt.engine.core.common.businessentities.VolumeType;
import org.ovirt.engine.core.common.businessentities.image_storage_domain_map;
import org.ovirt.engine.core.common.businessentities.storage_domains;
-import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.errors.VdcBLLException;
import org.ovirt.engine.core.common.errors.VdcBllErrors;
import org.ovirt.engine.core.common.utils.ListUtils;
@@ -440,16 +438,6 @@
return result;
}
-
- public static boolean isStoragePoolValid(Guid storagePoolId) {
- storage_pool pool =
DbFacade.getInstance().getStoragePoolDao().get(storagePoolId);
- if (pool == null || pool.getstatus() != StoragePoolStatus.Up) {
- return false;
- }
- return true;
- }
-
-
public static boolean PerformImagesChecks(
List<String> messages,
Guid storagePoolId,
@@ -463,12 +451,6 @@
Collection<? extends Disk> diskImageList) {
boolean returnValue = true;
-
- if (checkIsValid && !isStoragePoolValid(storagePoolId)) {
- returnValue = false;
- ListUtils.nullSafeAdd(messages,
VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_REPOSITORY_NOT_FOUND.toString());
- }
-
List<DiskImage> images = filterImageDisks(diskImageList, true, false);
if (returnValue && checkImagesLocked) {
returnValue = checkImagesLocked(messages, images);
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 c552ddb..be63a0d 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
@@ -6,6 +6,7 @@
import org.ovirt.engine.core.bll.command.utils.StorageDomainSpaceChecker;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.action.MoveVmParameters;
@@ -69,7 +70,9 @@
// CheckTemplateInStorageDomain later
VmHandler.updateDisksFromDb(getVm());
List<DiskImage> diskImages =
ImagesHandler.filterImageDisks(getVm().getDiskMap().values(), false, false);
- retValue = retValue && ImagesHandler.PerformImagesChecks(
+ retValue = retValue &&
+ validate(new StoragePoolValidator(getStoragePool()).isUp()) &&
+ ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
Guid.Empty,
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 f663707..287d24e 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
@@ -13,6 +13,7 @@
import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter;
import org.ovirt.engine.core.bll.quota.QuotaStorageDependent;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -34,6 +35,7 @@
import org.ovirt.engine.core.common.businessentities.VmEntityType;
import org.ovirt.engine.core.common.businessentities.VmTemplate;
import org.ovirt.engine.core.common.businessentities.VmTemplateStatus;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.locks.LockingGroup;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.compat.TransactionScopeOption;
@@ -226,6 +228,14 @@
boolean firstTime = true;
SnapshotsValidator snapshotsValidator = new SnapshotsValidator();
List<Disk> diskList = Arrays.asList(getDisk());
+
+ if (!listVms.isEmpty()) {
+ storage_pool sp =
getStoragePoolDAO().get(listVms.get(0).getStoragePoolId());
+ if (!validate(new StoragePoolValidator(sp).isUp())) {
+ return false;
+ }
+ }
+
for (VM vm : listVms) {
if (!validate(snapshotsValidator.vmNotDuringSnapshot(vm.getId()))
||
!validate(snapshotsValidator.vmNotInPreview(vm.getId())) ||
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 3c62495..534a9a3 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
@@ -11,6 +11,7 @@
import org.ovirt.engine.core.bll.quota.QuotaManager;
import org.ovirt.engine.core.bll.quota.QuotaStorageDependent;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -50,6 +51,7 @@
setSnapshotName(snapshot.getDescription());
}
}
+ setStoragePoolId(getVm().getStoragePoolId());
}
@Override
@@ -161,7 +163,8 @@
protected boolean canDoAction() {
initializeObjectState();
- if (!validateVmNotDuringSnapshot() ||
+ if (!validate(new StoragePoolValidator(getStoragePool()).isUp()) ||
+ !validateVmNotDuringSnapshot() ||
!validateVmNotInPreview() ||
!validateSnapshotExists() ||
!validate(new VmValidator(getVm()).vmDown()) ||
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 8bdb1b4..a925ff2 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,6 +12,7 @@
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;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -147,6 +148,10 @@
return false;
}
+ if (!validate(new StoragePoolValidator(getStoragePool()).isUp())) {
+ return false;
+ }
+
if (!ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
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 e7bdbd7..d101948 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
@@ -12,6 +12,7 @@
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.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -301,7 +302,7 @@
if (!getSnapshotDao().exists(getVmId(),
getParameters().getDstSnapshotId())) {
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST);
} else {
- result = performImagesChecks();
+ result = validate(new
StoragePoolValidator(getStoragePool()).isUp()) && performImagesChecks();
if (result && (getVm().getStatus() != VMStatus.Down)) {
result = false;
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN);
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 71b40a5..5808d75 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
@@ -8,6 +8,7 @@
import org.ovirt.engine.core.bll.job.ExecutionHandler;
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.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -217,6 +218,7 @@
if (vmDisk != null) {
result =
result
+ && validate(new
StoragePoolValidator(getStoragePool()).isUp())
&& ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
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 bd78384..0f1ee92 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
@@ -4,6 +4,7 @@
import java.util.List;
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
+import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -17,6 +18,7 @@
import org.ovirt.engine.core.common.businessentities.VmDynamic;
import org.ovirt.engine.core.common.businessentities.VmPoolMap;
import org.ovirt.engine.core.common.businessentities.VmType;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.businessentities.tags;
import org.ovirt.engine.core.common.businessentities.vm_pools;
import org.ovirt.engine.core.compat.Guid;
@@ -174,7 +176,7 @@
* @return <c>true</c> if [is vm free] [the specified vm id]; otherwise,
<c>false</c>.
*/
protected static boolean isVmFree(Guid vmId, java.util.ArrayList<String>
messages) {
- boolean returnValue;
+ boolean returnValue = true;
// check that there isn't another user already attached to this VM:
if (vmAssignedToUser(vmId, messages)) {
@@ -202,7 +204,16 @@
List<DiskImage> vmImages =
ImagesHandler.filterImageDisks(disks, true, true);
Guid storageDomainId = vmImages.size() > 0 ?
vmImages.get(0).getstorage_ids().get(0) : Guid.Empty;
VM vm = DbFacade.getInstance().getVmDao().get(vmId);
- returnValue =
+ storage_pool sp =
DbFacade.getInstance().getStoragePoolDao().get(vm.getStoragePoolId());
+
+ ValidationResult spUpResult = new
StoragePoolValidator(sp).isUp();
+ if (!spUpResult.isValid()) {
+ messages.add(spUpResult.getMessage().name());
+ returnValue = false;
+ }
+
+ if (returnValue) {
+ returnValue =
ImagesHandler.PerformImagesChecks(
messages,
vm.getStoragePoolId(),
@@ -214,6 +225,7 @@
!Guid.Empty.equals(storageDomainId),
true,
disks);
+ }
if (returnValue) {
ValidationResult vmNotLockResult = new
VmValidator(vm).vmNotLocked();
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 2757e08..344caf4 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
@@ -11,6 +11,7 @@
import org.ovirt.engine.core.bll.command.utils.StorageDomainSpaceChecker;
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.bll.validator.VmValidator;
import org.ovirt.engine.core.common.VdcActionUtils;
import org.ovirt.engine.core.common.action.RunVmParams;
@@ -27,6 +28,7 @@
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmDevice;
import org.ovirt.engine.core.common.businessentities.storage_domains;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.config.Config;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.queries.GetAllIsoImagesListParameters;
@@ -40,6 +42,7 @@
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.StorageDomainDAO;
+import org.ovirt.engine.core.dao.StoragePoolDAO;
import org.ovirt.engine.core.dao.VmDeviceDAO;
import org.ovirt.engine.core.utils.log.Log;
import org.ovirt.engine.core.utils.log.LogFactory;
@@ -105,6 +108,15 @@
if (!vmDuringSnapshotResult.isValid()) {
message.add(vmDuringSnapshotResult.getMessage().name());
retValue = false;
+ }
+
+ if (retValue) {
+ storage_pool sp =
getStoragePoolDAO().get(vm.getStoragePoolId());
+ ValidationResult spUpResult = new
StoragePoolValidator(sp).isUp();
+ if (!spUpResult.isValid()) {
+ message.add(spUpResult.getMessage().name());
+ retValue = false;
+ }
}
if (retValue && !performImageChecksForRunningVm(vm,
message, runParams, vmDisks)) {
@@ -356,4 +368,8 @@
protected StorageDomainDAO getStorageDomainDAO() {
return DbFacade.getInstance().getStorageDomainDao();
}
+
+ protected StoragePoolDAO getStoragePoolDAO() {
+ return DbFacade.getInstance().getStoragePoolDao();
+ }
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java
index a712b4b..87fab15 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StoragePoolValidator.java
@@ -3,6 +3,7 @@
import java.util.List;
import org.ovirt.engine.core.bll.ValidationResult;
+import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.StorageType;
import org.ovirt.engine.core.common.businessentities.VDSGroup;
import org.ovirt.engine.core.common.businessentities.storage_pool;
@@ -60,4 +61,12 @@
return hasDefaultCluster;
}
+ public ValidationResult isUp() {
+ if (storagePool == null || storagePool.getstatus() !=
StoragePoolStatus.Up) {
+ return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_REPOSITORY_NOT_FOUND);
+ }
+
+ return ValidationResult.VALID;
+ }
+
}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
index e94e2d6..9b473c5 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java
@@ -31,6 +31,7 @@
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap;
import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId;
+import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
import org.ovirt.engine.core.common.businessentities.StorageType;
import org.ovirt.engine.core.common.businessentities.VM;
@@ -425,6 +426,7 @@
storage_pool storagePool = new storage_pool();
storagePool.setId(storagePoolId);
storagePool.setcompatibility_version(compatibilityVersion);
+ storagePool.setstatus(StoragePoolStatus.Up);
when(storagePoolDAO.get(storagePoolId)).thenReturn(storagePool);
return storagePool;
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java
index 5d84635..b143162 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java
@@ -17,12 +17,15 @@
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.common.action.RemoveSnapshotParameters;
import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmTemplate;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.VdcBllMessages;
import org.ovirt.engine.core.dao.DiskImageDAO;
+import org.ovirt.engine.core.dao.StoragePoolDAO;
import org.ovirt.engine.core.dao.VmTemplateDAO;
/** A test case for the {@link RemoveSnapshotCommand} class. */
@@ -39,6 +42,9 @@
private DiskImageDAO diskImageDAO;
@Mock
+ private StoragePoolDAO spDao;
+
+ @Mock
private SnapshotsValidator snapshotValidator;
@Before
@@ -48,6 +54,7 @@
RemoveSnapshotParameters params = new
RemoveSnapshotParameters(snapGuid, vmGuid);
cmd = spy(new RemoveSnapshotCommand<RemoveSnapshotParameters>(params));
+ doReturn(spDao).when(cmd).getStoragePoolDAO();
doReturn(vmTemplateDAO).when(cmd).getVmTemplateDAO();
doReturn(diskImageDAO).when(cmd).getDiskImageDao();
doReturn(snapshotValidator).when(cmd).createSnapshotValidator();
@@ -79,9 +86,15 @@
@Test
public void testCanDoActionVmUp() {
+ Guid spId = Guid.NewGuid();
VM vm = new VM();
vm.setId(Guid.NewGuid());
vm.setStatus(VMStatus.Up);
+ vm.setStoragePoolId(spId);
+
+ storage_pool sp = new storage_pool();
+ sp.setId(spId);
+ sp.setstatus(StoragePoolStatus.Up);
cmd.setSnapshotName("someSnapshot");
doReturn(ValidationResult.VALID).when(snapshotValidator).vmNotDuringSnapshot(any(Guid.class));
@@ -89,6 +102,7 @@
doReturn(ValidationResult.VALID).when(snapshotValidator).snapshotExists(any(Guid.class),
any(Guid.class));
doReturn(true).when(cmd).validateImagesAndVMStates();
doReturn(vm).when(cmd).getVm();
+ doReturn(sp).when(spDao).get(spId);
doReturn(Collections.emptyList()).when(cmd).getSourceImages();
CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd,
VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN);
}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java
index 1903744..9a2fa02 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java
@@ -26,10 +26,12 @@
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.StorageDomainStatus;
+import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmDynamic;
import org.ovirt.engine.core.common.businessentities.storage_domains;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend;
import org.ovirt.engine.core.compat.Guid;
@@ -38,6 +40,7 @@
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.SnapshotDao;
import org.ovirt.engine.core.dao.StorageDomainDAO;
+import org.ovirt.engine.core.dao.StoragePoolDAO;
import org.ovirt.engine.core.dao.VmDAO;
import org.ovirt.engine.core.dao.VmDynamicDAO;
import org.ovirt.engine.core.dao.network.VmNetworkInterfaceDao;
@@ -71,6 +74,9 @@
private StorageDomainDAO storageDomainDAO;
@Mock
+ private StoragePoolDAO storagePoolDAO;
+
+ @Mock
private SnapshotDao snapshotDao;
@Mock
@@ -80,6 +86,7 @@
private Guid diskImageId = Guid.NewGuid();
private Guid storageDomainId = Guid.NewGuid();
private Guid dstSnapshotId = Guid.NewGuid();
+ private Guid spId = Guid.NewGuid();
private VmDynamic mockDynamicVm;
private Snapshot mockSnapshot;
private RestoreAllSnapshotsCommand<RestoreAllSnapshotsParameters>
spyCommand;
@@ -148,6 +155,7 @@
private void mockDaos() {
mockDiskImageDao();
mockStorageDomainDao();
+ mockStoragePoolDao();
mockDynamicVmDao();
doReturn(snapshotDao).when(spyCommand).getSnapshotDao();
doReturn(vmNetworkInterfaceDAO).when(spyCommand).getVmNetworkInterfaceDao();
@@ -183,12 +191,21 @@
when(storageDomainDAO.get(storageDomainId)).thenReturn(storageDomains);
}
+ private void mockStoragePoolDao() {
+ storage_pool sp = new storage_pool();
+ sp.setId(spId);
+ sp.setstatus(StoragePoolStatus.Up);
+ when(storagePoolDAO.get(spId)).thenReturn(sp);
+ doReturn(storagePoolDAO).when(spyCommand).getStoragePoolDAO();
+ }
+
/**
* Mock a VM.
*/
private VM mockVm() {
VM vm = new VM();
vm.setId(vmId);
+ vm.setStoragePoolId(spId);
vm.setStatus(VMStatus.Down);
AuditLogableBaseMockUtils.mockVmDao(spyCommand, vmDAO);
when(vmDAO.get(vmId)).thenReturn(vm);
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 377228c..3bf448d 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
@@ -36,12 +36,14 @@
import org.ovirt.engine.core.common.businessentities.Disk;
import org.ovirt.engine.core.common.businessentities.DiskImage;
import org.ovirt.engine.core.common.businessentities.IVdsAsyncCommand;
+import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
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.VmDeviceId;
import org.ovirt.engine.core.common.businessentities.VmStatic;
import org.ovirt.engine.core.common.businessentities.storage_domains;
+import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend;
import org.ovirt.engine.core.common.utils.VmDeviceType;
@@ -56,6 +58,7 @@
import
org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBaseMockUtils;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.StorageDomainDAO;
+import org.ovirt.engine.core.dao.StoragePoolDAO;
import org.ovirt.engine.core.dao.VmDAO;
import org.ovirt.engine.core.dao.VmDeviceDAO;
import org.ovirt.engine.core.utils.MockConfigRule;
@@ -81,6 +84,9 @@
@Mock
private VmDAO vmDAO;
+
+ @Mock
+ private StoragePoolDAO spDao;
@Spy
private final VmRunHandler vmRunHandler = VmRunHandler.getInstance();
@@ -321,7 +327,13 @@
}
protected void mockVmRunHandler() {
+ storage_pool sp = new storage_pool();
+ sp.setstatus(StoragePoolStatus.Up);
+ when(spDao.get(any(Guid.class))).thenReturn(sp);
+ doReturn(spDao).when(vmRunHandler).getStoragePoolDAO();
+
doReturn(vmRunHandler).when(command).getVmRunHandler();
+
doReturn(true).when(vmRunHandler).performImageChecksForRunningVm(any(VM.class),
anyListOf(String.class),
@@ -351,6 +363,7 @@
initDAOMocks(disks, Collections.singletonList(vmDevice));
final VM vm = new VM();
vm.setStatus(VMStatus.Up);
+ vm.setStoragePoolId(Guid.NewGuid());
doReturn(vm).when(command).getVm();
doReturn(new VdsSelector(vm, new NGuid(), true, new
VdsFreeMemoryChecker(command))).when(command)
.getVdsSelector();
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java
index 2f518d9..71f9ff4 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StoragePoolValidatorTest.java
@@ -82,6 +82,17 @@
assertMessage(result,
VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_WITH_DEFAULT_VDS_GROUP_CANNOT_BE_LOCALFS);
}
+ @Test
+ public void testIsUpdValid() {
+ assertTrue("Storage pool should be up", validator.isUp().isValid());
+ }
+
+ @Test
+ public void testIsUpdInvalid() {
+ storagePool.setstatus(StoragePoolStatus.Problematic);
+ assertMessage(validator.isUp(),
VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_REPOSITORY_NOT_FOUND);
+ }
+
private static void assertMessage(ValidationResult result, VdcBllMessages
bllMsg) {
assertEquals("Wrong canDoAction message is returned", bllMsg,
result.getMessage());
}
--
To view, visit http://gerrit.ovirt.org/11323
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5caa3724fb73c8fa8932c8071f6d27a78d681bb3
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
