Moti Asayag has posted comments on this change.
Change subject: engine: Configure management network at host activation
......................................................................
Patch Set 10: (10 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java
Line 71: runVdsCommand(VDSCommandType.SetVdsStatus,
Line 72: new SetVdsStatusVDSCommandParameters(getVdsId(),
VDSStatus.Unassigned));
Line 73:
Line 74: VDSReturnValue returnValue =
Line 75:
getBackend().getResourceManager().RunVdsCommand(VDSCommandType.ActivateVds,
Done
Line 76: new
ActivateVdsVDSCommandParameters(getVdsId()));
Line 77: setSucceeded(returnValue.getSucceeded());
Line 78:
Line 79: if (getSucceeded()) {
Line 99: }
Line 100: }
Line 101:
Line 102: private void createManagementNetworkIfRequired(VDS host) {
Line 103: if (host != null) {
You may have a look at ActivateVdsVDSCommand and VdsManager.activate()
seems that a host with null value is legit...
Line 104: Network managementNetwork =
Line 105:
getNetworkDAO().getByNameAndDataCenter(Config.<String>
GetValue(ConfigValues.ManagementNetwork),
Line 106: host.getStoragePoolId());
Line 107:
Line 100: }
Line 101:
Line 102: private void createManagementNetworkIfRequired(VDS host) {
Line 103: if (host != null) {
Line 104: Network managementNetwork =
it saves a call to Config.<String> GetValue(ConfigValues.ManagementNetwork).
i moved this to the findXXX method. See it on next patch.
Line 105:
getNetworkDAO().getByNameAndDataCenter(Config.<String>
GetValue(ConfigValues.ManagementNetwork),
Line 106: host.getStoragePoolId());
Line 107:
Line 108: VdsNetworkInterface nic =
findNicForManagementNetwork(host, managementNetwork);
Line 108: VdsNetworkInterface nic =
findNicForManagementNetwork(host, managementNetwork);
Line 109: if (nic != null) {
Line 110: List<VdsNetworkInterface> interfaces =
filterBondsWithoutSlaves(host.getInterfaces());
Line 111: if (interfaces.contains(nic)) {
Line 112: nic.setNetworkName(managementNetwork.getName());
No, such case is not supported and requires an intervation by the host admin.
At worse, if such demand will arrive from the community, we could add a support
for it when time arrives.
Line 113: SetupNetworksParameters parameters = new
SetupNetworksParameters();
Line 114: parameters.setVdsId(host.getId());
Line 115: parameters.setInterfaces(interfaces);
Line 116: parameters.setCheckConnectivity(true);
Line 123: }
Line 124: }
Line 125:
Line 126: private VdsNetworkInterface findNicForManagementNetwork(final VDS
host, Network network) {
Line 127: if (StringUtils.isEmpty(host.getActiveNic()) ||
network.getName().equals(host.getActiveNic())) {
I don't see how it refers to the network's implementation on the host.
Would you elaborate on your concern?
Line 128: return null;
Line 129: }
Line 130:
Line 131: Map<String, VdsNetworkInterface> nameToIface =
Entities.entitiesByName(host.getInterfaces());
Line 128: return null;
Line 129: }
Line 130:
Line 131: Map<String, VdsNetworkInterface> nameToIface =
Entities.entitiesByName(host.getInterfaces());
Line 132: VdsNetworkInterface nic =
nameToIface.get(host.getActiveNic());
No, see http://www.ovirt.org/Features/Normalized_ovirtmgmt_Initialization#Engine
"Activation fails also if it is a vlan with a mismatching vlan tag. "
Line 133: if (nic != null && !nicHasValidVlanId(network, nic)) {
Line 134: addCustomValue("VlanId", resolveVlanId(nic.getVlanId()));
Line 135: addCustomValue("MgmtVlanId",
resolveVlanId(network.getVlanId()));
Line 136: addCustomValue("InterfaceName", nic.getName());
Line 141: return nic;
Line 142: }
Line 143:
Line 144: private String resolveVlanId(Integer vlanId) {
Line 145: return vlanId == null ? "none" : vlanId.toString();
Like any of the audit log messages....
Line 146: }
Line 147:
Line 148: private boolean nicHasValidVlanId(Network network,
VdsNetworkInterface nic) {
Line 149: int nicVlanId = nic.getVlanId() == null ? 0 : nic.getVlanId();
Line 155: List<VdsNetworkInterface> filteredList = new
ArrayList<VdsNetworkInterface>();
Line 156: Map<String, Integer> bonds = new HashMap<String, Integer>();
Line 157:
Line 158: for (VdsNetworkInterface iface : interfaces) {
Line 159: if (Boolean.TRUE.equals(iface.getBonded())) {
I'm not sure what is less worse...
Line 160: bonds.put(iface.getName(), 0);
Line 161: }
Line 162: }
Line 163:
Line 167: }
Line 168: }
Line 169:
Line 170: for (VdsNetworkInterface iface : interfaces) {
Line 171: if (!bonds.containsKey(iface.getName()) ||
bonds.get(iface.getName()) >= 2) {
Cause such configuration isn't support by the engine and/or VDSM.
Line 172: filteredList.add(iface);
Line 173: }
Line 174: }
Line 175:
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 452: NETWORK_UPDATE_VM_INTERFACE_LINK_UP=Link State is UP.
Line 453: NETWORK_UPDATE_VM_INTERFACE_LINK_DOWN=Link State is DOWN.
Line 454: NETWORK_UPDATE_VM_INTERFACE_FAILED=Failed to update Interface
${InterfaceName} (${InterfaceType}) for VM ${VmName}. (User: ${UserName})
Line 455: NETWORK_HOST_USING_WRONG_CLUSER_VLAN=${VdsName} is having wrong vlan
id: ${VlanIdHost}, expected vlan id: ${VlanIdCluster}
Line 456: NETWORK_HOST_MISSING_CLUSER_VLAN=${VdsName} is missing vlan id:
${VlanIdCluster} that is expected by the cluster
Done
Line 457: INVALID_INTERFACE_FOR_MANAGEMENT_NETWORK_CONFIGURATION=Host
${VdsName} has an invalid interface ${InterfaceName} for the management network
configuration.
Line 458: VLAN_ID_MISMATCH_FOR_MANAGEMENT_NETWORK_CONFIGURATION=Host ${VdsName}
has an interface ${InterfaceName} for the management network configuration with
VLAN-ID (${VlanId}) other than data-center definition (${MgmtVlanId}).
Line 459: SETUP_NETWORK_FAILED_FOR_MANAGEMENT_NETWORK_CONFIGURATION=Failed to
configure management network on host ${VdsName}.
Line 460: PERSIST_NETWORK_FAILED_FOR_MANAGEMENT_NETWORK=Failed to persist
management network configuration on host ${VdsName}.
--
To view, visit http://gerrit.ovirt.org/10919
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia82e594b888b84b3aa079cc9aa76fa0dddc59f1d
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
