Yaniv Bronhaim has posted comments on this change.
Change subject: engine: Improved duplicate host validation
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
Generally I agree with that validation. Few comments
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 317: returnValue = false;
Line 318: } else if (getVdsDAO().getByName(vdsName) != null) {
Line 319:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED);
Line 320: returnValue = false;
Line 321: } else if (getVdsDAO().getAllForHostname(hostName).size()
!= 0) {
This condition does the same validation as hostname leads to the host's
interface Address.. translating hostname to IP in each compare is more
accurate, not sure which is faster, and how updated vds_interface table.. as
host's ip address can be changed frequently
Line 322:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST);
Line 323: returnValue = false;
Line 324: } else {
Line 325: returnValue = returnValue &&
serverWithIpAddressExists(vds.getVdsGroupId(), hostName);
Line 320: returnValue = false;
Line 321: } else if (getVdsDAO().getAllForHostname(hostName).size()
!= 0) {
Line 322:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST);
Line 323: returnValue = false;
Line 324: } else {
why not to keep the 'else if' scopes and set
ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST here as all conditions?
Line 325: returnValue = returnValue &&
serverWithIpAddressExists(vds.getVdsGroupId(), hostName);
Line 326: returnValue = returnValue &&
validateSingleHostAttachedToLocalStorage();
Line 327:
Line 328: if (Config.<Boolean>
GetValue(ConfigValues.UseSecureConnectionWithServers)
Line 353: }
Line 354: return returnValue;
Line 355: }
Line 356:
Line 357: private boolean serverWithIpAddressExists(Guid vdsGroupId, String
hostName) {
In InitVdsOnUp we check the intefaces table, you could use the methods from
there, or move them to general location.
Line 358: try {
Line 359: if
(getInterfaceDao().getAllInterfacesWithIpAddress(vdsGroupId,
getHostAddress(hostName)).size() != 0) {
Line 360: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST);
Line 361: }
--
To view, visit http://gerrit.ovirt.org/14402
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I37c5ee3546a21400678c2f1f7d1e54bb726988b3
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
