Xavi Francisco has posted comments on this change.

Change subject: engine: Add custom mount options to NFS SD
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/27694/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java:

Line 84:                         if (nfsVersion != null) {
Line 85:                             entity.setNfsVersion(map(nfsVersion, 
null));
Line 86:                         }
Line 87:                     }
Line 88:                     if (storage.isSetMountOptions()) {
> Yes, that makes sense, it's just that there are no backend changes in the c
Alissa: you are right that there's no verification that the mount options are 
not duplicated or wrong. The thing is that these verifications should be 
implemented for all the storage domains that accept mount options, not only 
NFS. 
The mount command accepts the same option many times and neither raises and 
error nor complains so IMHO we have two options here: either we allow that and 
rely on mount to fail if the mount options are conflictive or build a parser 
for those options. This patch relies on the first situation. If we need to add 
the parser it probably should be done in another patch. I do not like the idea 
of having a mount options parser only for NFS.
Line 89:                         
entity.setMountOptions(storage.getMountOptions());
Line 90:                     }
Line 91:                     break;
Line 92:                 case LOCALFS:


-- 
To view, visit http://gerrit.ovirt.org/27694
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c7d51f5bf1ffb3491788b9fcda770a55b94cf50
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Xavi Francisco <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to