Moti Asayag has posted comments on this change.
Change subject: engine: Release Mac from Pool
......................................................................
Patch Set 6: (3 inline comments)
Basically the changes address the main issue.
See few comments inside.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java
Line 57:
(VmInterfaceType.forValue(getParameters().getInterface().getType()).getDescription()).toString());
Line 58:
this.setVmName(getVmStaticDAO().get(getParameters().getVmId()).getVmName());
Line 59:
Line 60: boolean succeeded = false;
Line 61: boolean macAddedToPool = false;
please add a space line before the try block
Line 62: try {
Line 63: if (StringUtils.isEmpty(getMacAddress())) {
Line 64:
getParameters().getInterface().setMacAddress(MacPoolManager.getInstance().allocateNewMac());
Line 65: macAddedToPool = true;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
Line 157: * <p/>
Line 158: * <li>The original MAC address is re-allocated.</li>
Line 159: * <li>The new MAC address is freed.</li>
Line 160: */
Line 161: @Override
since the command is marked as non-transactive and you already perform the
rollback logic within the execute method and since this method will not be
called - please remove it as part of this patch.
Line 162: public void rollback() {
Line 163: super.rollback();
Line 164: if (macShouldBeChanged) {
Line 165:
MacPoolManager.getInstance().addMac(oldIface.getMacAddress());
Line 243: }
Line 244: }
Line 245:
Line 246: macShouldBeChanged =
!StringUtils.equals(oldIface.getMacAddress(), getMacAddress());
Line 247: if (macShouldBeChanged) {
the validation below are repeated in both class in this patch.
Please consider moving them to be a validation methods inside their base class.
this can be done later as a separate patch (and later own extracting the
validations into a validator :-) )
Line 248: if
(Pattern.matches(ValidationUtils.INVALID_NULLABLE_MAC_ADDRESS,
getMacAddress())) {
Line 249:
addCanDoActionMessage(VdcBllMessages.NETWORK_INVALID_MAC_ADDRESS);
Line 250: return false;
Line 251: }
--
To view, visit http://gerrit.ovirt.org/10697
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcec5bbd129f4413ec077102379a417bfd945bc3
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Muli Salem <msa...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Muli Salem <msa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
