Allon Mureinik has uploaded a new change for review.
Change subject: core: Remove image status check from ImagesHandler
......................................................................
core: Remove image status check from ImagesHandler
Extracted all image status checks from ImagesHandler and moved them to
the new DiskImagesValidator class, as a step in the effort to decouple
ImagesHandler.PerformImagesChecks.
Change-Id: I7c26d6cf37227aded9318523f0a807e19eeaa7bc
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.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/lsm/LiveMigrateVmDisksCommand.java
A
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
A
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java
17 files changed, 260 insertions(+), 80 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/01/14101/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
index af6d114..edcd8e5 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
@@ -9,6 +9,7 @@
import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
+import org.ovirt.engine.core.bll.validator.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.AddVmFromSnapshotParameters;
@@ -278,7 +279,8 @@
List<DiskImage> disksToCheck =
ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getSourceVmFromDb().getId()),
true, false);
- if
(!ImagesHandler.checkImagesLocked(getReturnValue().getCanDoActionMessages(),
disksToCheck)) {
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(disksToCheck);
+ if (!validate(diskImagesValidator.diskImagesNotLocked())) {
return false;
}
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 8378c76..1ef8979 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
@@ -19,6 +19,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -207,7 +208,10 @@
return false;
}
- if
(!ImagesHandler.checkImagesIllegal(getReturnValue().getCanDoActionMessages(),
mImages)) {
+ List<DiskImage> diskImagesToCheck =
ImagesHandler.filterImageDisks(mImages, true, false);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImagesToCheck);
+ if (!validate(diskImagesValidator.diskImagesNotIllegal()) ||
+ !validate(diskImagesValidator.diskImagesNotLocked())) {
return false;
}
@@ -223,8 +227,7 @@
true,
true,
true,
- true,
- ImagesHandler.filterImageDisks(mImages, true, false))) {
+ diskImagesToCheck)) {
return false;
}
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 fcd8292..0fe973e 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
@@ -14,6 +14,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -306,16 +307,17 @@
MultipleStorageDomainsValidator sdValidator =
new
MultipleStorageDomainsValidator(getVm().getStoragePoolId(),
ImagesHandler.getAllStorageIdsForImageIds(disksList));
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(disksList);
result = validate(spValidator.isUp())
&&
validate(snapshotValidator.vmNotDuringSnapshot(getVmId()))
&& validate(snapshotValidator.vmNotInPreview(getVmId()))
&& validate(vmValidator.vmNotDuringMigration())
&& validate(vmValidator.vmNotRunningStateless())
&& validate(vmValidator.vmNotIlegal())
+ && validate(diskImagesValidator.diskImagesNotLocked())
&& ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true,
true,
true,
true,
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 b7a85d2..52f0c99 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
@@ -13,6 +13,7 @@
import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
import org.ovirt.engine.core.bll.utils.ClusterUtils;
import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
+import org.ovirt.engine.core.bll.validator.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
@@ -96,7 +97,9 @@
// load the disks of vm from database
VmHandler.updateDisksFromDb(getVm());
- if
(!ImagesHandler.checkImagesIllegal(getReturnValue().getCanDoActionMessages(),
getVm().getDiskList())) {
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(getDisksBasedOnImage());
+ if (!validate(diskImagesValidator.diskImagesNotIllegal()) ||
+ !validate(diskImagesValidator.diskImagesNotLocked())) {
return false;
}
@@ -167,7 +170,6 @@
&& ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true,
false,
false,
true,
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 5c9d920..fc0696f 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
@@ -449,18 +449,13 @@
public static boolean PerformImagesChecks(
List<String> messages,
Guid storagePoolId,
- boolean checkImagesLocked,
boolean checkImagesIllegalInVdsm,
boolean checkImagesExist,
boolean checkIsValid,
List<DiskImage> diskImageList) {
boolean returnValue = true;
- if (checkImagesLocked) {
- returnValue = checkImagesLocked(messages, diskImageList);
- }
-
- if (returnValue && checkIsValid) {
+ if (checkIsValid) {
if (diskImageList.size() > 0) {
returnValue = returnValue &&
checkDiskImages(messages,
@@ -473,33 +468,6 @@
ListUtils.nullSafeAdd(messages,
VdcBllMessages.ACTION_TYPE_FAILED_VM_HAS_NO_DISKS.toString());
}
}
- return returnValue;
- }
-
- public static boolean checkImagesLocked(List<String> messages,
List<DiskImage> images) {
- return checkImagesNotInStatus(messages, images, ImageStatus.LOCKED,
VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED);
- }
-
- public static boolean checkImagesIllegal(List<String> messages,
List<DiskImage> images) {
- return checkImagesNotInStatus(messages, images, ImageStatus.ILLEGAL,
VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL);
- }
-
- private static boolean checkImagesNotInStatus(List<String> messages,
List<DiskImage> images, ImageStatus status, VdcBllMessages failMessage) {
- boolean returnValue = true;
- List<String> disksInStatus = new ArrayList<String>();
- for (DiskImage diskImage : images) {
- if (diskImage.getImageStatus() == status) {
- disksInStatus.add(diskImage.getDiskAlias());
- returnValue = false;
- }
- }
-
- if (disksInStatus.size() > 0) {
- ListUtils.nullSafeAdd(messages, failMessage.toString());
- ListUtils.nullSafeAdd(messages,
- String.format("$%1$s %2$s", "diskAliases",
StringUtils.join(disksInStatus, ", ")));
- }
-
return returnValue;
}
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 6de6851..4eceb8a 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.snapshots.SnapshotsValidator;
import org.ovirt.engine.core.bll.storage.StoragePoolValidator;
+import org.ovirt.engine.core.bll.validator.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -70,16 +71,18 @@
// CheckTemplateInStorageDomain later
VmHandler.updateDisksFromDb(getVm());
List<DiskImage> diskImages =
ImagesHandler.filterImageDisks(getVm().getDiskMap().values(), false, false);
+ List<DiskImage> diskImagesToValidate =
ImagesHandler.filterImageDisks(diskImages, true, false);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImagesToValidate);
retValue = retValue &&
validate(new StoragePoolValidator(getStoragePool()).isUp()) &&
+ validate(diskImagesValidator.diskImagesNotLocked()) &&
ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
true,
true,
true,
- true,
- ImagesHandler.filterImageDisks(diskImages,
true, false));
+ diskImagesToValidate);
ensureDomainMap(diskImages, getParameters().getStorageDomainId());
for(DiskImage disk : diskImages) {
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 f8ae1e7..aa8b649 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
@@ -15,6 +15,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -213,15 +214,16 @@
return false;
}
- List<Disk> diskList = Arrays.asList(getDisk());
- if (!ImagesHandler.PerformImagesChecks(
+ List<DiskImage> diskList =
ImagesHandler.filterImageDisks(Arrays.asList(getDisk()), true, false);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskList);
+ if (!validate(diskImagesValidator.diskImagesNotLocked()) ||
+ !ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
storagePoolId,
- true,
false,
false,
true,
- ImagesHandler.filterImageDisks(diskList, true, false))) {
+ 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 930566d..54c1f67 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
@@ -14,6 +14,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -263,10 +264,12 @@
protected boolean validateImages() {
List<DiskImage> imagesToValidate =
ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getVmId()), true,
false);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(imagesToValidate);
- return
ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(),
+ return validate(diskImagesValidator.diskImagesNotLocked()) &&
+
ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true, true, true, true, imagesToValidate);
+ true, true, true, imagesToValidate);
}
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 2557dae..fedf6df 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
@@ -16,6 +16,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -165,10 +166,14 @@
return false;
}
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(vmImages); // NO!
+ if (!getParameters().getForce() &&
!validate(diskImagesValidator.diskImagesNotLocked())) {
+ return false;
+ }
+
if (!ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- !getParameters().getForce(),
false,
false,
true,
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 9741b9d..66effab 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
@@ -15,6 +15,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -340,14 +341,17 @@
}
protected boolean performImagesChecks() {
- return ImagesHandler.PerformImagesChecks
+ List<DiskImage> diskImagesToCheck =
+ ImagesHandler.filterImageDisks(getImagesList(), true, false);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImagesToCheck);
+ return validate(diskImagesValidator.diskImagesNotLocked()) &&
+ ImagesHandler.PerformImagesChecks
(getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true,
false,
false,
true,
- ImagesHandler.filterImageDisks(getImagesList(), true,
false));
+ diskImagesToCheck);
}
@Override
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 f314270..33c50e9 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
@@ -9,6 +9,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.AuditLogType;
@@ -219,7 +220,9 @@
return false;
}
- if
(!ImagesHandler.checkImagesIllegal(getReturnValue().getCanDoActionMessages(),
diskImages)) {
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImages);
+ if (!validate(diskImagesValidator.diskImagesNotIllegal()) ||
+ !validate(diskImagesValidator.diskImagesNotLocked())) {
return false;
}
@@ -232,7 +235,6 @@
|| !ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true,
false,
false,
true,
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 5e86b99..195791d 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
@@ -1,11 +1,14 @@
package org.ovirt.engine.core.bll;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedList;
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -166,10 +169,18 @@
}
}
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(vmImages);
+ ValidationResult disksNotLockedResult =
diskImagesValidator.diskImagesNotLocked();
+ if (!disksNotLockedResult.isValid()) {
+ List<String> messagesToAdd = new LinkedList<String>();
+ messagesToAdd.add(disksNotLockedResult.getMessage().name());
+
messagesToAdd.addAll(disksNotLockedResult.getVariableReplacements());
+ return failVmFree(messages, messagesToAdd);
+ }
+
if (!ImagesHandler.PerformImagesChecks(
messages,
vm.getStoragePoolId(),
- true,
false,
false,
true,
@@ -189,7 +200,11 @@
for (String messageToAdd : messagesToAdd) {
messages.add(messageToAdd);
}
+ return failVmFree(messages, Arrays.asList(messagesToAdd));
+ }
+ private static boolean failVmFree(List<String> messages, List<String>
messagesToAdd) {
+ messages.addAll(messagesToAdd);
messages.add(VdcBllMessages.VAR__TYPE__DESKTOP_POOL.toString());
messages.add(VdcBllMessages.VAR__ACTION__ATTACH_DESKTOP_TO.toString());
return false;
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 0d44939..f3786f7 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
@@ -12,6 +12,7 @@
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.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.bll.validator.VmValidator;
@@ -293,9 +294,11 @@
*/
protected boolean performImageChecksForRunningVm
(VM vm, List<String> message, RunVmParams runParams,
List<DiskImage> vmDisks) {
- return ImagesHandler.PerformImagesChecks(message,
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(vmDisks);
+ return validate(diskImagesValidator.diskImagesNotLocked(), message) &&
+ ImagesHandler.PerformImagesChecks(message,
vm.getStoragePoolId(),
- true, false, false,
+ false, false,
!vm.isAutoStartup() || !runParams.getIsInternal(),
vmDisks);
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
index cad601f..bbc5baa 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java
@@ -19,6 +19,7 @@
import org.ovirt.engine.core.bll.tasks.SPMAsyncTaskHandler;
import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
+import org.ovirt.engine.core.bll.validator.DiskImagesValidator;
import org.ovirt.engine.core.bll.validator.StorageDomainValidator;
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.LiveMigrateDiskParameters;
@@ -219,7 +220,8 @@
protected boolean checkImagesStatus() {
List<DiskImage> disksToCheck =
ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getVmId()), true,
false);
- return
ImagesHandler.checkImagesLocked(getReturnValue().getCanDoActionMessages(),
disksToCheck);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(disksToCheck);
+ return validate(diskImagesValidator.diskImagesNotLocked());
}
private boolean isDiskNotShareable(Guid imageId) {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java
new file mode 100644
index 0000000..bb084c8
--- /dev/null
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java
@@ -0,0 +1,70 @@
+package org.ovirt.engine.core.bll.validator;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.apache.commons.lang.StringUtils;
+import org.ovirt.engine.core.bll.ValidationResult;
+import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.ImageStatus;
+import org.ovirt.engine.core.dal.VdcBllMessages;
+
+/**
+ * A validator for the {@link DiskImage} class. Since most usecases require
validations of multiple {@link DiskImage}s
+ * (e.g., all the disks belonging to a VM/template), this class works on a
{@link Collection} of {@link DiskImage}s.
+ *
+ */
+public class DiskImagesValidator {
+
+ private Collection<DiskImage> diskImages;
+
+ public DiskImagesValidator(Collection<DiskImage> disks) {
+ this.diskImages = disks;
+ }
+
+ /**
+ * Validates that non of the disk are {@link ImageStatus#ILLEGAL}.
+ *
+ * @return A {@link ValidationResult} with the validation information.
+ */
+ public ValidationResult diskImagesNotIllegal() {
+ return diskImagesNotInStatus(ImageStatus.ILLEGAL,
VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL);
+ }
+
+ /**
+ * Validates that non of the disk are {@link ImageStatus#LOCKED}.
+ *
+ * @return A {@link ValidationResult} with the validation information.
+ */
+ public ValidationResult diskImagesNotLocked() {
+ return diskImagesNotInStatus(ImageStatus.LOCKED,
VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED);
+ }
+
+ /**
+ * Validates that non of the disk are in the given {@link #status}.
+ *
+ * @param status
+ * The status to check
+ * @param failMessage
+ * The validation message to return in case of failure.
+ * @return A {@link ValidationResult} with the validation information. If
none of the disks are in the given status,
+ * {@link ValidationResult#VALID} is returned. If one or more
disks are in that status, a
+ * {@link ValidationResult} with {@link #failMessage} and the
names of the disks in that status is returned.
+ */
+ private ValidationResult diskImagesNotInStatus(ImageStatus status,
VdcBllMessages failMessage) {
+ List<String> disksInStatus = new ArrayList<String>();
+ for (DiskImage diskImage : diskImages) {
+ if (diskImage.getImageStatus() == status) {
+ disksInStatus.add(diskImage.getDiskAlias());
+ }
+ }
+
+ if (!disksInStatus.isEmpty()) {
+ return new ValidationResult(failMessage,
+ String.format("$diskAliases %s",
StringUtils.join(disksInStatus, ", ")));
+ }
+
+ return ValidationResult.VALID;
+ }
+}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
index fd70867..7e6792b 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java
@@ -2,20 +2,15 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.assertFalse;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.LinkedList;
-import java.util.List;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import org.ovirt.engine.core.common.businessentities.DiskImage;
-import org.ovirt.engine.core.common.businessentities.ImageStatus;
import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.dal.VdcBllMessages;
/** A test case for {@link ImagesHandler} */
public class ImagesHandlerTest {
@@ -65,23 +60,6 @@
public void testGetDiskAliasWithDefaultNotNullAlias() {
disk1.setDiskAlias("alias");
assertEquals("alias", ImagesHandler.getDiskAliasWithDefault(disk1,
"default"));
- }
-
- @Test
- public void testCheckImagesIllegalWithIllegalDisk() {
- disk1.setImageStatus(ImageStatus.ILLEGAL);
- List<DiskImage> images = Arrays.asList(disk1, disk2);
- List<String> messages = new LinkedList<String>();
- assertFalse(ImagesHandler.checkImagesIllegal(messages, images));
-
assertTrue(messages.contains(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL.toString()));
- }
-
- @Test
- public void testCheckImagesIllegalWithoutIllegalDisk() {
- List<DiskImage> images = Arrays.asList(disk1, disk2);
- List<String> messages = new LinkedList<String>();
- assertTrue(ImagesHandler.checkImagesIllegal(messages, images));
- assertTrue(messages.isEmpty());
}
@Test
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java
new file mode 100644
index 0000000..6152636
--- /dev/null
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java
@@ -0,0 +1,116 @@
+package org.ovirt.engine.core.bll.validator;
+
+import static org.junit.Assert.assertThat;
+import static org.junit.matchers.JUnitMatchers.both;
+import static org.junit.matchers.JUnitMatchers.hasItem;
+import static
org.ovirt.engine.core.bll.validator.ValidationResultMatchers.failsWith;
+import static
org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid;
+import static
org.ovirt.engine.core.bll.validator.ValidationResultMatchers.replacements;
+
+import java.util.Arrays;
+
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.ImageStatus;
+import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.dal.VdcBllMessages;
+import org.ovirt.engine.core.utils.RandomUtils;
+import org.ovirt.engine.core.utils.RandomUtilsSeedingRule;
+
+public class DiskImagesValidatorTest {
+ @ClassRule
+ public static RandomUtilsSeedingRule rusr = new RandomUtilsSeedingRule();
+
+ private DiskImage disk1;
+ private DiskImage disk2;
+ private DiskImagesValidator validator;
+
+ @Before
+ public void setUp() {
+ disk1 = createDisk();
+ disk1.setDiskAlias("disk1");
+ disk2 = createDisk();
+ disk2.setDiskAlias("disk2");
+ validator = new DiskImagesValidator(Arrays.asList(disk1, disk2));
+ }
+
+ private static DiskImage createDisk() {
+ DiskImage disk = new DiskImage();
+ disk.setId(Guid.NewGuid());
+ disk.setDiskAlias(RandomUtils.instance().nextString(10));
+ disk.setActive(true);
+ disk.setImageStatus(ImageStatus.OK);
+
+ return disk;
+ }
+
+ private static String createAliasReplacements(DiskImage... disks) {
+ // Take the first alias
+ StringBuilder msg = new StringBuilder("$diskAliases
").append(disks[0].getDiskAlias());
+
+ // And and the rest
+ for (int i = 1; i < disks.length; ++i) {
+ msg.append(", ").append(disks[i].getDiskAlias());
+ }
+
+ return msg.toString();
+ }
+
+ @Test
+ public void diskImagesNotIllegalBothOK() {
+ assertThat("Neither disk is illegal",
validator.diskImagesNotIllegal(), isValid());
+ }
+
+ @Test
+ public void diskImagesNotIllegalFirstIllegal() {
+ disk1.setImageStatus(ImageStatus.ILLEGAL);
+ assertThat(validator.diskImagesNotIllegal(),
+
both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).and(replacements(hasItem(createAliasReplacements(disk1)))));
+ }
+
+ @Test
+ public void diskImagesNotIllegalSecondtIllegal() {
+ disk2.setImageStatus(ImageStatus.ILLEGAL);
+ assertThat(validator.diskImagesNotIllegal(),
+
both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).and(replacements(hasItem(createAliasReplacements(disk2)))));
+ }
+
+ @Test
+ public void diskImagesNotIllegalBothIllegal() {
+ disk1.setImageStatus(ImageStatus.ILLEGAL);
+ disk2.setImageStatus(ImageStatus.ILLEGAL);
+ assertThat(validator.diskImagesNotIllegal(),
+
both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).and(replacements
+ (hasItem(createAliasReplacements(disk1, disk2)))));
+ }
+
+ @Test
+ public void diskImagesNotLockedBothOK() {
+ assertThat("Neither disk is Locked", validator.diskImagesNotLocked(),
isValid());
+ }
+
+ @Test
+ public void diskImagesNotLockedFirstLocked() {
+ disk1.setImageStatus(ImageStatus.LOCKED);
+ assertThat(validator.diskImagesNotLocked(),
+
both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED)).and(replacements(hasItem(createAliasReplacements(disk1)))));
+ }
+
+ @Test
+ public void diskImagesNotLockedSecondtLocked() {
+ disk2.setImageStatus(ImageStatus.LOCKED);
+ assertThat(validator.diskImagesNotLocked(),
+
both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED)).and(replacements(hasItem(createAliasReplacements(disk2)))));
+ }
+
+ @Test
+ public void diskImagesNotLockedBothLocked() {
+ disk1.setImageStatus(ImageStatus.LOCKED);
+ disk2.setImageStatus(ImageStatus.LOCKED);
+ assertThat(validator.diskImagesNotLocked(),
+
both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED)).and(replacements
+ (hasItem(createAliasReplacements(disk1, disk2)))));
+ }
+}
--
To view, visit http://gerrit.ovirt.org/14101
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c26d6cf37227aded9318523f0a807e19eeaa7bc
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
