Moti Asayag has posted comments on this change. Change subject: engine: Release Mac from Pool ......................................................................
Patch Set 5: (2 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java Line 179: if (!StringUtils.isEmpty(getMacAddress())) { Line 180: if (Pattern.matches(ValidationUtils.INVALID_NULLABLE_MAC_ADDRESS, getMacAddress())) { Line 181: addCanDoActionMessage(VdcBllMessages.NETWORK_INVALID_MAC_ADDRESS); Line 182: return false; Line 183: } pulling the mac allocation to the execute method will grant us better control on releasing it if the command fails Line 184: Line 185: Boolean allowDupMacs = Config.<Boolean> GetValue(ConfigValues.AllowDuplicateMacAddresses); Line 186: // this must be the last check because it adds the mac address to the pool Line 187: if (!MacPoolManager.getInstance().addMac(getMacAddress()) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java Line 108: Line 109: succeeded = updateHost(); Line 110: } finally { Line 111: setSucceeded(succeeded); Line 112: if (!succeeded && macAddressChanged) { there is a potential race here: free the mac on line 102 than it can be used by other Vnic and than you do updateHost() which is a long operation during that period the freed mac address is taken from the pool i think that releasing the mac addresses better be done at the end Line 113: MacPoolManager.getInstance().addMac(oldIface.getMacAddress()); Line 114: MacPoolManager.getInstance().freeMac(getInterface()); Line 115: } Line 116: } -- 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: 5 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