Tal Nisan has uploaded a new change for review. Change subject: core: Prevent usage of shareable disks on Gluster domains ......................................................................
core: Prevent usage of shareable disks on Gluster domains To avoid risk of split brain on Gluster domains this patch prevents the creation of shareable disks on Gluster domains as well as the update of existing disks to shareable. Also on creation of a new Gluster domain via webadmin there a message that tells to use Quorum on the Gluster server to ensure data integrity Change-Id: I6d741c93d32fa568c1d1f10429cb325a9b01c359 Bug-Url: https://bugzilla.redhat.com/1024654 Signed-off-by: Tal Nisan <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.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/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/GlusterStorageModel.java M frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/GlusterStorageView.java M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 10 files changed, 67 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/85/21585/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java index 28ae5f1..77c3420 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java @@ -84,6 +84,10 @@ return false; } + if (getParameters().getDiskInfo().isShareable() && getStorageDomain().getStorageType() == StorageType.GLUSTERFS) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN); + } + VM vm = getVm(); if (vm != null) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java index 29cd841..1500ed4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java @@ -32,6 +32,7 @@ import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; 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.businessentities.VmDevice; @@ -213,6 +214,12 @@ } if (isUpdatedToShareable(getOldDisk(), getNewDisk())) { + + StorageDomain storageDomain = getStorageDomainDAO().get(((DiskImage)getNewDisk()).getStorageIds().get(0)); + if (storageDomain.getStorageType() == StorageType.GLUSTERFS) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN); + } + List<DiskImage> diskImageList = getDiskImageDao().getAllSnapshotsForImageGroup(getOldDisk().getId()); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java index 6535c86..41c7ac9 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java @@ -36,7 +36,9 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskInterface; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StoragePool; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; @@ -56,6 +58,7 @@ import org.ovirt.engine.core.dao.DiskImageDAO; import org.ovirt.engine.core.dao.ImageDao; import org.ovirt.engine.core.dao.SnapshotDao; +import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.VdsDAO; import org.ovirt.engine.core.dao.VmDAO; @@ -92,6 +95,8 @@ private VmDeviceDAO vmDeviceDAO; @Mock private StoragePoolDAO storagePoolDao; + @Mock + private StorageDomainDAO storageDomainDao; @Mock private DbFacade dbFacade; @@ -150,9 +155,12 @@ @Test public void canDoActionFailedShareableDiskVolumeFormatUnsupported() throws Exception { UpdateVmDiskParameters parameters = createParameters(); - parameters.setDiskInfo(createShareableDisk(VolumeFormat.COW)); + DiskImage disk = createShareableDisk(VolumeFormat.COW); + StorageDomain storage = addNewStorageDomainToDisk(disk, StorageType.NFS); + parameters.setDiskInfo(disk); when(diskDao.get(diskImageGuid)).thenReturn(createDiskImage()); + when(storageDomainDao.get(storage.getId())).thenReturn(storage); initializeCommand(parameters); assertFalse(command.canDoAction()); @@ -162,14 +170,35 @@ } @Test + public void canDoActionFailedShareableDiskOnGlusterDomain() throws Exception { + UpdateVmDiskParameters parameters = createParameters(); + DiskImage disk = createShareableDisk(VolumeFormat.RAW); + StorageDomain storage = addNewStorageDomainToDisk(disk, StorageType.GLUSTERFS); + parameters.setDiskInfo(disk); + + when(diskDao.get(diskImageGuid)).thenReturn(createDiskImage()); + when(storageDomainDao.get(storage.getId())).thenReturn(storage); + initializeCommand(parameters); + + assertFalse(command.canDoAction()); + assertTrue(command.getReturnValue().getCanDoActionMessages().contains(VdcBllMessages.ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN.toString())); + } + + + @Test public void nullifiedSnapshotOnUpdateDiskToShareable() { UpdateVmDiskParameters parameters = createParameters(); - parameters.setDiskInfo(createShareableDisk(VolumeFormat.RAW)); + DiskImage disk = createShareableDisk(VolumeFormat.RAW); + parameters.setDiskInfo(disk); + StorageDomain storage = addNewStorageDomainToDisk(disk, StorageType.NFS); + parameters.setDiskInfo(disk); DiskImage oldDisk = createDiskImage(); oldDisk.setVmSnapshotId(Guid.newGuid()); when(diskDao.get(diskImageGuid)).thenReturn(oldDisk); + when(storageDomainDao.get(storage.getId())).thenReturn(storage); + initializeCommand(parameters); assertTrue(command.canDoAction()); @@ -336,6 +365,7 @@ doReturn(snapshotDao).when(command).getSnapshotDao(); doReturn(diskImageDao).when(command).getDiskImageDao(); doReturn(storagePoolDao).when(command).getStoragePoolDAO(); + doReturn(storageDomainDao).when(command).getStorageDomainDAO(); doReturn(vmStaticDAO).when(command).getVmStaticDAO(); doReturn(baseDiskDao).when(command).getBaseDiskDao(); doReturn(imageDao).when(command).getImageDao(); @@ -384,7 +414,6 @@ private void mockCtorRelatedDaoCalls(List<VM> vms) { mockGetForDisk(vms); mockGetVmsListForDisk(vms); - } private void mockVmsStoragePoolInfo(List<VM> vms) { @@ -481,6 +510,14 @@ return disk; } + private StorageDomain addNewStorageDomainToDisk(DiskImage diskImage, StorageType storageType) { + StorageDomain storage = new StorageDomain(); + storage.setId(Guid.newGuid()); + storage.setStorageType(storageType); + diskImage.setStorageIds(new ArrayList<Guid>(Arrays.asList(storage.getId()))); + return storage; + } + private VmDevice createVmDevice(Guid diskId, Guid vmId) { return new VmDevice(new VmDeviceId(diskId, vmId), VmDeviceGeneralType.DISK, 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 6437081..a5a169c 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 @@ -129,6 +129,7 @@ ACTION_TYPE_FAILED_VM_DISK_SNAPSHOT_IS_ATTACHED_TO_ANOTHER_VM(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_DISK_SNAPSHOT_IS_PLUGGED_TO_ANOTHER_VM(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT(ErrorType.CONFLICT), + ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISKS_LOCKED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISKS_ILLEGAL(ErrorType.INTERNAL_ERROR), ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS(ErrorType.INTERNAL_ERROR), 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 07ad791..358721e 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -145,6 +145,7 @@ ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot ${action} ${type}. The following VM's activated disks are disk snapshots:\n\ ${disksInfo}. \n\ Please deactivate them and try again. +ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN=Cannot ${action} ${type}. Shareable disks are not supported on Gluster domains. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The following attached disks are in ILLEGAL status: ${diskAliases} - please remove them and try again. ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS=Cannot ${action} ${type}. The following disks could not be moved: ${diskAliases}. Please make sure that all disks are active or inactive in the VM. 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 c5b2103..28bc129 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 @@ -355,6 +355,9 @@ @DefaultStringValue("Cannot ${action} ${type}. The following VM's activated disks are disk snapshots: \n ${disksInfo}. \nPlease deactivate them and try again.") String ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT(); + @DefaultStringValue("Cannot ${action} ${type}. Shareable disks are not supported on Gluster domains.") + String ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN(); + @DefaultStringValue("Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes.") String ACTION_TYPE_FAILED_DISKS_LOCKED(); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/GlusterStorageModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/GlusterStorageModel.java index 370b5c5..37a0f71 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/GlusterStorageModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/GlusterStorageModel.java @@ -7,6 +7,7 @@ import org.ovirt.engine.ui.uicommonweb.models.Model; import org.ovirt.engine.ui.uicommonweb.validation.IValidation; import org.ovirt.engine.ui.uicommonweb.validation.NotEmptyValidation; +import org.ovirt.engine.ui.uicompat.ConstantsManager; @SuppressWarnings("unused") public class GlusterStorageModel extends Model implements IStorageModel { @@ -76,6 +77,10 @@ mountOptions = value; } + public String getConfigurationMessage() { + return ConstantsManager.getInstance().getConstants().glusterDomainConfigurationMessage(); + } + public GlusterStorageModel() { diff --git a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java index 97e7612..24c55d1 100644 --- a/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java +++ b/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java @@ -1,6 +1,7 @@ package org.ovirt.engine.ui.uicompat; +import com.google.gwt.i18n.client.Constants; import org.ovirt.engine.core.common.businessentities.VmPool; public interface UIConstants extends com.google.gwt.i18n.client.Constants { @@ -2075,5 +2076,8 @@ @DefaultStringValue("Manage Policy Units") String managePolicyUnits(); + + @Constants.DefaultStringValue("For data integrity make sure that the server is configured with Quorum (both client and server Quorum)") + String glusterDomainConfigurationMessage(); } diff --git a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/GlusterStorageView.java b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/GlusterStorageView.java index 330e56c..c925376 100644 --- a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/GlusterStorageView.java +++ b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/GlusterStorageView.java @@ -70,6 +70,7 @@ Label mountOptionsLabel; @UiField + @Path(value = "configurationMessage") Label message; 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 913da5d..de877a0 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 @@ -142,6 +142,7 @@ ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot ${action} ${type}. The following VM's activated disks are disk snapshots:\n\ ${disksInfo}. \n\ Please deactivate them and try again. +ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN=Cannot ${action} ${type}. Shareable disks are not supported on Gluster domains. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The following attached disks are in ILLEGAL status: ${diskAliases} - please remove them and try again. ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS=Cannot ${action} ${type}. The following disks could not be moved: ${diskAliases}. Please make sure that all disks are active or inactive in the VM. -- To view, visit http://gerrit.ovirt.org/21585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6d741c93d32fa568c1d1f10429cb325a9b01c359 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
