Moti Asayag has uploaded a new change for review. Change subject: engine: Block multicast MAC addresses for vNICs ......................................................................
engine: Block multicast MAC addresses for vNICs The patch blocks the possibility to provide a non- unicast mac address to the vnic: neither broadcast or multicast mac addresses aren't allowed for a vnic. Change-Id: Ida4082be249391494053b56540d5458fecec65da Bug-Url: https://bugzilla.redhat.com/1011912 Signed-off-by: Moti Asayag <masa...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VmNic.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ValidationUtils.java M backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/network/VmNetworkInterfaceValidationTest.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/MacAddressValidation.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 15 files changed, 45 insertions(+), 43 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/91/19891/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java index d95c7ec..7f3b220 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java @@ -52,7 +52,7 @@ private List<DiskImage> _templateDisks; private StorageDomain sourceDomain; private Guid sourceDomainId = Guid.Empty; - private final static Pattern VALIDATE_MAC_ADDRESS = Pattern.compile(VmNic.VALID_MAC_ADDRESS_FORMAT); + private final static Pattern VALIDATE_MAC_ADDRESS = Pattern.compile(VmNic.UNICAST_MAC_ADDRESS_FORMAT); /** * Constructor for command creation when compensation is applied on startup diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java index eeb1f9d..0fe6229 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java @@ -24,6 +24,7 @@ public class MacPoolManager { + private static final String MAC_ADDRESS_MULTICAST_LSB = "13579bBdDfF"; private static final int HEX_RADIX = 16; private static final String INIT_ERROR_MSG = "{0}: Error in initializing MAC Addresses pool manager."; private static final MacPoolManager INSTANCE = new MacPoolManager(); @@ -150,13 +151,12 @@ } else if (value.length() < 12) { value = StringUtils.leftPad(value, 12, '0'); } - StringBuilder builder = new StringBuilder(); - for (int j = 0; j < value.length(); j += 2) { - builder.append(value.substring(j, j + 2)); - builder.append(":"); + + value = createMacAddress(value); + if (value == null) { + break; } - value = builder.toString(); - value = value.substring(0, value.length() - 1); + if (!availableMacs.contains(value)) { availableMacs.add(value); } @@ -167,6 +167,26 @@ return true; } + private String createMacAddress(String value) { + StringBuilder builder = new StringBuilder(); + + for (int j = 0; j < value.length(); j += 2) { + String group = value.substring(j, j + 2); + + // skip multi-cast MAC Addresses + if (j == 0 && StringUtils.contains(MAC_ADDRESS_MULTICAST_LSB, group.charAt(1))) { + return null; + } + + builder.append(group); + if (j + 2 < value.length()) { + builder.append(":"); + } + } + + return builder.toString(); + } + public String allocateNewMac() { String mac = null; lockObj.writeLock().lock(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java index 4fa2b7c..6b07345 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java @@ -1,7 +1,6 @@ package org.ovirt.engine.core.bll.network.vm; import java.util.List; -import java.util.regex.Pattern; import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.VmCommand; @@ -23,7 +22,6 @@ import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; -import org.ovirt.engine.core.common.utils.ValidationUtils; public abstract class AbstractVmInterfaceCommand<T extends AddVmInterfaceParameters> extends VmCommand<T> { @@ -74,12 +72,6 @@ Boolean allowDupMacs = Config.<Boolean> GetValue(ConfigValues.AllowDuplicateMacAddresses); return MacPoolManager.getInstance().isMacInUse(getMacAddress()) && !allowDupMacs ? new ValidationResult(VdcBllMessages.NETWORK_MAC_ADDRESS_IN_USE) - : ValidationResult.VALID; - } - - protected ValidationResult macAddressValid() { - return Pattern.matches(ValidationUtils.INVALID_NULLABLE_MAC_ADDRESS, getMacAddress()) - ? new ValidationResult(VdcBllMessages.NETWORK_INVALID_MAC_ADDRESS) : ValidationResult.VALID; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java index 9dc3eb4..8f5b89d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java @@ -136,10 +136,8 @@ return false; } - if (StringUtils.isNotEmpty(getMacAddress())) { - if (!validate(macAddressValid()) || !validate(macAvailable())) { - return false; - } + if (StringUtils.isNotEmpty(getMacAddress()) && !validate(macAvailable())) { + return false; } else if (MacPoolManager.getInstance().getAvailableMacsCount() <= 0) { addCanDoActionMessage(VdcBllMessages.MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES); return false; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java index c1d432d..b8a54ab 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java @@ -211,7 +211,7 @@ } macShouldBeChanged = !StringUtils.equals(oldIface.getMacAddress(), getMacAddress()); - if (macShouldBeChanged && (!validate(macAddressValid()) || !validate(macAvailable()))) { + if (macShouldBeChanged && !validate(macAvailable())) { return false; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VmNic.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VmNic.java index 844b44f..b3b50d3 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VmNic.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/VmNic.java @@ -14,7 +14,8 @@ * <code>VmNic</code> defines a type of {@link NetworkInterface} for instances of {@link VM}. */ public class VmNic extends NetworkInterface<VmNetworkStatistics> { - public static final String VALID_MAC_ADDRESS_FORMAT = "(\\p{XDigit}{2}:){5}\\p{XDigit}{2}"; + public static final String UNICAST_MAC_ADDRESS_FORMAT = "\\p{XDigit}[02468aAcCeE](:\\p{XDigit}{2}){5}"; + public static final String NON_NULLABLE_MAC_ADDRESS_FORMAT = "^.*(?<!(00:){5}00)$"; private static final long serialVersionUID = 7428150502868988886L; @@ -73,12 +74,15 @@ @NotNull(message = VmNic.VALIDATION_MESSAGE_MAC_ADDRESS_NOT_NULL, groups = { UpdateVmNic.class }) @Pattern.List({ - @Pattern(regexp = "(^$)|(" + VALID_MAC_ADDRESS_FORMAT + ")", - message = VmNic.VALIDATION_MESSAGE_MAC_ADDRESS_INVALID, + @Pattern(regexp = "(^$)|(" + UNICAST_MAC_ADDRESS_FORMAT + ")", + message = VALIDATION_MESSAGE_MAC_ADDRESS_INVALID, groups = { CreateEntity.class }), - @Pattern(regexp = VALID_MAC_ADDRESS_FORMAT, - message = VmNic.VALIDATION_MESSAGE_MAC_ADDRESS_INVALID, - groups = { UpdateEntity.class }) + @Pattern(regexp = UNICAST_MAC_ADDRESS_FORMAT, + message = VALIDATION_MESSAGE_MAC_ADDRESS_INVALID, + groups = { UpdateEntity.class }), + @Pattern(regexp = NON_NULLABLE_MAC_ADDRESS_FORMAT, + message = VALIDATION_MESSAGE_MAC_ADDRESS_INVALID, + groups = { CreateEntity.class, UpdateEntity.class }) }) @Override public String getMacAddress() { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 0559f6a..09a0092 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -415,7 +415,6 @@ NETWORK_VLAN_IN_USE(ErrorType.CONFLICT), NETWORK_ADDR_MANDATORY_IN_STATIC_IP(ErrorType.BAD_PARAMETERS), NETWORK_MAC_ADDRESS_IN_USE(ErrorType.CONFLICT), - NETWORK_INVALID_MAC_ADDRESS(ErrorType.BAD_PARAMETERS), NETWORK_INTERFACE_NAME_ALREADY_IN_USE(ErrorType.CONFLICT), NETWORK_INTERFACE_IN_USE_BY_VLAN(ErrorType.CONFLICT), NETWORK_ALREADY_ATTACHED_TO_INTERFACE(ErrorType.CONFLICT), diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ValidationUtils.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ValidationUtils.java index e4522c4..cfe3eab 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ValidationUtils.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/ValidationUtils.java @@ -24,8 +24,7 @@ public static final String NO_TRIMMING_WHITE_SPACES_PATTERN = "^$|\\S.*\\S"; public static final String IP_PATTERN = "^\\b((25[0-5]|2[0-4]\\d|[01]\\d\\d|\\d?\\d)\\.){3}(25[0-5]|2[0-4]\\d|[01]\\d\\d|\\d?\\d)\\b$|^$"; - /** Invalid mac address (for now just checking 00:00:00:00:00:00 */ - public static final String INVALID_NULLABLE_MAC_ADDRESS = "^(00:){5}00$"; + /** the mask will be replaced with zero-padded number in the generated names of the VMs in the pool, * see NameForVmInPoolGeneratorTest PoolNameValidationTest for valid and invalid expressions of this pattern */ public static final String POOL_NAME_PATTERN = "^[\\p{L}0-9._-]+[" + VmPool.MASK_CHARACTER + "]*[\\p{L}0-9._-]*$|^[\\p{L}0-9._-]*[" + VmPool.MASK_CHARACTER + "]*[\\p{L}0-9._-]+$"; diff --git a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/network/VmNetworkInterfaceValidationTest.java b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/network/VmNetworkInterfaceValidationTest.java index 7c66eaa..ba2caad 100644 --- a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/network/VmNetworkInterfaceValidationTest.java +++ b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/businessentities/network/VmNetworkInterfaceValidationTest.java @@ -23,7 +23,7 @@ private static final String VALID_NIC_NAME = "nic"; - private static final String VALID_MAC_ADDRESS = "01:23:45:67:89:ab"; + private static final String VALID_MAC_ADDRESS = "00:23:45:67:89:ab"; private static final Random random = new Random(); diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 7dd6b18..ea1dbb5 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -489,8 +489,6 @@ ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_CONNECTION_DATA_ILLEGAL=Cannot import Storage Domain. Internal Error: The connection data is illegal. ERROR_CANNOT_ADD_DEPRECATED_EXISTING_SAN_EXPORT_STORAGE_DOMAIN=Cannot import SAN Export Storage Domain as it is no longer supported. NETWORK_MAC_ADDRESS_IN_USE=MAC Address is already in use. -NETWORK_INVALID_MAC_ADDRESS=The specified MAC Address cannot be set.\n\ - -Please check MAC Address validity. NETWORK_INTERFACE_IN_USE_BY_VM=Cannot ${action} ${type}. There is at least one running VM that uses this Network. NETWORK_CLUSTER_NETWORK_IN_USE=Cannot ${action} ${type}. Network is being used by at least one Cluster. ERROR_CANNOT_DEACTIVATE_MASTER_WITH_NON_DATA_DOMAINS=Cannot deactivate a Master Storage Domain while there are ISO/Export active domains in the Data Center.\n\ diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java index 5a433dd..077dfd4 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java @@ -3,7 +3,6 @@ import java.util.regex.Pattern; import org.apache.commons.lang.StringUtils; -import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.config.entity.ConfigKey; /** @@ -16,7 +15,8 @@ */ public class MacAddressPoolRangesValueHelper extends StringValueHelper { - private static Pattern MAC_ADDRESS_PATTERN = Pattern.compile(VmNic.VALID_MAC_ADDRESS_FORMAT); + private static final String VALID_MAC_ADDRESS_FORMAT = "(\\p{XDigit}{2}:){5}\\p{XDigit}{2}"; + private static final Pattern MAC_ADDRESS_PATTERN = Pattern.compile(VALID_MAC_ADDRESS_FORMAT); @Override public ValidationResult validate(ConfigKey key, String value) { diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 732a8ad..2475c86 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -1336,9 +1336,6 @@ @DefaultStringValue("MAC Address is already in use.") String NETWORK_MAC_ADDRESS_IN_USE(); - @DefaultStringValue("The specified MAC Address cannot be set.\n-Please check MAC Address validity.") - String NETWORK_INVALID_MAC_ADDRESS(); - @DefaultStringValue("Cannot ${action} ${type}. There is at least one running VM that uses this Network.") String NETWORK_INTERFACE_IN_USE_BY_VM(); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/MacAddressValidation.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/MacAddressValidation.java index c3e5cda..53d1b39 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/MacAddressValidation.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/MacAddressValidation.java @@ -2,12 +2,11 @@ import org.ovirt.engine.ui.uicompat.ConstantsManager; -@SuppressWarnings("unused") public class MacAddressValidation extends RegexValidation { public MacAddressValidation() { - setExpression("^([\\dabcdefABCDEF]{2}:?){6}$"); //$NON-NLS-1$ + setExpression("^[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}$"); //$NON-NLS-1$ setMessage(ConstantsManager.getInstance().getConstants().invalidMacAddressMsg()); } } diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 167e1b3..ed7926a 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -475,8 +475,6 @@ ERROR_CANNOT_ADD_DEPRECATED_EXISTING_SAN_EXPORT_STORAGE_DOMAIN=Cannot import SAN Export Storage Domain as it is no longer supported. ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_LUNS_PROBLEM=Cannot import Storage Domain. Not all LUNs connected to Storage Domain. NETWORK_MAC_ADDRESS_IN_USE=MAC Address is already in use. -NETWORK_INVALID_MAC_ADDRESS=The specified MAC Address cannot be set.\n\ - -Please check MAC Address validity. NETWORK_INTERFACE_IN_USE_BY_VM=Cannot ${action} ${type}. There is at least one running VM that uses this Network. NETWORK_CLUSTER_NETWORK_IN_USE=Cannot ${action} ${type}. Network is being used by at least one Cluster. ERROR_CANNOT_DEACTIVATE_MASTER_WITH_NON_DATA_DOMAINS=Cannot deactivate a Master Storage Domain while there are ISO/Export active domains in the Data Center.\n\ diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 22c38a0..9944185 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -493,8 +493,6 @@ ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_CONNECTION_DATA_ILLEGAL=Cannot import Storage Domain. Internal Error: The connection data is illegal. ERROR_CANNOT_ADD_DEPRECATED_EXISTING_SAN_EXPORT_STORAGE_DOMAIN=Cannot import SAN Export Storage Domain as it is no longer supported. NETWORK_MAC_ADDRESS_IN_USE=MAC Address is already in use. -NETWORK_INVALID_MAC_ADDRESS=The specified MAC Address cannot be set.\n\ - -Please check MAC Address validity. NETWORK_INTERFACE_IN_USE_BY_VM=Cannot ${action} ${type}. There is at least one running VM that uses this Network. NETWORK_CLUSTER_NETWORK_IN_USE=Cannot ${action} ${type}. Network is being used by at least one Cluster. ERROR_CANNOT_DEACTIVATE_MASTER_WITH_NON_DATA_DOMAINS=Cannot deactivate a Master Storage Domain while there are ISO/Export active domains in the Data Center.\n\ -- To view, visit http://gerrit.ovirt.org/19891 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ida4082be249391494053b56540d5458fecec65da Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Moti Asayag <masa...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
