Hello Fred Rolland, I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/37610 to review the following change. Change subject: engine: Do not allow NFS when creating POSIX SD ...................................................................... engine: Do not allow NFS when creating POSIX SD If the user tries to create a POSIX storage domain with NFS as VFS type, the operation will not be performed. The following message will be displayed: "Do not mount NFS storage by creating a POSIX compliant file system Storage Domain. Always create an NFS Storage Domain instead." Change-Id: I7ce3189b02ffa4cb20dffb31aebe420fe187a785 Bug-Url: https://bugzilla.redhat.com/1109055 Signed-off-by: Fred Rolland <froll...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionTestCommon.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.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 backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/StorageDomainMapperTest.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.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/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 11 files changed, 35 insertions(+), 12 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/10/37610/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java index ec92f61..58bc26f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectStorageToVdsCommand.java @@ -90,6 +90,10 @@ return failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE); } + if (storageType == StorageType.POSIXFS && conn.getVfsType().toLowerCase().equals("nfs")) { + return failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE); + } + if ((storageType == StorageType.POSIXFS || storageType == StorageType.NFS) && !validate(validateMountOptions())) { return false; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java index 92bb823..c203141 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java @@ -57,7 +57,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); @@ -82,7 +82,7 @@ @Test public void addNFSEmptyConn() { - StorageServerConnections newPosixConnection = createPosixConnection("", StorageType.POSIXFS, "nfs", "timeo=30"); + StorageServerConnections newPosixConnection = createPosixConnection("", StorageType.POSIXFS, "cifs", "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, @@ -94,7 +94,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); parameters.setVdsId(Guid.Empty); @@ -108,7 +108,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); newPosixConnection.setid(""); parameters.setStorageServerConnection(newPosixConnection); @@ -127,7 +127,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); newPosixConnection.setid(""); parameters.setStorageServerConnection(newPosixConnection); @@ -145,7 +145,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); newPosixConnection.setid(""); parameters.setStorageServerConnection(newPosixConnection); @@ -163,7 +163,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); newPosixConnection.setid(Guid.newGuid().toString()); parameters.setStorageServerConnection(newPosixConnection); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionTestCommon.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionTestCommon.java index 5feef4c..1a00b56 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionTestCommon.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionTestCommon.java @@ -123,6 +123,18 @@ } @Test + public void testPosixNFSVFSType() { + StorageServerConnections newPosixConnection = + createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", + StorageType.POSIXFS, + "nfs", + "timeo=30"); + parameters.setStorageServerConnection(newPosixConnection); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(getCommand(), + VdcBllMessages.VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE); + } + + @Test public void testConnectionWithInvalidMountOptionsFails() { StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java index 066de01..88de2bb 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java @@ -93,7 +93,7 @@ createPosixConnection( "multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); prepareCommand(); @@ -504,7 +504,7 @@ StorageServerConnections newPosixConnection = createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", StorageType.POSIXFS, - "nfs", + "cifs", "timeo=30"); parameters.setStorageServerConnection(newPosixConnection); 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 7e9a979..8ddc33f 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 @@ -870,6 +870,7 @@ VALIDATION_STORAGE_CONNECTION_INVALID(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_INVALID_PORT(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE(ErrorType.BAD_PARAMETERS), + VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_EMPTY_IQN(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION(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 d6980fe..a88ea79 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -810,6 +810,7 @@ VALIDATION_STORAGE_CONNECTION_INVALID=Mount path is illegal, please use [IP:/path or FQDN:/path] convention. VALIDATION_STORAGE_CONNECTION_INVALID_PORT=Invalid value for port, should be an integer greater than 0. VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE=VFS type cannot be empty. +VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE=Do not mount NFS storage by creating a POSIX compliant file system Storage Domain. Always create an NFS Storage Domain instead. VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY=Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}. VALIDATION_STORAGE_CONNECTION_EMPTY_IQN=Target details cannot be empty. VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION=${fieldName} field cannot be empty. diff --git a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/StorageDomainMapperTest.java b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/StorageDomainMapperTest.java index 22457c6..fb4d9c2 100644 --- a/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/StorageDomainMapperTest.java +++ b/backend/manager/modules/restapi/types/src/test/java/org/ovirt/engine/api/restapi/types/StorageDomainMapperTest.java @@ -196,7 +196,7 @@ connection.setid(connId.toString()); connection.setstorage_type(org.ovirt.engine.core.common.businessentities.StorageType.POSIXFS); connection.setconnection("1.2.135.255:/myshare/data"); - connection.setVfsType("nfs"); + connection.setVfsType("cifs"); connection.setMountOptions("timeo=30"); StorageConnection RESTConnection = new StorageConnection(); @@ -204,7 +204,7 @@ RESTConnection.setType(StorageType.POSIXFS.toString().toLowerCase()); RESTConnection.setAddress("1.2.135.255"); RESTConnection.setPath("/myshare/data"); - RESTConnection.setVfsType("nfs"); + RESTConnection.setVfsType("cifs"); RESTConnection.setMountOptions("timeo=30"); StorageServerConnections mappedResult = StorageDomainMapper.map(RESTConnection, null); 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 df810da..44cb24d 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 @@ -2836,6 +2836,9 @@ @DefaultStringValue("VFS type cannot be empty") String VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE(); + @DefaultStringValue("Do not mount NFS storage by creating a POSIX compliant file system Storage Domain. Always create an NFS Storage Domain instead.") + String VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE(); + @DefaultStringValue("Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}.") String VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY(); 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 36e786a..8e06a89 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 @@ -2034,7 +2034,7 @@ @DefaultStringValue("There can be only one bootable disk defined") String onlyOneBootableDisk(); - @DefaultStringValue("Enter a valid FS type (e.g. nfs/glusterfs/cifs/smbfs etc.") + @DefaultStringValue("Enter a valid FS type (e.g. glusterfs/cifs/smbfs etc.") String posixVfsTypeHint(); @DefaultStringValue("Enter additional Mount Options, as you would normally provide them to the mount command using the -o argument.\nThe mount options should be provided in a comma-separated list. See 'man mount' for a list of valid mount options.") 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 02c39b4..8a0937d 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 @@ -747,6 +747,7 @@ VALIDATION_STORAGE_CONNECTION_INVALID=Mount path is illegal, please use [IP:/path or FQDN:/path] convention. VALIDATION_STORAGE_CONNECTION_INVALID_PORT=Invalid value for port, should be an integer greater than 0. VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE=VFS type cannot be empty. +VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE=Do not mount NFS storage by creating a POSIX compliant file system Storage Domain. Always create an NFS Storage Domain instead. VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY=Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}. VALIDATION_STORAGE_CONNECTION_EMPTY_IQN=Target details cannot be empty. VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION=${fieldName} field cannot be empty. 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 bc1b606..914e41d 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 @@ -805,6 +805,7 @@ STORAGE_OPERATION_FAILED_SPM_NETWORK_PROBLEMS=Storage related operations can't be performed while the Storage Pool Manager is down.\nPlease make sure the Storage Pool Manager is up and running, and check network connectivity. VALIDATION_STORAGE_CONNECTION_INVALID=Mount path is illegal, please use [IP:/path or FQDN:/path] convention. VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE=VFS type cannot be empty. +VALIDATION_STORAGE_CONNECTION_POSIX_WITH_NFS_VFSTYPE=Do not mount NFS storage by creating a POSIX compliant file system Storage Domain. Always create an NFS Storage Domain instead. VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY=Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}. VALIDATION_STORAGE_CONNECTION_EMPTY_IQN=Target details cannot be empty. VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION=${fieldName} field cannot be empty. -- To view, visit http://gerrit.ovirt.org/37610 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7ce3189b02ffa4cb20dffb31aebe420fe187a785 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches