Mike Kolesnik has posted comments on this change. Change subject: core: macPool per DC, db changes ......................................................................
Patch Set 16: (11 comments) http://gerrit.ovirt.org/#/c/26795/16/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 8: Line 9: import org.hibernate.validator.constraints.NotEmpty; Line 10: import org.ovirt.engine.core.compat.Guid; Line 11: Line 12: public class MacPool implements BusinessEntity<Guid>, Commented, Serializable{ Why do you need a comment? Line 13: private Guid id; Line 14: Line 15: @NotNull Line 16: private String name; Line 12: public class MacPool implements BusinessEntity<Guid>, Commented, Serializable{ Line 13: private Guid id; Line 14: Line 15: @NotNull Line 16: private String name; Does a private pool has to have a name? Line 17: Line 18: private boolean shared; Line 19: Line 20: private boolean allowDuplicateMacAddresses; Line 25: private Set<MacRange> ranges = new HashSet<MacRange>(); Line 26: Line 27: @Override Line 28: public Guid getId() { Line 29: return this.id; No need to use "this" Line 30: } Line 31: Line 32: @Override Line 33: public void setId(Guid id) { http://gerrit.ovirt.org/#/c/26795/16/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/MacRange.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/MacRange.java: Line 5: import javax.validation.constraints.NotNull; Line 6: Line 7: import org.ovirt.engine.core.compat.Guid; Line 8: Line 9: public class MacRange implements BusinessEntity<Guid>, Commented, Serializable{ Why do you need it to be commented? Line 10: private Guid id; Line 11: private Guid macPoolId; Line 12: Line 13: @NotNull Line 10: private Guid id; Line 11: private Guid macPoolId; Line 12: Line 13: @NotNull Line 14: private String macFrom; //TODO MM: change to PgMacaddr Why do you want to use PgMacaddr ? Why should business logic be aware of DB internal implementation? Line 15: Line 16: @NotNull Line 17: private String macTo; Line 18: private String comment; http://gerrit.ovirt.org/#/c/26795/16/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StoragePool.java: Line 50: Line 51: private QuotaEnforcementTypeEnum quotaEnforcementType; Line 52: Line 53: @NotNull(groups = {CreateEntity.class, UpdateEntity.class}) Line 54: private MacPool macPool; You should hold the ID of the pool only, no reason to fetch this data each time a DC is fetched. Line 55: Line 56: public StoragePool() { Line 57: id = Guid.Empty; Line 58: status = StoragePoolStatus.Uninitialized; http://gerrit.ovirt.org/#/c/26795/16/packaging/dbscripts/create_tables.sql File packaging/dbscripts/create_tables.sql: Why change this file? http://gerrit.ovirt.org/#/c/26795/16/packaging/dbscripts/upgrade/03_05_0300_add_mac_pool_ranges_to_storage_pool.sql File packaging/dbscripts/upgrade/03_05_0300_add_mac_pool_ranges_to_storage_pool.sql: Line 5: allow_duplicate_mac_addresses BOOLEAN NOT NULL, Line 6: comment CHARACTER VARYING(255) Line 7: ); Line 8: Line 9: CREATE TABLE default_mac_pool ( Why do you add this table? Currently there is no request or design to be able to select the default pool, I think it can be identified by ID only or maybe a field in the mac_pool table which is false by default and not changeable. Line 10: id UUID NOT NULL PRIMARY KEY, Line 11: mac_pool_id UUID NOT NULL REFERENCES mac_pool (id) ON DELETE RESTRICT Line 12: ); Line 13: Line 13: Line 14: CREATE TABLE mac_range ( Line 15: id UUID NOT NULL PRIMARY KEY, Line 16: mac_pool_id UUID NOT NULL REFERENCES mac_pool (id) ON DELETE CASCADE, Line 17: from_mac MACADDR NOT NULL, Is this type supported by our DAL? Line 18: to_mac MACADDR NOT NULL, Line 19: comment CHARACTER VARYING(255) Line 20: ); Line 21: Line 24: INSERT INTO mac_pool (id, name, shared, allow_duplicate_mac_addresses, comment) SELECT Line 25: uuid_generate_v1(), Line 26: 'Default', Line 27: TRUE, Line 28: FALSE, You should take the actual value from the config option. Line 29: 'Default shared MAC pool'; Line 30: Line 31: INSERT INTO default_mac_pool (id, mac_pool_id) select uuid_generate_v1(), ((select id from mac_pool limit 1)); Line 32: Line 33: INSERT INTO mac_range (id, mac_pool_id, from_mac, to_mac) SELECT Line 34: uuid_generate_v1(), Line 35: (select mac_pool_id from default_mac_pool limit 1), Line 36: '00:1a:4a:4:f1:00', Line 37: '00:1a:4a:4:f1:ff'; You should take the actual ranges from the config option and insert them all. Line 38: Line 39: update storage_pool set mac_pool_id=(select mac_pool_id from default_mac_pool limit 1); Line 40: alter table storage_pool alter COLUMN mac_pool_id SET NOT NULL; -- 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: 16 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
