Allon Mureinik has uploaded a new change for review.
Change subject: core: Remove ImagesHandler.PerformImagesChecks
......................................................................
core: Remove ImagesHandler.PerformImagesChecks
After the recent changes, PerformImagesChecks had the following
functionality:
For each disk:
- GetImageInfo from VDSM
- if it does not exists, fail
- if it's status is different from the engine's status, update the
engine's status.
Analyzing ImageStatus' different values, this validation is useless:
- OK: The engine did not issue any command regarding this image. It may
have been manually removed from the storage itself, but that could
also happen right after the check finishes successfully.
- LOCKED: The engine is in the middle of performing a command on the
image. It's status in VDSM is meaningless, as it may change any moment
now.
- ILLEGAL/INVALID: The engine issued a command on the image, and it was
interrupted somehow (e.g., restart, network failure, etc.). The image
may have been partly or fully compromised, and it cannot be trusted
anyway.
In conclusion, this validation NEVER returns any useful information, and
thus could (and should!) be removed.
For good measures, an additional validation that the disks aren't
INVALID in the engine side was added to the commands where it was
missing, just to be safe.
Change-Id: I983a4c3a016909385ec16f21803ef3a82e4dcaf6
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/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/RemoveSnapshotCommand.java
5 files changed, 3 insertions(+), 67 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/14/14114/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 b07d090..8194847 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
@@ -221,15 +221,6 @@
return false;
}
- if (!ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- true,
- true,
- diskImagesToCheck)) {
- return false;
- }
-
Map<Guid, StorageDomain> storageDomains = new HashMap<Guid,
StorageDomain>();
Set<Guid> destImageDomains = getStorageGuidSet();
destImageDomains.removeAll(sourceImageDomainsImageMap.keySet());
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 57b76d7..76395ea 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
@@ -315,12 +315,7 @@
&& validate(vmValidator.vmNotRunningStateless())
&& validate(vmValidator.vmNotIlegal())
&& validate(diskImagesValidator.diskImagesNotLocked())
- && ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- true,
- true,
- disksList)
+ && validate(diskImagesValidator.diskImagesNotIllegal())
&& validate(vmValidator.vmNotLocked())
&& validate(sdValidator.allDomainsExistAndActive())
&& validate(sdValidator.allDomainsWithinThresholds());
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 d08abe4..2bf55da 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
@@ -446,29 +446,6 @@
return result;
}
- public static boolean PerformImagesChecks(
- List<String> messages,
- Guid storagePoolId,
- boolean checkImagesIllegalInVdsm,
- boolean checkImagesExist,
- List<DiskImage> diskImageList) {
-
- boolean returnValue = true;
- ArrayList<DiskImage> irsImages = new ArrayList<DiskImage>();
-
- if (returnValue && checkImagesExist) {
- boolean isImagesExist = isImagesExists(diskImageList,
storagePoolId, irsImages);
- if (!isImagesExist) {
- returnValue = false;
- ListUtils.nullSafeAdd(messages,
VdcBllMessages.ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST.toString());
- }
- }
- if (returnValue && checkImagesIllegalInVdsm) {
- returnValue = checkImagesLegalityInVdsm(messages, diskImageList,
irsImages);
- }
- return returnValue;
- }
-
/**
* @return A unique {@link Set} of all the storage domain IDs relevant to
all the given images
* @param images The images to get the storage domain IDs for
@@ -512,26 +489,6 @@
}
}
return validationResult.isValid();
- }
-
- private static boolean checkImagesLegalityInVdsm
- (List<String> messages, List<DiskImage> images, List<DiskImage>
irsImages) {
- boolean returnValue = true;
- int i = 0;
- for (DiskImage diskImage : images) {
- if (diskImage != null) {
- DiskImage image = irsImages.get(i++);
- if (image.getImageStatus() != ImageStatus.OK) {
- diskImage.setImageStatus(image.getImageStatus());
-
DbFacade.getInstance().getImageDao().update(diskImage.getImage());
- returnValue = false;
- ListUtils.nullSafeAdd(messages,
-
VdcBllMessages.ACTION_TYPE_FAILED_VM_IMAGE_IS_ILLEGAL.toString());
- break;
- }
- }
- }
- return returnValue;
}
public static void fillImagesBySnapshots(VM vm) {
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 82000cd..b72ba33 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
@@ -76,12 +76,7 @@
retValue = retValue &&
validate(new StoragePoolValidator(getStoragePool()).isUp()) &&
validate(diskImagesValidator.diskImagesNotLocked()) &&
- ImagesHandler.PerformImagesChecks(
- getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- true,
- true,
- diskImagesToValidate);
+ validate(diskImagesValidator.diskImagesNotIllegal());
ensureDomainMap(diskImages, getParameters().getStorageDomainId());
for(DiskImage disk : diskImages) {
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 1457b36..a8ea541 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
@@ -267,9 +267,7 @@
DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(imagesToValidate);
return validate(diskImagesValidator.diskImagesNotLocked()) &&
-
ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(),
- getVm().getStoragePoolId(),
- true, true, imagesToValidate);
+ validate(diskImagesValidator.diskImagesNotIllegal());
}
protected boolean validateImageNotInTemplate() {
--
To view, visit http://gerrit.ovirt.org/14114
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I983a4c3a016909385ec16f21803ef3a82e4dcaf6
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
