Allon Mureinik has uploaded a new change for review.
Change subject: core: Unfilter ImagesHandler.PerformImagesChecks
......................................................................
core: Unfilter ImagesHandler.PerformImagesChecks
Before this patch, ImagesHandler.PerformImagesChecks received a
collection of Disks, and filtered them so only DiskImages which aren't
sharable were evaluated.
This patch removes the filtering from this methods, and makes it the
responsibility of the calling method to perform any required filtering.
This patch also adds said filtering wherever relevant, so no behavior is
changed.
This achieves two goals:
1. Added flexibility, to allow possible validation on sharable disks.
2. A clearer API - it is now clear, in compile time, that this method
only performs validations on DiskImages, as opposed to the way it
was before the patch, where the function could be passed a faulty
(e.g., non-existent) LunDisk, which would just be silently ignored.
Change-Id: Icc93d1bb72daed053f0f37ae3c3f12efef915669
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
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/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/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
9 files changed, 14 insertions(+), 17 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/21/12321/1
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 1328390..8b79d6d 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
@@ -216,7 +216,7 @@
true,
true,
true,
- mImages)) {
+ ImagesHandler.filterEditableImageDisks(mImages))) {
return false;
}
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 64b9af5..792c048 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
@@ -2,7 +2,6 @@
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -276,7 +275,7 @@
moveOrCopyAllImageGroups(getVm().getId(), getDisksBasedOnImage());
}
- private Collection<DiskImage> getDisksBasedOnImage() {
+ private List<DiskImage> getDisksBasedOnImage() {
if (disksImages == null) {
disksImages =
ImagesHandler.filterEditableImageDisks(getVm().getDiskMap().values());
}
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 3775187..a5a75d6 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
@@ -437,22 +437,21 @@
boolean checkImagesIllegal,
boolean checkImagesExist,
boolean checkIsValid,
- Collection<? extends Disk> diskImageList) {
+ List<DiskImage> diskImageList) {
boolean returnValue = true;
- List<DiskImage> images = filterEditableImageDisks(diskImageList);
if (returnValue && checkImagesLocked) {
- returnValue = checkImagesLocked(messages, images);
+ returnValue = checkImagesLocked(messages, diskImageList);
}
if (returnValue && checkIsValid) {
- if (images.size() > 0) {
+ if (diskImageList.size() > 0) {
returnValue = returnValue &&
checkDiskImages(messages,
storagePoolId,
checkImagesIllegal,
checkImagesExist,
- images);
+ diskImageList);
} else if (checkImagesExist) {
returnValue = false;
ListUtils.nullSafeAdd(messages,
VdcBllMessages.ACTION_TYPE_FAILED_VM_HAS_NO_DISKS.toString());
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 e604d49..be04050 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
@@ -79,7 +79,7 @@
true,
true,
true,
- diskImages);
+
ImagesHandler.filterEditableImageDisks(diskImages));
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 de5ab6f..ac2cb2f 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
@@ -225,8 +225,6 @@
}
private boolean canRemoveVmImageDisk() {
- List<Disk> diskList = Arrays.asList(getDisk());
-
if (!listVms.isEmpty()) {
Guid storagePoolId = listVms.get(0).getStoragePoolId();
storage_pool sp = getStoragePoolDAO().get(storagePoolId);
@@ -234,6 +232,7 @@
return false;
}
+ List<Disk> diskList = Arrays.asList(getDisk());
if (!ImagesHandler.PerformImagesChecks(
getReturnValue().getCanDoActionMessages(),
storagePoolId,
@@ -241,7 +240,7 @@
false,
false,
true,
- diskList)) {
+ ImagesHandler.filterEditableImageDisks(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 39cb8d8..f99ed1e 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
@@ -260,9 +260,10 @@
}
protected boolean validateImages() {
+ List<DiskImage> imagesToValidate =
ImagesHandler.filterEditableImageDisks(getDiskDao().getAllForVm(getVmId()));
return
ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true, true, true, true, getDiskDao().getAllForVm(getVmId()));
+ true, true, true, true, imagesToValidate);
}
protected boolean validateImageNotInTemplate() {
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 a98f1ae..1db637d 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
@@ -346,7 +346,7 @@
false,
false,
true,
- getImagesList());
+
ImagesHandler.filterEditableImageDisks(getImagesList()));
}
@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 5db8238..606a1da 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
@@ -1,6 +1,5 @@
package org.ovirt.engine.core.bll;
-import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -213,7 +212,7 @@
}
updateVmDisksFromDb();
- Collection<DiskImage> diskImages =
+ List<DiskImage> diskImages =
ImagesHandler.filterImageDisks(getVm().getDiskMap().values(),
false, true);
if (!diskImages.isEmpty()) {
Set<Guid> storageIds =
ImagesHandler.getAllStorageIdsForImageIds(diskImages);
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 bf20606..b454892 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
@@ -212,7 +212,7 @@
false,
false,
true,
- disks)) {
+ vmImages)) {
return failVmFree(messages);
}
--
To view, visit http://gerrit.ovirt.org/12321
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icc93d1bb72daed053f0f37ae3c3f12efef915669
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
