Daniel Erez has uploaded a new change for review. Change subject: core: validate disks existence on create snapshot ......................................................................
core: validate disks existence on create snapshot CreateAllSnapshotsFromVmCommand: Added validation for specified disks existence. * Included disks IDs in CanDo message. * Updated tests accordingly. Change-Id: I4d75ad139d048f83b08d9289e43d909b29f89695 Bug-Url: https://bugzilla.redhat.com/1057143 Signed-off-by: Daniel Erez <[email protected]> --- 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/validator/DiskImagesValidator.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 M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 9 files changed, 86 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/23862/1 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 2005bf9..445760a 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 @@ -116,8 +116,7 @@ } protected List<DiskImage> getDisksListForChecks() { - List<DiskImage> disksListForChecks = getParameters().getDisks() == null ? - getDisksList() : getParameters().getDisks(); + List<DiskImage> disksListForChecks = getDisksList(); if (getParameters().getDiskIdsToIgnoreInChecks().isEmpty()) { return disksListForChecks; } @@ -434,6 +433,10 @@ return false; } + if (!isSpecifiedDisksExist(getParameters().getDisks())) { + return false; + } + // Initialize validators. VmValidator vmValidator = createVmValidator(); SnapshotsValidator snapshotValidator = createSnapshotValidator(); @@ -494,6 +497,19 @@ validate(vmValidator.vmNotSavingRestoring()); } + private boolean isSpecifiedDisksExist(List<DiskImage> disks) { + if (disks == null || disks.isEmpty()) { + return true; + } + + DiskImagesValidator diskImagesValidator = createDiskImageValidator(disks); + if (!validate(diskImagesValidator.diskImagesNotExist())) { + return false; + } + + return true; + } + @Override protected void setActionMessageParameters() { addCanDoActionMessage(VdcBllMessages.VAR__ACTION__CREATE); 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..62e72fe 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 @@ -76,6 +76,27 @@ } /** + * Validates that the disks exists + * + * @return A {@link ValidationResult} with the validation information. + */ + public ValidationResult diskImagesNotExist() { + List<String> disksNotExistInDbIds = new ArrayList<String>(); + for (DiskImage diskImage : diskImages) { + if (!isDiskExists(diskImage.getId())) { + disksNotExistInDbIds.add(diskImage.getId().toString()); + } + } + + if (!disksNotExistInDbIds.isEmpty()) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_NOT_EXIST, + String.format("$diskIds %s", StringUtils.join(disksNotExistInDbIds, ", "))); + } + + return ValidationResult.VALID; + } + + /** * Validates that non of the disk are in the given {@link #status}. * * @param status 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..2a595f6 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 @@ -7,6 +7,7 @@ import static org.mockito.Mockito.spy; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.junit.Before; @@ -220,6 +221,24 @@ } @Test + public void testImagesDoesNotExist() { + setUpGeneralValidations(); + setUpDiskValidations(); + + DiskImage diskImage1 = getNewDiskImage(); + DiskImage diskImage2 = getNewDiskImage(); + + List<DiskImage> diskImagesFromParams = new ArrayList<>(); + diskImagesFromParams.addAll(Arrays.asList(diskImage1, diskImage2)); + cmd.getParameters().setDisks(diskImagesFromParams); + + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_NOT_EXIST)).when(diskImagesValidator) + .diskImagesNotExist(); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_DISKS_NOT_EXIST); + } + + @Test public void testAllDomainsExistAndActive() { setUpGeneralValidations(); setUpDiskValidations(); @@ -274,4 +293,10 @@ diskList.add(newDiskImage); return diskList; } + + private static DiskImage getNewDiskImage() { + DiskImage diskImage = new DiskImage(); + diskImage.setId(Guid.newGuid()); + return diskImage; + } } 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..81a6292 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 @@ -144,6 +144,21 @@ } @Test + public void diskImagesDontExist() { + doReturn(false).when(validator).isDiskExists(disk1.getId()); + doReturn(false).when(validator).isDiskExists(disk2.getId()); + assertThat(validator.diskImagesNotExist(), + both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_NOT_EXIST))); + } + + @Test + public void diskImagesExist() { + doReturn(true).when(validator).isDiskExists(disk1.getId()); + doReturn(true).when(validator).isDiskExists(disk2.getId()); + assertEquals(validator.diskImagesNotExist(), ValidationResult.VALID); + } + + @Test public void diskImagesNotLockedBothOK() { assertThat("Neither disk is Locked", validator.diskImagesNotLocked(), isValid()); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index e05d0fe..1355a1b 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -746,6 +746,7 @@ ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_DISK_NOT_EXIST(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_DISKS_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_REQUESTED_DISK_SIZE_IS_TOO_SMALL(ErrorType.BAD_PARAMETERS), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 8e33664..5bed487 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -939,6 +939,7 @@ ACTION_TYPE_FAILED_VM_SNAPSHOT_NOT_IN_PREVIEW=Cannot ${action} ${type} to a Snapshot that is not being previewed. Please select the correct Snapshot to restore to: Either the one being previewed, or the one before the preview. ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED=Cannot ${action} a shareable ${type} (${diskAliases}). This operation is not supported. ACTION_TYPE_FAILED_DISK_NOT_EXIST=Cannot ${action} ${type}. The specified disk does not exist. +ACTION_TYPE_FAILED_DISKS_NOT_EXIST=Cannot ${action} ${type}. The following disk(s) ID(s) does not exist: ${diskIds}. ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have been specified. ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK=Cannot ${action} ${type}. The following disk(s) are not attached to any VM: ${diskAliases}. ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK=Cannot ${action} ${type}. The selected disk is not a template disk. Only template disks can be copied. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index c34da6b..1109463 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -2539,6 +2539,9 @@ @DefaultStringValue("Cannot ${action} ${type}. The specified disk does not exist.") String ACTION_TYPE_FAILED_DISK_NOT_EXIST(); + @DefaultStringValue("Cannot ${action} ${type}. The following disk(s) ID(s) does not exist: ${diskIds}.") + String ACTION_TYPE_FAILED_DISKS_NOT_EXIST(); + @DefaultStringValue("Cannot ${action} ${type}. No disks have been specified.") String ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index e3f40a3..82b706c 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -909,6 +909,7 @@ ACTION_TYPE_FAILED_VM_SNAPSHOT_NOT_IN_PREVIEW=Cannot ${action} ${type} to a Snapshot that is not being previewed. Please select the correct Snapshot to restore to: Either the one being previewed, or the one before the preview. ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED=Cannot ${action} a shareable ${type}. This operation is not supported. ACTION_TYPE_FAILED_DISK_NOT_EXIST=Cannot ${action} ${type}. The specified disk does not exist. +ACTION_TYPE_FAILED_DISKS_NOT_EXIST=Cannot ${action} ${type}. The following disk(s) ID(s) does not exist: ${diskIds}. ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have been specified. ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK=Cannot ${action} ${type}. The following disk(s) are not attached to any VM: ${diskAliases}. ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK=Cannot ${action} ${type}. The selected disk is not a template disk. Only template disks can be copied. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 49c339a..7b95461 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -936,6 +936,7 @@ ACTION_TYPE_FAILED_VM_SNAPSHOT_NOT_IN_PREVIEW=Cannot ${action} ${type} to a Snapshot that is not being previewed. Please select the correct Snapshot to restore to: Either the one being previewed, or the one before the preview. ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED=Cannot ${action} a shareable ${type} (${diskAliases}). This operation is not supported. ACTION_TYPE_FAILED_DISK_NOT_EXIST=Cannot ${action} ${type}. The specified disk does not exist. +ACTION_TYPE_FAILED_DISKS_NOT_EXIST=Cannot ${action} ${type}. The following disk(s) ID(s) does not exist: ${diskIds}. ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have been specified. ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK=Cannot ${action} ${type}. The following disk(s) are not attached to any VM: ${diskAliases}. ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK=Cannot ${action} ${type}. The selected disk is not a template disk. Only template disks can be copied. -- To view, visit http://gerrit.ovirt.org/23862 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4d75ad139d048f83b08d9289e43d909b29f89695 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Daniel Erez <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
