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/AddDiskToVmCommandTest.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/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 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 12 files changed, 99 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/38/21638/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 5f896ab..6e7395a 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 @@ -153,6 +153,7 @@ // vm agnostic checks returnValue = validate(new StorageDomainValidator(getStorageDomain()).isDomainExistAndActive()) && + !isShareableDiskOnGlusterDomain() && checkImageConfiguration() && hasFreeSpace(getStorageDomain()) && checkExceedingMaxBlockDiskSize() && @@ -171,6 +172,15 @@ return returnValue; } + private boolean isShareableDiskOnGlusterDomain() { + if (getParameters().getDiskInfo().isShareable() && getStorageDomain().getStorageType() == StorageType.GLUSTERFS) { + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN); + return true; + } + + return false; + } + private boolean canAddShareableDisk() { if (getParameters().getDiskInfo().isShareable()) { if (!Config.<Boolean> GetValue(ConfigValues.ShareableDiskEnabled, 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 f4ca71a..43894af 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,8 @@ 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.StorageDomainStatic; +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 +215,12 @@ } if (isUpdatedToShareable(getOldDisk(), getNewDisk())) { + + StorageDomainStatic sds = getStorageDomainStaticDAO().get(((DiskImage)getNewDisk()).getStorageIds().get(0)); + if (sds.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/AddDiskToVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java index c6445cf..48beff7 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskToVmCommandTest.java @@ -300,6 +300,26 @@ .contains(VdcBllMessages.SHAREABLE_DISK_IS_NOT_SUPPORTED_BY_VOLUME_FORMAT.toString())); } + @Test + public void canDoActionShareableDiskOnGlusterFails() { + DiskImage image = createShareableDiskImage(); + image.setvolumeFormat(VolumeFormat.RAW); + + AddDiskParameters parameters = createParameters(); + parameters.setDiskInfo(image); + + Guid storageId = Guid.newGuid(); + initializeCommand(storageId, parameters); + mockVm(); + mockStorageDomain(storageId, StorageType.GLUSTERFS, Version.v3_1); + mockStoragePoolIsoMap(); + + assertFalse(command.canDoAction()); + assertTrue(command.getReturnValue(). + getCanDoActionMessages(). + contains(VdcBllMessages.ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN.toString())); + } + /** * Initialize the command for testing, using the given storage domain id for the parameters. * @@ -417,6 +437,10 @@ return mockStorageDomain(storageId, availableSize, usedSize, StorageType.UNKNOWN, new Version()); } + private StorageDomain mockStorageDomain(Guid storageId, StorageType storageType, Version version) { + return mockStorageDomain(storageId, 6, 4, storageType, version); + } + private StorageDomain mockStorageDomain(Guid storageId, Version version) { return mockStorageDomain(storageId, 6, 4, StorageType.UNKNOWN, version); } 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 15ab978..3155dd2 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 @@ -34,7 +34,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; @@ -55,6 +57,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.StorageDomainStaticDAO; import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.VdsDAO; import org.ovirt.engine.core.dao.VmDAO; @@ -91,6 +94,8 @@ private VmDeviceDAO vmDeviceDAO; @Mock private StoragePoolDAO storagePoolDao; + @Mock + private StorageDomainStaticDAO storageDomainStaticDao; @Mock private DbFacade dbFacade; @@ -149,9 +154,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(storageDomainStaticDao.get(storage.getId())).thenReturn(storage.getStorageStaticData()); initializeCommand(parameters); assertFalse(command.canDoAction()); @@ -161,14 +169,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(storageDomainStaticDao.get(storage.getId())).thenReturn(storage.getStorageStaticData()); + 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(storageDomainStaticDao.get(storage.getId())).thenReturn(storage.getStorageStaticData()); + initializeCommand(parameters); assertTrue(command.canDoAction()); @@ -297,6 +326,7 @@ doReturn(snapshotDao).when(command).getSnapshotDao(); doReturn(diskImageDao).when(command).getDiskImageDao(); doReturn(storagePoolDao).when(command).getStoragePoolDAO(); + doReturn(storageDomainStaticDao).when(command).getStorageDomainStaticDAO(); doReturn(vmStaticDAO).when(command).getVmStaticDAO(); doReturn(baseDiskDao).when(command).getBaseDiskDao(); doReturn(imageDao).when(command).getImageDao(); @@ -345,7 +375,6 @@ private void mockCtorRelatedDaoCalls(List<VM> vms) { mockGetForDisk(vms); mockGetVmsListForDisk(vms); - } private void mockVmsStoragePoolInfo(List<VM> vms) { @@ -452,4 +481,12 @@ disk.setShareable(true); 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; + } } 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 5665c27..52dab11 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 @@ -126,6 +126,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 1814e73..360be84 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 24e647f..930266c 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 29237d7..4014184 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 { @@ -2123,5 +2124,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/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 10d3649..097a992 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 @@ -138,6 +138,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/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 e11d1ec..0243a07 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/21638 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6d741c93d32fa568c1d1f10429cb325a9b01c359 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Tal Nisan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
