Liron Ar has posted comments on this change.

Change subject: core: avoid disconnecting from used connections
......................................................................


Patch Set 2:

(2 comments)

....................................................
Commit Message
Line 10: attempts to connect all the hosts to the lun. If one or more hosts fail
Line 11: to see the chosen luns for the extension - the engine attempts to
Line 12: disconnect all the hosts from the target.
Line 13: 
Line 14: In that scenario the connection isn't loaded from the DB, therefore 
it's
Done
Line 15: id will be always null - which can cause to disconnection from targets
Line 16: that are used by other luns that are in use.
Line 17: 
Line 18: Change-Id: I7008e64ebce57fbbdb350810a5ffca6e0a7800cc


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java
Line 118:         }
Line 119:         return (List<StorageServerConnections>) 
CollectionUtils.subtract(connections, toRemove);
Line 120:     }
Line 121: 
Line 122:     private void 
fillConnectionDetailsIfNeeded(StorageServerConnections connection) {
1. I prefer to leave it as is, this method just fills details and can be used 
for further usage then the Id, i prefer to decouple if from the id check..i 
wouldn't want it to be "fill details and return if there is a set id".

2. This is the storage helper, this class shouldn't be aware about who's the 
caller (in the case the extend command) and it will be used for any 
caller..therefore i prefer to keep the name as is

thanks!
Line 123:         if (connection.getid() == null) {
Line 124:             List<StorageServerConnections> dbConnections = 
DbFacade.getInstance().getStorageServerConnectionDao().getAllForConnection(connection);
Line 125:             if (!dbConnections.isEmpty()) {
Line 126:                 connection.setid(dbConnections.get(0).getid());


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7008e64ebce57fbbdb350810a5ffca6e0a7800cc
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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