Moti Asayag has posted comments on this change.
Change subject: core: Adds custom properties to VmNetworkInterface
......................................................................
Patch Set 15: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/UpdateVmTemplateInterfaceCommand.java
Line 67: if (!StringUtils.equals(oldIface.getName(),
getInterfaceName()) && !interfaceNameUnique(interfaces)) {
Line 68: return false;
Line 69: }
Line 70:
Line 71: // validate custom properties
ain't this a bit redundant ? it is well explained due to correctly naming the
function.
Line 72: if
(!nicValidator.validateCustomProperties(getReturnValue().getCanDoActionMessages()))
{
Line 73: return false;
Line 74: }
Line 75:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java
Line 168:
addCanDoActionMessage(VdcBllMessages.MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES);
Line 169: return false;
Line 170: }
Line 171:
Line 172: // validate custom properties
same as before.
Line 173: if
(!nicValidator.validateCustomProperties(getReturnValue().getCanDoActionMessages()))
{
Line 174: return false;
Line 175: }
Line 176:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
Line 130:
Line 131: private boolean propertiesRequiringVmUpdateDeviceWereUpdated() {
Line 132: return (!StringUtils.equals(oldIface.getNetworkName(),
getNetworkName()))
Line 133: || oldIface.isLinked() != getInterface().isLinked()
Line 134: ||
!StringUtils.equals(oldIface.getCustomProperties(),
getInterface().getCustomProperties());
as mentioned in previous patch, this comparison might fail since for properties:
aaa=bbb, xxx=yyy and xxx=yyy, aaa=bbb - the condition will fail.
Line 135: }
Line 136:
Line 137: @Override
Line 138: protected boolean canDoAction() {
Line 197: if (macShouldBeChanged && (!validate(macAddressValid()) ||
!validate(macAvailable()))) {
Line 198: return false;
Line 199: }
Line 200:
Line 201: // validate custom properties
same...
Line 202: if
(!nicValidator.validateCustomProperties(getReturnValue().getCanDoActionMessages()))
{
Line 203: return false;
Line 204: }
Line 205:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java
Line 58: protected String clusterVersion() {
Line 59: return String.format(CLUSTER_VERSION_REPLACEMENT_FORMAT,
version.getValue());
Line 60: }
Line 61:
Line 62: public boolean validateCustomProperties(List<String>
canDoActionMessages) {
s/canDoActionMessages/messages
since canDoAction are valid only in context of commands.
Line 63: // validate custom properties
Line 64: List<ValidationError> errors =
Line 65:
DevicePropertiesUtils.getInstance().validateProperties(version,
Line 66: VmDeviceGeneralType.INTERFACE,
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VmNetworkInterface.java
Line 213: if (plugged != other.plugged) {
Line 214: return false;
Line 215: }
Line 216:
Line 217: if (!ObjectUtils.objectsEqual(customProperties,
other.customProperties))
As mentioned previously, this is a problem and should be changed.
please maintain if block indentation.
Line 218: {
Line 219: return false;
Line 220: }
Line 221:
--
To view, visit http://gerrit.ovirt.org/14815
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I218db49043ddf0524178f6ff903e605bc0fe661a
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches