Mike Kolesnik has posted comments on this change.

Change subject: core: macPool per DC, db changes
......................................................................


Patch Set 33:

(2 comments)

http://gerrit.ovirt.org/#/c/26795/33/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/MacPool.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/MacPool.java:

Line 83:         this.description = description;
Line 84:     }
Line 85: 
Line 86:     @Override
Line 87:     public boolean equals(Object o) {
> a) I didn't know for which fields we want to check identity, so I just add 
* Generally all fields are checked, not sure it's the best approach but that's 
what is done currently.
* Seems like you need to tune your IDE then since this code is not in parity 
with what Eclipse would generate which avoids problems such as null value, 
proper hashcode using prime numbers, and other errors that might be hiding 
here..
* In oVirt, Instance cannot equal it's father or son, it doesn't make sense..
Line 88:         if (this == o) {
Line 89:             return true;
Line 90:         }
Line 91:         if (!(o instanceof MacPool)) {


http://gerrit.ovirt.org/#/c/26795/33/packaging/dbscripts/upgrade/03_05_0720_add_mac_pool_ranges_to_storage_pool.sql
File 
packaging/dbscripts/upgrade/03_05_0720_add_mac_pool_ranges_to_storage_pool.sql:

Line 34: INSERT INTO mac_pool_ranges (mac_pool_id,
Line 35:                        from_mac,
Line 36:                        to_mac)
Line 37: SELECT (SELECT id FROM mac_pools),
Line 38:        mac_range[1],
> I'm against this. 
Normally you're right, but since upgrade is a delicate procedure that if fails 
would leave the system in an unusable state with need to get some professional 
support, it's better to cover a little extra ground than fight forest fires 
later..
Line 39:        mac_range[2]
Line 40: FROM (SELECT   string_to_array(unnest(string_to_array(option_value, 
',')),'-') as mac_range
Line 41:         FROM   vdc_options
Line 42:         WHERE  option_name = 'MacPoolRanges') as mac_ranges;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id30f3c384ecf933daaacdbdd6542e88afb98f7ca
Gerrit-PatchSet: 33
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