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 <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.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 7 files changed, 57 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/06/23706/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 fa9f0e0..acfd5a3 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 @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; +import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.exception.ExceptionUtils; import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.job.ExecutionHandler; @@ -117,8 +118,7 @@ } protected List<DiskImage> getDisksListForChecks() { - List<DiskImage> disksListForChecks = getParameters().getDisks() == null ? - getDisksList() : getParameters().getDisks(); + List<DiskImage> disksListForChecks = getDisksList(); if (getParameters().getDiskIdsToIgnoreInChecks().isEmpty()) { return disksListForChecks; } @@ -441,6 +441,10 @@ return false; } + if (!isSpecifiedDisksExist(getParameters().getDisks())) { + return false; + } + // Initialize validators. VmValidator vmValidator = createVmValidator(); SnapshotsValidator snapshotValidator = createSnapshotValidator(); @@ -501,6 +505,23 @@ validate(vmValidator.vmNotSavingRestoring()); } + private boolean isSpecifiedDisksExist(List<DiskImage> disks) { + if (disks != null) { + List<DiskImage> disksNotExistsInDb = ImagesHandler.imagesSubtract(getParameters().getDisks(), getDisksList()); + if (!disksNotExistsInDb.isEmpty()) { + List<String> disksNotExistsInDbIds = new ArrayList<>(disksNotExistsInDb.size()); + for (DiskImage diskImage : disksNotExistsInDb) { + disksNotExistsInDbIds.add(diskImage.getId().toString()); + } + + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_NOT_EXIST, + String.format("$%1$s %2$s", "diskIds", StringUtils.join(disksNotExistsInDbIds, ","))); + } + } + + return true; + } + @Override protected void setActionMessageParameters() { addCanDoActionMessage(VdcBllMessages.VAR__ACTION__CREATE); 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..8c44c80 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,26 @@ } @Test + public void testImagesDoesNotExist() { + setUpGeneralValidations(); + setUpDiskValidations(); + + DiskImage diskImage1 = getNewDiskImage(); + DiskImage diskImage2 = getNewDiskImage(); + DiskImage diskImage3 = getNewDiskImage(); + + List<DiskImage> diskImagesFromVm = new ArrayList<>(); + diskImagesFromVm.addAll(Arrays.asList(diskImage1, diskImage2)); + doReturn(diskImagesFromVm).when(cmd).getDisksList(); + + List<DiskImage> diskImagesFromParams = new ArrayList<>(); + diskImagesFromParams.addAll(Arrays.asList(diskImage1, diskImage3)); + cmd.getParameters().setDisks(diskImagesFromParams); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_DISKS_NOT_EXIST); + } + + @Test public void testAllDomainsExistAndActive() { setUpGeneralValidations(); setUpDiskValidations(); @@ -274,4 +295,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/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 43a16b7..47c3bed 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 3df97e7..c300b2d 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) 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 2fd13f8..cf723c6 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 @@ -2538,6 +2538,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) 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 0bfd487..5a140de 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 @@ -908,6 +908,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) 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 96a5a2e..74ac9c7 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 @@ -935,6 +935,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) 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/23706 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4d75ad139d048f83b08d9289e43d909b29f89695 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
