Allon Mureinik has posted comments on this change.

Change subject: core: add canDoAction to remove storage connection
......................................................................


Patch Set 4: (2 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommand.java
Line 46:            }
Line 47:         }
Line 48:         else if(storageType.equals(StorageType.ISCSI)) {
Line 49:            List<String> domainNames = new ArrayList<>();
Line 50:            List<String> diskNames = new ArrayList<>();
It's a list you add elements to, and always iterate one by one - you never use 
random access.
Line 51:            //go to luns to storage connections map table, get it from 
there
Line 52:            List<LUNs> luns = 
getLunDao().getAllForStorageServerConnection(connectionId);
Line 53:            if(luns.size() > 0) {
Line 54:                 String volumeGroupId = null;


Line 71:                     domainNamesForMessage = 
prepareEntityNamesForMessage(domainNames);
Line 72:                     if(diskNames.size() < 1) {
Line 73:                         return 
prepareFailureMessageForDomains(domainNamesForMessage)  ;
Line 74:                      }
Line 75:                      else {
you don't need an else block - the if never terminates normally:

if (condition) {
   return bla;
}

retun bli;
Line 76:                         String diskNamesForMessage = 
prepareEntityNamesForMessage(diskNames);
Line 77:                         return 
prepareFailureMessageForDomainsAndDisks(domainNamesForMessage,diskNamesForMessage);
Line 78:                     }
Line 79:                 }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7a633b06195a7b8ca1cd4fc5023c05010a3161d
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to