Muli Salem has posted comments on this change. Change subject: engine: Support Duplicate Mac Addresses ......................................................................
Patch Set 5: (13 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java Line 29: private static final MacPoolManager INSTANCE = new MacPoolManager(); Line 30: Line 31: private static Log log = LogFactory.getLog(MacPoolManager.class); Line 32: Line 33: // Maps that hold the allocated Mac addresses as keys, and counters as values Done Line 34: private final Map<String, Integer> allocatedMacs = new HashMap<String, Integer>(); Line 35: private final Map<String, Integer> allocatedCustomMacs = new HashMap<String, Integer>(); Line 36: Line 37: private final Set<String> availableMacs = new HashSet<String>(); Line 196: AuditLogDirector.log(logable, AuditLogType.MAC_ADDRESSES_POOL_NOT_INITIALIZED); Line 197: } Line 198: Line 199: private void internalFreeMac(String mac) { Line 200: if (allocatedMacs.get(mac) != null) { Done Line 201: removeMacFromMap(allocatedMacs, mac); Line 202: availableMacs.add(mac); Line 203: } else if (allocatedCustomMacs.get(mac) != null) { Line 204: removeMacFromMap(allocatedCustomMacs, mac); Line 199: private void internalFreeMac(String mac) { Line 200: if (allocatedMacs.get(mac) != null) { Line 201: removeMacFromMap(allocatedMacs, mac); Line 202: availableMacs.add(mac); Line 203: } else if (allocatedCustomMacs.get(mac) != null) { Done Line 204: removeMacFromMap(allocatedCustomMacs, mac); Line 205: } Line 206: } Line 207: Line 264: @SuppressWarnings("serial") Line 265: private class MacPoolExceededMaxException extends RuntimeException { Line 266: } Line 267: Line 268: private boolean allowDuplicate() { I prefer it for code neatness. Line 269: return Config.<Boolean> GetValue(ConfigValues.AllowDuplicateMacAddresses); Line 270: } Line 271: Line 272: private VmNetworkInterfaceDao getVmNetworkInterfaceDao() { Line 268: private boolean allowDuplicate() { Line 269: return Config.<Boolean> GetValue(ConfigValues.AllowDuplicateMacAddresses); Line 270: } Line 271: Line 272: private VmNetworkInterfaceDao getVmNetworkInterfaceDao() { Done Line 273: return DbFacade.getInstance().getVmNetworkInterfaceDao(); Line 274: } Line 275: Line 276: /* Line 274: } Line 275: Line 276: /* Line 277: * Adds the given Mac to the given Map, either by putting it in the Map, or incrementing its counter. If Mac is Line 278: * already taken and the ConfigValue AllowDuplicateMacAddresses is false, return false and does not add Mac to map Done Line 279: */ Line 280: private boolean addMacToMap(Map<String, Integer> macMap, String mac) { Line 281: boolean returnValue = false; Line 282: if (!macMap.containsKey(mac)) { Line 277: * Adds the given Mac to the given Map, either by putting it in the Map, or incrementing its counter. If Mac is Line 278: * already taken and the ConfigValue AllowDuplicateMacAddresses is false, return false and does not add Mac to map Line 279: */ Line 280: private boolean addMacToMap(Map<String, Integer> macMap, String mac) { Line 281: boolean returnValue = false; Done Line 282: if (!macMap.containsKey(mac)) { Line 283: macMap.put(mac, 1); // First time this mac is used Line 284: returnValue = true; Line 285: } else if (allowDuplicate()) { Line 279: */ Line 280: private boolean addMacToMap(Map<String, Integer> macMap, String mac) { Line 281: boolean returnValue = false; Line 282: if (!macMap.containsKey(mac)) { Line 283: macMap.put(mac, 1); // First time this mac is used Done Line 284: returnValue = true; Line 285: } else if (allowDuplicate()) { Line 286: incrementMacInMap(macMap, mac); Line 287: returnValue = true; Line 288: } Line 289: return returnValue; Line 290: } Line 291: Line 292: private void incrementMacInMap(Map<String, Integer> macMap, String mac) { Prefer to leave it for code clearity. Line 293: macMap.put(mac, macMap.get(mac) + 1); Line 294: } Line 295: Line 296: private void decrementMacInMap(Map<String, Integer> macMap, String mac) { Line 292: private void incrementMacInMap(Map<String, Integer> macMap, String mac) { Line 293: macMap.put(mac, macMap.get(mac) + 1); Line 294: } Line 295: Line 296: private void decrementMacInMap(Map<String, Integer> macMap, String mac) { Prefer to leave it for code clearity. Line 297: macMap.put(mac, macMap.get(mac) - 1); Line 298: } .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java Line 84: } Line 85: } finally { Line 86: setSucceeded(succeeded); Line 87: if (!succeeded) { Line 88: MacPoolManager.getInstance().freeMac(getParameters().getInterface().getMacAddress()); Done Line 89: } Line 90: } Line 91: } Line 92: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java Line 108: } finally { Line 109: setSucceeded(succeeded); Line 110: if (!succeeded && macAddressChanged) { Line 111: MacPoolManager.getInstance().addMac(oldIface.getMacAddress()); Line 112: MacPoolManager.getInstance().freeMac(getInterface().getMacAddress()); It was because the change in API of MacPoolManager. The newer patches again deal with Mac address and not with the iface itself, so this is necessary. I am no pushing in a new patch with getMacAddress(). Line 113: } Line 114: } Line 115: } Line 116: Line 155: public void rollback() { Line 156: super.rollback(); Line 157: if (macAddressChanged) { Line 158: MacPoolManager.getInstance().addMac(oldIface.getMacAddress()); Line 159: MacPoolManager.getInstance().freeMac(getInterface().getMacAddress()); Done Line 160: } Line 161: } Line 162: Line 163: @Override -- To view, visit http://gerrit.ovirt.org/10698 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie96eda19de2d3a44e24806095fb690e4eba41165 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Muli Salem <msa...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Muli Salem <msa...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches