Federico Simoncelli has uploaded a new change for review. Change subject: core: change diskImagesNotIllegal in diskImagesAvailable ......................................................................
core: change diskImagesNotIllegal in diskImagesAvailable Change-Id: I2f04d33029bac5a5e179b2d9efa4d65d53768976 Signed-off-by: Federico Simoncelli <[email protected]> --- 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/ExportRepoImageCommand.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/GetDiskAlignmentCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.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 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/lsm/LiveMigrateVmDisksCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java 16 files changed, 51 insertions(+), 38 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/93/20993/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 72f3505..a92f8de 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 @@ -324,7 +324,7 @@ List<DiskImage> disksToCheck = ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getSourceVmFromDb().getId()), true, false, true); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(disksToCheck); - if (!validate(diskImagesValidator.diskImagesNotLocked())) { + if (!validate(diskImagesValidator.diskImagesAvailable())) { 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 6c059bc..c4b168e 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 @@ -311,8 +311,7 @@ List<DiskImage> diskImagesToCheck = ImagesHandler.filterImageDisks(mImages, true, false, true); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(diskImagesToCheck); - if (!validate(diskImagesValidator.diskImagesNotIllegal()) || - !validate(diskImagesValidator.diskImagesNotLocked())) { + if (!validate(diskImagesValidator.diskImagesAvailable())) { 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 364d65b..be2c184 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 @@ -411,8 +411,7 @@ if (disksList.size() > 0) { MultipleStorageDomainsValidator sdValidator = createMultipleStorageDomainsValidator(disksList); DiskImagesValidator diskImagesValidator = createDiskImageValidator(disksList); - if (!(validate(diskImagesValidator.diskImagesNotLocked()) - && validate(diskImagesValidator.diskImagesNotIllegal()) + if (!(validate(diskImagesValidator.diskImagesAvailable()) && validate(sdValidator.allDomainsExistAndActive()) && validate(sdValidator.allDomainsWithinThresholds()))) { return false; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java index 1bbc0b1..eb87b0b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java @@ -232,8 +232,7 @@ } DiskImagesValidator diskImagesValidator = new DiskImagesValidator(Arrays.asList(getDiskImage())); - if (!validate(diskImagesValidator.diskImagesNotIllegal()) - || !validate(diskImagesValidator.diskImagesNotLocked())) { + if (!validate(diskImagesValidator.diskImagesAvailable())) { 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 b9d3b22..599bc0a 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 @@ -98,8 +98,7 @@ VmHandler.updateDisksFromDb(getVm()); List<DiskImage> disksForExport = getDisksBasedOnImage(); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(disksForExport); - if (!validate(diskImagesValidator.diskImagesNotIllegal()) || - !validate(diskImagesValidator.diskImagesNotLocked())) { + if (!validate(diskImagesValidator.diskImagesAvailable())) { return false; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java index 16ee97e..9b6a973 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java @@ -104,8 +104,7 @@ if (getDiskType() == DiskStorageType.IMAGE) { DiskImagesValidator diskImagesValidator = new DiskImagesValidator(Arrays.asList((DiskImage) getDisk())); - if (!validate(diskImagesValidator.diskImagesNotLocked()) || - !validate(diskImagesValidator.diskImagesNotIllegal())) { + if (!validate(diskImagesValidator.diskImagesAvailable())) { return false; } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java index b1b5443..4035846 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java @@ -318,7 +318,7 @@ return validate(new SnapshotsValidator().vmNotDuringSnapshot(vm.getId())) // This check was added to prevent migration of VM while its disks are being migrated // TODO: replace it with a better solution - && validate(new DiskImagesValidator(ImagesHandler.getPluggedActiveImagesForVm(vm.getId())).diskImagesNotLocked()) + && validate(new DiskImagesValidator(ImagesHandler.getPluggedActiveImagesForVm(vm.getId())).diskImagesAvailable()) && SchedulingManager.getInstance().canSchedule(getVdsGroup(), getVm(), getVdsBlackList(), 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 05e2a0f..01bad53 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 @@ -80,8 +80,7 @@ DiskImagesValidator diskImagesValidator = new DiskImagesValidator(diskImagesToValidate); retValue = retValue && validate(new StoragePoolValidator(getStoragePool()).isUp()) && - validate(diskImagesValidator.diskImagesNotLocked()) && - validate(diskImagesValidator.diskImagesNotIllegal()); + validate(diskImagesValidator.diskImagesAvailable()); 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 40465f7..af43697 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 @@ -306,8 +306,7 @@ ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getVmId()), true, false, true); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(imagesToValidate); - return validate(diskImagesValidator.diskImagesNotLocked()) && - validate(diskImagesValidator.diskImagesNotIllegal()); + return validate(diskImagesValidator.diskImagesAvailable()); } 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 d0381ed..16d6b5a 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 @@ -408,7 +408,7 @@ List<DiskImage> diskImagesToCheck = ImagesHandler.filterImageDisks(getImagesList(), true, false, true); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(diskImagesToCheck); - return validate(diskImagesValidator.diskImagesNotLocked()); + return validate(diskImagesValidator.diskImagesAvailable()); } @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 485d486..b4451b1 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 @@ -238,8 +238,7 @@ } DiskImagesValidator diskImagesValidator = new DiskImagesValidator(diskImages); - if (!validate(diskImagesValidator.diskImagesNotIllegal()) || - !validate(diskImagesValidator.diskImagesNotLocked())) { + if (!validate(diskImagesValidator.diskImagesAvailable())) { return false; } 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 f3953ea..7649a8e 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 @@ -265,7 +265,7 @@ protected boolean checkImagesStatus() { List<DiskImage> disksToCheck = ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getVmId()), true, false, true); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(disksToCheck); - return validate(diskImagesValidator.diskImagesNotLocked()); + return validate(diskImagesValidator.diskImagesAvailable()); } 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 index a0ccb68..ad5e4a9 100644 --- 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 @@ -32,15 +32,6 @@ } /** - * 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. @@ -102,6 +93,37 @@ return ValidationResult.VALID; } + /** + * Validates that all the disks are available (e.g. not locked an legal). + * + * @return A {@link ValidationResult} with the validation information. If all the disks are available + * {@link ValidationResult#VALID} is returned. If one or more disks are in a different state, + * a {@link ValidationResult} with the relevant failure message and disks names is returned. + */ + public ValidationResult diskImagesAvailable() { + ValidationResult lockedDisks = diskImagesNotLocked(); + + if (lockedDisks != ValidationResult.VALID) { + return lockedDisks; + } + + List<String> illegalDisks = new ArrayList<String>(); + + for (DiskImage diskImage : diskImages) { + if (diskImage.getImageStatus() != ImageStatus.OK) { + illegalDisks.add(diskImage.getDiskAlias()); + } + } + + // ILLEGAL and any other (yet) unknown status are reported as illegal + if (!illegalDisks.isEmpty()) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL, + String.format("$diskAliases %s", StringUtils.join(illegalDisks, ", "))); + } + + return ValidationResult.VALID; + } + public ValidationResult diskImagesSnapshotsNotAttachedToOtherVms(boolean onlyPlugged) { LinkedList<String> pluggedDiskSnapshotInfo = new LinkedList<>(); for (DiskImage diskImage : diskImages) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java index a5600430..f8428d0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java @@ -129,7 +129,7 @@ */ public ValidationResult validateImagesForRunVm(VM vm, List<DiskImage> vmDisks) { return !vm.isAutoStartup() ? - new DiskImagesValidator(vmDisks).diskImagesNotLocked() : ValidationResult.VALID; + new DiskImagesValidator(vmDisks).diskImagesAvailable() : ValidationResult.VALID; } public ValidationResult validateIsoPath(VM vm, String diskPath, String floppyPath) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java index 76e8c6c..6a06ba3 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java @@ -199,7 +199,7 @@ setUpDiskValidations(); doReturn(getNonEmptyDiskList()).when(cmd).getDisksList(); doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED)).when(diskImagesValidator) - .diskImagesNotLocked(); + .diskImagesAvailable(); assertFalse(cmd.canDoAction()); assertTrue(cmd.getReturnValue() .getCanDoActionMessages() @@ -212,7 +212,7 @@ setUpDiskValidations(); doReturn(getNonEmptyDiskList()).when(cmd).getDisksList(); doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).when(diskImagesValidator) - .diskImagesNotIllegal(); + .diskImagesAvailable(); assertFalse(cmd.canDoAction()); assertTrue(cmd.getReturnValue() .getCanDoActionMessages() @@ -246,8 +246,7 @@ } private void setUpDiskValidations() { - doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesNotLocked(); - doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesNotIllegal(); + doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesAvailable(); doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsExistAndActive(); doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsWithinThresholds(); } 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 index 5a271ff..5cfbdae 100644 --- 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 @@ -94,20 +94,20 @@ @Test public void diskImagesNotIllegalBothOK() { - assertThat("Neither disk is illegal", validator.diskImagesNotIllegal(), isValid()); + assertThat("Neither disk is illegal", validator.diskImagesAvailable(), isValid()); } @Test public void diskImagesNotIllegalFirstIllegal() { disk1.setImageStatus(ImageStatus.ILLEGAL); - assertThat(validator.diskImagesNotIllegal(), + assertThat(validator.diskImagesAvailable(), both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).and(replacements(hasItem(createAliasReplacements(disk1))))); } @Test public void diskImagesNotIllegalSecondtIllegal() { disk2.setImageStatus(ImageStatus.ILLEGAL); - assertThat(validator.diskImagesNotIllegal(), + assertThat(validator.diskImagesAvailable(), both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).and(replacements(hasItem(createAliasReplacements(disk2))))); } @@ -115,7 +115,7 @@ public void diskImagesNotIllegalBothIllegal() { disk1.setImageStatus(ImageStatus.ILLEGAL); disk2.setImageStatus(ImageStatus.ILLEGAL); - assertThat(validator.diskImagesNotIllegal(), + assertThat(validator.diskImagesAvailable(), both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL)).and(replacements (hasItem(createAliasReplacements(disk1, disk2))))); } -- To view, visit http://gerrit.ovirt.org/20993 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2f04d33029bac5a5e179b2d9efa4d65d53768976 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
