Martin Mucha has posted comments on this change.

Change subject: core: extract current implementation to strategy.
......................................................................


Patch Set 4:

(4 comments)

answers.

http://gerrit.ovirt.org/#/c/26400/4//COMMIT_MSG
Commit Message:

Line 13: methods of MacPoolManager.java remain where they were, but for the
Line 14: actual work they delegate to MacPoolManagerOriginal.java
Line 15: 
Line 16: Change-Id: I75498bb68ec6dbe28650027116bf44a0797faf93
Line 17: Bug-Url: https://bugzilla.redhat.com/??????
> please update the bug number (or remove it completely)
Done


http://gerrit.ovirt.org/#/c/26400/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerOriginal.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerOriginal.java:

Line 94:             lockObj.writeLock().unlock();
Line 95:         }
Line 96:     }
Line 97: 
Line 98:     //to allow testing
> s/getVmNicInterfacesFromDB/getVmNicsFromDB or getVmInterfacesFromDB
Done
Line 99:     List<VmNic> getVmNicInterfacesFromDB() {
Line 100:         return DbFacade.getInstance().getVmNicDao().getAll();
Line 101:     }
Line 102: 


Line 132:         List<String> macAddresses = 
MacAddressRangeUtils.initRange(start, end, maxMacsInPool);
Line 133: 
Line 134:         if (macAddresses.size() + availableMacs.size() > 
maxMacsInPool) {
Line 135:             availableMacs.addAll(macAddresses.subList(0, 
maxMacsInPool - availableMacs.size()));
Line 136:             throw new MacPoolExceededMaxException();    //TODO MM: 
what's this?  why returning boolean value then?
> you're right: there is no option this method will return false. Its return 
Done
Line 137:         } else {
Line 138:             availableMacs.addAll(macAddresses);
Line 139: //            System.out.println("!!"+availableMacs.size());
Line 140:             return true;


Line 135:             availableMacs.addAll(macAddresses.subList(0, 
maxMacsInPool - availableMacs.size()));
Line 136:             throw new MacPoolExceededMaxException();    //TODO MM: 
what's this?  why returning boolean value then?
Line 137:         } else {
Line 138:             availableMacs.addAll(macAddresses);
Line 139: //            System.out.println("!!"+availableMacs.size());
> please remove.
Done
Line 140:             return true;
Line 141:         }
Line 142:     }
Line 143: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I75498bb68ec6dbe28650027116bf44a0797faf93
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <[email protected]>
Gerrit-Reviewer: Martin Mucha <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: [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