Mike Kolesnik has posted comments on this change.

Change subject: core: pool per DC
......................................................................


Patch Set 37:

(3 comments)

http://gerrit.ovirt.org/#/c/26799/37/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolPerDc.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolPerDc.java:

Line 51:                 initializeMacPool(macPool);
Line 52:             }
Line 53:             initialized = true;
Line 54:             log.info("MAC pool successfully initialized");
Line 55:         } catch(Throwable t) {
> will cause message is not printed in some scenarios, yes, improbable ones, 
I thought you took care of that OOME :)
Line 56:             log.error("MAC pool not initialized.");
Line 57:             throw t;
Line 58:         } finally {
Line 59:             lockObj.writeLock().unlock();


http://gerrit.ovirt.org/#/c/26799/37/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/MacPoolPerDcTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/MacPoolPerDcTest.java:

Line 36: 
Line 37:     @Rule
Line 38:     public ErrorCollector errorCollector = new ErrorCollector();
Line 39:     private MacPool macPoolA;
Line 40:     private StoragePool storagePoolA;
> well *class name* should be named DataCenter. Variable name for class Stora
Then by that logic you shouldn't suffix the mock fields with Mock.

Also when using primitives you need to name the same name as the primitive 
class, such as:

Boolean boolean;
Line 41:     private VmNic vmNicA;
Line 42: 
Line 43:     @Before
Line 44:     public void setUp() throws Exception {


Line 126:         pool.initialize();
Line 127: 
Line 128:         macPoolA.setAllowDuplicateMacAddresses(true);
Line 129:         pool.modifyPool(macPoolA);
Line 130:         //should not fail.
> well, you can't. These methods are not and were never not part of pool api(
You can allocate new mac address from the pool and then try to allocate it 
explicitly.

Also for other range you can just give the start of range address for 
allocation and see that it succeeds..

No need to introduce any new method or do any reflection..
Line 131:     }
Line 132: 
Line 133:     @Test
Line 134:     public void testModifyOfNotExistingMacPool() throws Exception {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If522a0b0d5f810281bed010b46b5242f7dbdcc29
Gerrit-PatchSet: 37
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: [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