Amit Aviram has uploaded a new change for review. Change subject: core: Preventing moving a shareable disk to a Gluster domain ......................................................................
core: Preventing moving a shareable disk to a Gluster domain Gluster domian does not support shareable disks- therefor MoveDisksCommand now prevents this action. Tests are also added. Change-Id: I37e500e86d2b6094bb257221a4ca899590aed610 Bug-Url: https://bugzilla.redhat.com/1135771 Signed-off-by: Amit Aviram <aavi...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.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, 67 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/98/35398/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java index 3e1a535..98a7d96 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java @@ -19,6 +19,7 @@ import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; @@ -71,6 +72,14 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED); } + for ( MoveDiskParameters param : getParameters().getParametersList() ) { + if (getDiskImageDao().get(param.getImageId()).isShareable()) { + if ( getStorageDomainDAO().get(param.getStorageDomainId()).getStorageType().equals(StorageType.GLUSTERFS) ) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS); + } + } + } + return true; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java index 03e82c8..d2011ef 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java @@ -21,11 +21,14 @@ import org.ovirt.engine.core.common.action.MoveDiskParameters; import org.ovirt.engine.core.common.action.MoveDisksParameters; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.dao.VmDAO; @RunWith(MockitoJUnitRunner.class) @@ -41,6 +44,9 @@ @Mock private VmDAO vmDao; + + @Mock + private StorageDomainDAO storageDomainDAO; /** * The command under test @@ -149,6 +155,19 @@ } @Test + public void moveShareableDiskToGlusterDomain() { + command.getParameters().setParametersList(createMoveDisksParameters()); + + // Create a shareable disk. + initDiskImage(diskImageId, true); + initStorageDomain(dstStorageId, StorageType.GLUSTERFS); + + command.updateParameters(); + assertFalse(command.canDoAction()); + } + + + @Test public void moveDiskAndLiveMigrateDisk() { Guid diskImageId1 = Guid.newGuid(); Guid diskImageId2 = Guid.newGuid(); @@ -213,7 +232,11 @@ } private void initDiskImage(Guid diskImageId) { - DiskImage diskImage = mockDiskImage(diskImageId); + initDiskImage(diskImageId, false); + } + + private void initDiskImage(Guid diskImageId, boolean isShareable) { + DiskImage diskImage = mockDiskImage(diskImageId, isShareable); when(diskImageDao.get(diskImageId)).thenReturn(diskImage); } @@ -224,11 +247,33 @@ } private DiskImage mockDiskImage(Guid diskImageId) { + return mockDiskImage(diskImageId, false); + } + + private DiskImage mockDiskImage(Guid diskImageId, boolean isShareable) { DiskImage diskImage = new DiskImage(); diskImage.setId(diskImageId); diskImage.setImageId(diskImageId); + diskImage.setShareable(isShareable); return diskImage; + } + + private void initStorageDomain(Guid dstStorageId) { + initStorageDomain(dstStorageId, StorageType.UNKNOWN); + } + + private void initStorageDomain(Guid dstStorageId, StorageType storageType) { + StorageDomain storageDomain = mockStorageDomain(dstStorageId, storageType); + when(storageDomainDAO.get(dstStorageId)).thenReturn(storageDomain); + } + + private StorageDomain mockStorageDomain(Guid dstStorageId, StorageType storageType) { + StorageDomain storageDomain = new StorageDomain(); + storageDomain.setId(dstStorageId); + storageDomain.setStorageType(storageType); + + return storageDomain; } /** Mock DAOs */ @@ -236,6 +281,7 @@ private void mockDaos() { mockVmDao(); mockDiskImageDao(); + mockStorageDomainDao(); } private void mockVmDao() { @@ -245,4 +291,8 @@ private void mockDiskImageDao() { doReturn(diskImageDao).when(command).getDiskImageDao(); } + + private void mockStorageDomainDao() { + doReturn(storageDomainDAO).when(command).getStorageDomainDAO(); + } } 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 1b652a4..9f79c99 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 @@ -833,6 +833,7 @@ ACTION_TYPE_FAILED_DISK_SNAPSHOTS_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_DISK_SNAPSHOTS_ACTIVE(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS(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), ACTION_TYPE_FAILED_CANNOT_RESIZE_READ_ONLY_DISK(ErrorType.CONFLICT), 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 daa45ed..1d2da37 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -1018,6 +1018,7 @@ ACTION_TYPE_FAILED_DISK_SNAPSHOTS_NOT_EXIST=Cannot ${action} ${type}. The following disk snapshot(s) ID(s) does not exist: ${diskSnapshotIds}. ACTION_TYPE_FAILED_DISK_SNAPSHOTS_ACTIVE=Cannot ${action} ${type}. The following disk snapshot(s) is active: ${diskSnapshotIds}. ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have been specified. +ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS=Cannot ${action} ${type}. Gluster domain does not support shareable disks. 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. ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME=Cannot ${action} ${type}. The source and target storage domains are the same. 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 9f09be8..3f71e1b 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 @@ -2737,6 +2737,9 @@ @DefaultStringValue("Cannot ${action} ${type}. No disks have been specified.") String ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED(); + @DefaultStringValue("Cannot ${action} ${type}. Gluster domain does not support shareable disks.") + String ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS(); + @DefaultStringValue("Cannot ${action} ${type}. The selected disk is not a template disk. Only template disks can be copied.") String ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK(); 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 a8aa981..e003f22 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 @@ -954,6 +954,7 @@ ACTION_TYPE_FAILED_DISK_SNAPSHOTS_NOT_EXIST=Cannot ${action} ${type}. The following disk snapshot(s) ID(s) does not exist: ${diskSnapshotIds}. ACTION_TYPE_FAILED_DISK_SNAPSHOTS_ACTIVE=Cannot ${action} ${type}. The following disk snapshot(s) is active: ${diskSnapshotIds}. ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have been specified. +ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS=Cannot ${action} ${type}. Gluster domain does not support shareable disks. 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. ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME=Cannot ${action} ${type}. The source and target storage domains are the same. 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 6edeffb..6c06e57 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 @@ -1008,6 +1008,7 @@ ACTION_TYPE_FAILED_DISK_SNAPSHOTS_NOT_EXIST=Cannot ${action} ${type}. The following disk snapshot(s) ID(s) does not exist: ${diskSnapshotIds}. ACTION_TYPE_FAILED_DISK_SNAPSHOTS_ACTIVE=Cannot ${action} ${type}. The following disk snapshot(s) is active: ${diskSnapshotIds}. ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have been specified. +ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS=Cannot ${action} ${type}. Gluster domain does not support shareable disks. 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. ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME=Cannot ${action} ${type}. The source and target storage domains are the same. -- To view, visit http://gerrit.ovirt.org/35398 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I37e500e86d2b6094bb257221a4ca899590aed610 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Amit Aviram <aavi...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches