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 <[email protected]>
---
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 <[email protected]>
Gerrit-Reviewer: Fred Rolland <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
