Mike Kolesnik has posted comments on this change.
Change subject: engine: Prevent management network change
......................................................................
Patch Set 1: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java
Line 316: }
Line 317: }
Line 318:
Line 319: private boolean
managementNetworkModifiedCorrectly(VdsNetworkInterface iface) {
Line 320: if (!Config.<String>
GetValue(ConfigValues.ManagementNetwork).equals(iface.getNetworkName())) {
I don't think this check should be inside, rather outside in the if that is
calling this method.
Also, how about extracting this to method in NetworkUtils (there is already a
very similar method there isManagementNetwork(Network) that you can extract a
method isManagementNetwork(String) method from)?
Line 321: return true;
Line 322: }
Line 323:
Line 324: if (iface.getBootProtocol() == NetworkBootProtocol.STATIC_IP
Line 320: if (!Config.<String>
GetValue(ConfigValues.ManagementNetwork).equals(iface.getNetworkName())) {
Line 321: return true;
Line 322: }
Line 323:
Line 324: if (iface.getBootProtocol() == NetworkBootProtocol.STATIC_IP
Can you please documents the reason for checking these checks in the method
javadoc?
Line 325: &&
vds.getHostName().matches(ValidationUtils.IP_PATTERN)) {
Line 326: return StringUtils.equals(vds.getHostName(),
iface.getAddress());
Line 327: }
Line 328:
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java
Line 37: import org.ovirt.engine.core.utils.MockConfigRule;
Line 38: import org.ovirt.engine.core.utils.RandomUtils;
Line 39:
Line 40: @RunWith(MockitoJUnitRunner.class)
Line 41: public class SetupNetworksHelperTest {
Please add a test to cover this behavior.
Line 42:
Line 43: private static final String BOND_NAME = "bond0";
Line 44:
Line 45: @Rule
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 457: NON_VM_NETWORK_CANNOT_SUPPORT_STP=Cannot ${action} ${type}. STP can
only be enabled on VM Networks.
Line 458: NETWORK_MTU_DIFFERENCES=Cannot ${action} ${type}. The following
Logical Networks don't have the same MTU value: ${NETWORK_MTU_DIFFERENCES_LIST}.
Line 459: NETWORK_MTU_OVERRIDE_NOT_SUPPORTED=Cannot ${action} ${type}.
Overriding MTU is not supported for this Data Center's compatibility version.
Line 460: ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action}
${type}. The management network '${NetworkName}' must be required, please
change the network to be required and try again.
Line 461: MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action}
${type}. The management network address cannot be modified without reinstalling
the host, since this address was used to create the host's certification.
Key should be called
ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED since it
contains the action/type replacements
Line 462: CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image
cannot be used in Preview command.
Line 463: CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\
Line 464: -Please check configuration entry name.
Line 465: TAGS_SPECIFIED_TAG_CANNOT_BE_THE_PARENT_OF_ITSELF=Operation canceled,
recursive Tag hierarchy cannot be defined.
....................................................
Commit Message
Line 14: the engine to the host. The host should be reinstalled if wishes
Line 15: to modify the management network address.
Line 16:
Line 17: Change-Id: If98a5853385ad484dfa6e5392797f96daeaaa381
Line 18: Bug-Url: https://bugzilla.redhat.com/893411
This bug is private
--
To view, visit http://gerrit.ovirt.org/12909
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If98a5853385ad484dfa6e5392797f96daeaaa381
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
