Greg Padgett has posted comments on this change.
Change subject: webadmin, engine: Use NFS v3 as default
......................................................................
Patch Set 1: (5 inline comments)
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/ConnectStorageServerVDSCommand.java
Line 40: }
Line 41: return result;
Line 42: }
Line 43:
Line 44: private static void addOrDefault(Map<String, String> map, Object
what, String alt, String name) {
Agree on both points. This was is to minimize code change while conforming to
the existing style/design.
A map with decorated/overloaded put method would indeed reduce the clutter here.
Line 45: map.put(name, what != null ? what.toString() : alt);
Line 46: }
Line 47:
Line 48: private static void addOrEmpty(Map<String, String> map, String
what, String name) {
Line 60: map.put(name, what.toString());
Line 61: }
Line 62: }
Line 63:
Line 64: public static Map<String, String> CreateStructFromConnection(final
storage_server_connections connection,
Most columns in the storage_server_connections database table are nullable, so
I suppose that's why. It would be a nice improvement to establish a more
explicit contract about which properties must be present.
Line 65: final storage_pool storage_pool) {
Line 66: // for information, see _connectionDict2ConnectionInfo in
vdsm/storage/hsm.py
Line 67: Map<String, String> con = new HashMap<String, String>();
Line 68: addOrDefault(con, connection.getid(), Guid.Empty.toString(),
"id");
Line 79: // For mnt_options and vfs_type - if they are null or empty
Line 80: // we should not send a key with an empty value
Line 81: addIfNotNullOrEmpty(con, connection.getMountOptions(),
"mnt_options");
Line 82: addIfNotNullOrEmpty(con, connection.getVfsType(),
"vfs_type");
Line 83: // For protocol_version - null in db indicates auto
version negotiation
The behavior definitely needs a better explanation here, and the comment is
incomplete, will fix. (null is 'auto' iff compatibility version is >3.0, else
null is 'vdsm default' (now v3) -- after upgrade, existing domains get the
'vdsm default' version)
Line 84: addOrDefault(con, connection.getNfsVersion(), "auto",
"protocol_version");
Line 85: addIfNotNullOrEmpty(con, connection.getNfsTimeo(),
"timeout");
Line 86: addIfNotNullOrEmpty(con, connection.getNfsRetrans(),
"retrans");
Line 87: }
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/NfsStorageModel.java
Line 141: // Initialize version list.
Line 142: setVersion(new ListModel());
Line 143:
Line 144: List<EntityModel> versionItems = new ArrayList<EntityModel>();
Line 145: versionItems.add(new EntityModel(constants.nfsVersion3(),
(short) 3));
It is, will add a comment.
Line 146: versionItems.add(new EntityModel(constants.nfsVersion4(),
(short) 4));
Line 147: versionItems.add(new
EntityModel(constants.nfsVersionAutoNegotiate(), null));
Line 148: getVersion().setItems(versionItems);
Line 149:
Line 143:
Line 144: List<EntityModel> versionItems = new ArrayList<EntityModel>();
Line 145: versionItems.add(new EntityModel(constants.nfsVersion3(),
(short) 3));
Line 146: versionItems.add(new EntityModel(constants.nfsVersion4(),
(short) 4));
Line 147: versionItems.add(new
EntityModel(constants.nfsVersionAutoNegotiate(), null));
Right, it's a nullable smallint in the db, and a Short in the engine. I'd
prefer to make the change to a string in a separate patch due to the amount of
code change required to make it happen. (The behavior change here would
probably get buried amidst all the other noise.)
Line 148: getVersion().setItems(versionItems);
Line 149:
Line 150: setRetransmissions(new EntityModel());
Line 151: setTimeout(new EntityModel());
--
To view, visit http://gerrit.ovirt.org/8242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I831ecc7050a5487d5365efb94342a3c170dddc6c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
