Liron Aravot has uploaded a new change for review. Change subject: core: avoid pk violation during import vm ......................................................................
core: avoid pk violation during import vm When importing a vm (not as a new entity) while having a disk with the same id, pk violation occurs - this patch adds a validation to prevent it from happening. Bug-Url: https://bugzilla.redhat.com/902040 related to Bug-Url: https://bugzilla.redhat.com/890922 Change-Id: I84afa8167357b96ce47bc1340a64561a6d232b2d Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/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, 70 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/32/11332/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java index d313a85..33a6ee0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java @@ -291,6 +291,10 @@ return false; } + if (!validateNoDuplicateDiskImages(imageList)) { + return false; + } + setVmTemplateId(getVm().getVmtGuid()); if (!templateExists() || !checkTemplateInStorageDomain() || !checkImagesGUIDsLegal() || !canAddVm()) { return false; @@ -365,6 +369,31 @@ return true; } + protected boolean isDiskExists(Guid id) { + return getBaseDiskDao().exists(id); + } + + protected boolean validateNoDuplicateDiskImages(Iterable<DiskImage> images) { + if (!getParameters().isImportAsNewEntity()) { + List<String> existingDisksAliases = new ArrayList<String>(); + for (DiskImage diskImage : images) { + if (isDiskExists(diskImage.getId())) { + existingDisksAliases.add(diskImage.getDiskAlias()); + } + } + + if (!existingDisksAliases.isEmpty()) { + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST); + addCanDoActionMessage(String.format("$%1$s %2$s", + "diskAliases", + StringUtils.join(existingDisksAliases, ", "))); + return false; + } + } + + return true; + } + /** * Validates that that the required cluster exists. * @return <code>true</code> if the validation passes, <code>false</code> otherwise. diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java index 6aae9da..68efd8d 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java @@ -8,6 +8,8 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import java.util.ArrayList; import java.util.Arrays; @@ -76,6 +78,7 @@ doReturn(true).when(cmd).canAddVm(); doReturn(true).when(cmd).checkTemplateInStorageDomain(); doReturn(true).when(cmd).checkImagesGUIDsLegal(); + doReturn(true).when(cmd).validateNoDuplicateDiskImages(any(Iterable.class)); doReturn(createSourceDomain()).when(cmd).getSourceDomain(); doReturn(createStorageDomain()).when(cmd).getStorageDomain(any(Guid.class)); doReturn(Collections.<VM> singletonList(createVM())).when(cmd).getVmsFromExportDomain(); @@ -208,6 +211,37 @@ } @Test + public void testValidateNoDuplicateDiskImagesShouldCheckWhenExist() { + ImportVmParameters params = createParameters(); + params.setImportAsNewEntity(false); + ImportVmCommand cmd = spy(new ImportVmCommand(params)); + doReturn(true).when(cmd).isDiskExists(any(Guid.class)); + assertFalse(params.getVm().getImages().isEmpty()); + assertFalse(cmd.validateNoDuplicateDiskImages(params.getVm().getImages())); + } + + @Test + public void testValidateNoDuplicateDiskImagesShouldCheckWhenNotExist() { + ImportVmParameters params = createParameters(); + params.setImportAsNewEntity(false); + ImportVmCommand cmd = spy(new ImportVmCommand(params)); + doReturn(false).when(cmd).isDiskExists(any(Guid.class)); + assertFalse(params.getVm().getImages().isEmpty()); + assertTrue(cmd.validateNoDuplicateDiskImages(params.getVm().getImages())); + } + + @Test + public void testValidateNoDuplicateDiskImagesWhenAsNewEntity() { + ImportVmParameters params = createParameters(); + params.setImportAsNewEntity(true); + ImportVmCommand cmd = spy(new ImportVmCommand(params)); + doReturn(true).when(cmd).isDiskExists(any(Guid.class)); + assertFalse(params.getVm().getImages().isEmpty()); + assertTrue(cmd.validateNoDuplicateDiskImages(params.getVm().getImages())); + verify(cmd, never()).isDiskExists(any(Guid.class)); + } + + @Test public void testAliasGenerationByAddVmImagesAndSnapshotsWithoutCollapse() { ImportVmParameters params = createParameters(); params.setCopyCollapse(false); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java index 360cd41..3383b17 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java @@ -79,6 +79,7 @@ ACTION_TYPE_FAILED_VM_MAX_RESOURCE_EXEEDED, ACTION_TYPE_FAILED_VM_IN_PREVIEW, ACTION_TYPE_FAILED_DISKS_LOCKED, + ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST, ACTION_TYPE_FAILED_VM_IS_LOCKED, ACTION_TYPE_FAILED_VM_DURING_EXPORT, ACTION_TYPE_FAILED_VM_IMAGE_IS_ILLEGAL, 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 8d21da7..056bb24 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -129,6 +129,7 @@ ACTION_TYPE_FAILED_STOARGE_DOMAIN_IS_WRONG=Cannot ${action} ${type}. Provided wrong storage domain, which is not related to disk. ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a Snapshot. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. +ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action} ${type}: The following disks already exists: ${diskAliases}. Please import as a clone. ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is locked. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_DURING_EXPORT=Cannot ${action} ${type}: VM is being exported now. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_IMAGE_IS_ILLEGAL=Cannot ${action} ${type}. VM's Image might be corrupted. 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 28eb476..e8f24ba 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 @@ -323,6 +323,9 @@ @DefaultStringValue("Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes.") String ACTION_TYPE_FAILED_DISKS_LOCKED(); + @DefaultStringValue("Cannot ${action} ${type}: The following disks already exists: ${diskAliases}. Please import as a clone.") + String ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST(); + @DefaultStringValue("Cannot ${action} ${type}: VM is locked. Please try again in a few minutes.") String ACTION_TYPE_FAILED_VM_IS_LOCKED(); 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 9687db0..52b7fd1 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 @@ -127,6 +127,7 @@ VM_NOT_FOUND=VM not found ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a Snapshot. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. +ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action} ${type}: The following disks already exists: ${diskAliases}. Please import as a clone. ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is locked. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_DURING_EXPORT=Cannot ${action} ${type}: VM is being exported now. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_IMAGE_IS_ILLEGAL=Cannot ${action} ${type}. VM's Image might be corrupted. 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 b46b14c..8cfe6a8 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 @@ -126,6 +126,7 @@ VM_NOT_FOUND=VM not found ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a Snapshot. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. +ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action} ${type}: The following disks already exists: ${diskAliases}. Please import as a clone. ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is locked. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_DURING_EXPORT=Cannot ${action} ${type}: VM is being exported now. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_IMAGE_IS_ILLEGAL=Cannot ${action} ${type}. VM's Image might be corrupted. -- To view, visit http://gerrit.ovirt.org/11332 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I84afa8167357b96ce47bc1340a64561a6d232b2d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
