Allon Mureinik has uploaded a new change for review.
Change subject: core: Remove checkIsValid from PerformImagesChecks
......................................................................
core: Remove checkIsValid from PerformImagesChecks
The recent changes to ImagesHandler.PerformImagesChecks have left the
checkIsValid parameter completely useless:
If it's true, both checkImagesIllegalInVdsm and checkImagesExist are
evaluated, and their respective tests are run if they are true.
If it's false, neither test is run.
In conclusion - it has no extra meaning, and can be removed.
This patch removes it from PerformImagesChecks' signature and all its
usages.
In case both other booleans are false, the function call is removed
altogether.
In case removing this function allows to remove some additional
parameters from the calling function (e.g., as is the can in
VmRunHandler), that is done as well.
Change-Id: I0055151c619dcf9e62144d280714c165a01408e8
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/CreateAllSnapshotsFromVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
13 files changed, 18 insertions(+), 80 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/06/14106/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 1ef8979..b07d090 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
@@ -226,7 +226,6 @@
getVm().getStoragePoolId(),
true,
true,
- true,
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 0fe973e..57b76d7 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
@@ -320,7 +320,6 @@
getVm().getStoragePoolId(),
true,
true,
- true,
disksList)
&& validate(vmValidator.vmNotLocked())
&& validate(sdValidator.allDomainsExistAndActive())
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 52f0c99..4a22ed5 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
@@ -166,14 +166,7 @@
&& validate(snapshotValidator.vmNotInPreview(getVmId()))
&& validate(new VmValidator(getVm()).vmDown())
&& validate(new
MultipleStorageDomainsValidator(getVm().getStoragePoolId(),
-
ImagesHandler.getAllStorageIdsForImageIds(getDisksBasedOnImage())).allDomainsExistAndActive())
- && ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- false,
- false,
- true,
- getDisksBasedOnImage()))) {
+
ImagesHandler.getAllStorageIdsForImageIds(getDisksBasedOnImage())).allDomainsExistAndActive())))
{
return false;
}
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 fc0696f..774471c 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
@@ -451,22 +451,18 @@
Guid storagePoolId,
boolean checkImagesIllegalInVdsm,
boolean checkImagesExist,
- boolean checkIsValid,
List<DiskImage> diskImageList) {
boolean returnValue = true;
- if (checkIsValid) {
- if (diskImageList.size() > 0) {
- returnValue = returnValue &&
- checkDiskImages(messages,
- storagePoolId,
- checkImagesIllegalInVdsm,
- checkImagesExist,
- diskImageList);
- } else if (checkImagesExist) {
- returnValue = false;
- ListUtils.nullSafeAdd(messages,
VdcBllMessages.ACTION_TYPE_FAILED_VM_HAS_NO_DISKS.toString());
- }
+ if (diskImageList.size() > 0) {
+ returnValue = checkDiskImages(messages,
+ storagePoolId,
+ checkImagesIllegalInVdsm,
+ checkImagesExist,
+ diskImageList);
+ } else if (checkImagesExist) {
+ returnValue = false;
+ ListUtils.nullSafeAdd(messages,
VdcBllMessages.ACTION_TYPE_FAILED_VM_HAS_NO_DISKS.toString());
}
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 4eceb8a..82000cd 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
@@ -81,7 +81,6 @@
getVm().getStoragePoolId(),
true,
true,
- true,
diskImagesToValidate);
ensureDomainMap(diskImages, getParameters().getStorageDomainId());
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 aa8b649..07e4855 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
@@ -216,14 +216,7 @@
List<DiskImage> diskList =
ImagesHandler.filterImageDisks(Arrays.asList(getDisk()), true, false);
DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskList);
- if (!validate(diskImagesValidator.diskImagesNotLocked()) ||
- !ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- storagePoolId,
- false,
- false,
- true,
- diskList)) {
+ if (!validate(diskImagesValidator.diskImagesNotLocked())) {
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 54c1f67..1457b36 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
@@ -269,7 +269,7 @@
return validate(diskImagesValidator.diskImagesNotLocked()) &&
ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(),
getVm().getStoragePoolId(),
- true, true, true, imagesToValidate);
+ 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 e5619ac..81eee30 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
@@ -170,16 +170,6 @@
if (!getParameters().getForce() &&
!validate(diskImagesValidator.diskImagesNotLocked())) {
return false;
}
-
- if (!ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- false,
- false,
- true,
- vmImages)) {
- return false;
- }
}
// Handle VM status with ImageLocked
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 66effab..0c5d9d9 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
@@ -344,14 +344,7 @@
List<DiskImage> diskImagesToCheck =
ImagesHandler.filterImageDisks(getImagesList(), true, false);
DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImagesToCheck);
- return validate(diskImagesValidator.diskImagesNotLocked()) &&
- ImagesHandler.PerformImagesChecks
- (getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- false,
- false,
- true,
- diskImagesToCheck);
+ return validate(diskImagesValidator.diskImagesNotLocked());
}
@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 33c50e9..f365520 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
@@ -231,14 +231,7 @@
new
MultipleStorageDomainsValidator(getVm().getStoragePoolId(), storageIds);
if (!validate(new StoragePoolValidator(getStoragePool()).isUp())
|| !validate(storageValidator.allDomainsExistAndActive())
- || !validate(storageValidator.allDomainsWithinThresholds())
- || !ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- false,
- false,
- true,
- diskImages)) {
+ ||
!validate(storageValidator.allDomainsWithinThresholds())) {
return false;
}
}
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 195791d..c9198b7 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
@@ -178,16 +178,6 @@
return failVmFree(messages, messagesToAdd);
}
- if (!ImagesHandler.PerformImagesChecks(
- messages,
- vm.getStoragePoolId(),
- false,
- false,
- true,
- vmImages)) {
- return failVmFree(messages);
- }
-
ValidationResult vmNotLockResult = new VmValidator(vm).vmNotLocked();
if (!vmNotLockResult.isValid()) {
return failVmFree(messages, vmNotLockResult.getMessage().name());
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 f3786f7..cab29a6 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
@@ -140,7 +140,7 @@
}
if (retValue) {
- retValue = performImageChecksForRunningVm(vm,
message, runParams, vmImages);
+ retValue = performImageChecksForRunningVm(message,
vmImages);
}
// Check if iso and floppy path exists
@@ -292,15 +292,9 @@
/**
* Check isValid only if VM is not HA VM
*/
- protected boolean performImageChecksForRunningVm
- (VM vm, List<String> message, RunVmParams runParams,
List<DiskImage> vmDisks) {
+ protected boolean performImageChecksForRunningVm(List<String> message,
List<DiskImage> vmDisks) {
DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(vmDisks);
- return validate(diskImagesValidator.diskImagesNotLocked(), message) &&
- ImagesHandler.PerformImagesChecks(message,
- vm.getStoragePoolId(),
- false, false,
- !vm.isAutoStartup() || !runParams.getIsInternal(),
- vmDisks);
+ return validate(diskImagesValidator.diskImagesNotLocked(), message);
}
/**
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 c9e245f..371b6df 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
@@ -325,9 +325,8 @@
anyListOf(String.class),
any(RunVmParams.class),
anyListOf(DiskImage.class));
-
doReturn(true).when(vmRunHandler).performImageChecksForRunningVm(any(VM.class),
+ doReturn(true).when(vmRunHandler).performImageChecksForRunningVm(
anyListOf(String.class),
- any(RunVmParams.class),
anyListOf(DiskImage.class));
}
--
To view, visit http://gerrit.ovirt.org/14106
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0055151c619dcf9e62144d280714c165a01408e8
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
