Allon Mureinik has posted comments on this change.
Change subject: engine:Validate maximum number of hosts in DC.(#771699)
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
1. See inline comments
2. AFAIK, the same validation should be added to InstallVdsCommand. This is
also probably needed in ApproveVdsCommand's flow, but shouldn't be duplicated -
adding Doron for insight about oVirt node and host registration process.
3. Regarding race - agree with with mkublin.
4. Regarding DAO - agree with mkublin. I'd prefer having another method than
potentially impeding performance like this.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 298: returnValue = false;
I think an early return on false would look better throughout this method, but
I don't think /this/ patch is the place to do it.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsCommand.java
Line 194: protected boolean isNumberOfVdsInStoragePoolExceed(Guid
storagePoolId) {
s/exceed/exceeded/
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 590:
ACTION_TYPE_FAILED_EXCEEDED_MAXIMUM_NUMBER_OF_HOSTS_IN_DATA_CENTER=Can not
${action} ${type}. The maximum number of Hosts allowed in Data Center has been
exceeded.
can be present what this number is?
....................................................
Commit Message
Line 8:
please add a link to bugzilla, as per the commit message template.
Line 9: When user add a host, there should be a check whether the maximum
number of
s/user add a host/the user adds a host/ - or even better "when a host is added"
--
To view, visit http://gerrit.ovirt.org/6514
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1ef2ce160bc9ee4855b310c9d170ef7c14a0a17
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
