Ayal Baron has posted comments on this change.
Change subject: core,webadmin: Failure to reconnect authenticated iSCSI LUNs
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java
Line 63: .getReturnValue();
Line 64:
Line 65: // connection details from VDS don't have CHAP credentials, so
apply them from the UI
Line 66: storage_server_connections sdConn =
getStorageDomain().getStorageStaticData().getConnection();
Line 67: if (sdConn != null && sdConn.getuser_name() != null &&
sdConn.getpassword() != null) {
actually chap supports shared secret (no username) so it should be:
sdConn != null && (sdConn.getuser_name() != null || sdConn.getpassword() !=
null)
why is this managed at lun level though? and if so, why are you using the same
credentials for all luns? there can be multiple sets of credentials as luns can
come from different targets
Line 68: for (LUNs lun : luns) {
Line 69: for (storage_server_connections lunConn :
lun.getLunConnections()) {
Line 70: lunConn.setuser_name(sdConn.getuser_name());
Line 71: lunConn.setpassword(sdConn.getpassword());
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeUtils.java
Line 24:
Line 25: public static Object asSingleResult(List<?> list) {
Line 26: return list.isEmpty() ? null : list.get(0);
Line 27: }
Line 28:
I'm guessing this is simply copy+paste? no code change?
Line 29: private static Log log = LogFactory.getLog(DbFacadeUtils.class);
Line 30:
Line 31: public static String encryptPassword(String password) {
Line 32: if (StringUtils.isEmpty(password)) {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java
Line 1618: .getSelectedItem() :
storageDomain.getStorageFormat());
Line 1619:
Line 1620: storageDomain.setstorage_name((String)
model.getName().getEntity());
Line 1621:
Line 1622: if ((Boolean) sanModel.getUseUserAuth().getEntity()) {
previous version did store the credentials in the db, what's changed that we
need this new code here?
Line 1623: storage_server_connections connection = new
storage_server_connections();
Line 1624: connection.setuser_name((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getUserName().getEntity() : ""); //$NON-NLS-1$
Line 1625: connection.setpassword((Boolean)
sanModel.getUseUserAuth().getEntity() ? (String)
sanModel.getPassword().getEntity() : ""); //$NON-NLS-1$
Line 1626: storageDomain.setConnection(connection);
--
To view, visit http://gerrit.ovirt.org/8344
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I15b4cba7418d9d818fb2fd69c708fdeb20942f9c
Gerrit-PatchSet: 2
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: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@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
