Ayal Baron has posted comments on this change.
Change subject: core: added information to ConnectStorageVDScommand logs
(#835546)
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
I agree with Allon, plus additional comments.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/ConnectStorageServerVDSCommandParameters.java
Line 47: for (storage_server_connections con : getConnectionList()) {
ah, I love the plural form in a singular object
Liron, if you're submitting another patch that moves the printing logic to
storage_server_connections, any chance you'd also change the name of the class?
Line 63: sb.append(con.getNfsTimeo());
not entirely correct as without current patch there is no type specific code,
plus the issue is much less severe.
Also, come to think of it, if iqn is not empty then the rest should not be
printed (doesn't iscsi have additional params to be printed?)
Likewise, if vfsType is not empty then mountOptions *should* be printed but
rest of the options shouldn't.
If neither of the above then only nfs options should be printed.
Maybe worthwhile setting a type field in connection upon creation.
--
To view, visit http://gerrit.ovirt.org/5748
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b613bd18ef9586040862cac56373705f3e65237
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
