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

Reply via email to