Skip to content

Commit

Permalink
core: report SwitchType also on bonds
Browse files Browse the repository at this point in the history 
• all networks get same switch type, so it need not be specified per
HostNetwork. It should be sufficient to have it specified only once in
paramaters class.

• onces that's done, we need not to update CreateOrUpdateBond class
with SwitchType just as HostNetwork used to contain it
(which would be totally incorrect(*)), and use SwitchType from
parameters class.

(*) it's problematic to share TO to pass data between more than 2 app
layers. It can work, until all its data makes sense everywhere, but
here we need only to pass switch type of CreateOrUpdateBond from bll
to vdsm. It's totally meaningless for UI and thus it must not be added
into this class.

Change-Id: Id5a8d2ebc70eb50a73d9c379beb0ea18e5a50ef1
Signed-off-by: Martin Mucha <mmucha@redhat.com>
  • Loading branch information
@tnisan
Martin Mucha authored and tnisan committed Nov 2, 2016
1 parent ffedfee commit 085636c
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 27 deletions.
7 changes: 3 additions & 4 deletions ...es/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksCommand.java
View file
Expand Up @@ -446,12 +446,14 @@ private FutureVDSCall<VDSReturnValue> invokeSetupNetworksCommand(int timeout) {
}

private HostSetupNetworksVdsCommandParameters createSetupNetworksParameters(int timeout) {
SwitchType clusterSwitchType = getCluster().getRequiredSwitchTypeForCluster();
final HostSetupNetworksVdsCommandParameters hostCmdParams = new HostSetupNetworksVdsCommandParameters(
getVds(),
getNetworksToConfigure(),
getAllNetworksToRemove(),
getParameters().getCreateOrUpdateBonds(),
getRemovedBondNames());
getRemovedBondNames(),
clusterSwitchType);
hostCmdParams.setRollbackOnFailure(getParameters().rollbackOnFailure());
hostCmdParams.setConnectivityTimeout(timeout);
hostCmdParams.setManagementNetworkChanged(isManagementNetworkChanged());
Expand Down Expand Up @@ -593,9 +595,6 @@ private List<HostNetwork> getNetworksToConfigure() {
networkToConfigure.setQos(hostNetworkQos);
}

SwitchType clusterSwitchType = getCluster().getRequiredSwitchTypeForCluster();
networkToConfigure.setSwitchType(clusterSwitchType);

networksToConfigure.add(networkToConfigure);
}
}
Expand Down
11 changes: 0 additions & 11 deletions ...er/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/HostNetwork.java
View file
Expand Up @@ -8,7 +8,6 @@
import org.ovirt.engine.core.common.businessentities.network.Ipv6BootProtocol;
import org.ovirt.engine.core.common.businessentities.network.Network;
import org.ovirt.engine.core.common.businessentities.network.NetworkAttachment;
import org.ovirt.engine.core.common.network.SwitchType;
import org.ovirt.engine.core.common.utils.ToStringBuilder;

public class HostNetwork {
Expand All @@ -18,7 +17,6 @@ public class HostNetwork {
private boolean bonding;
private boolean qosConfiguredOnInterface;
private HostNetworkQos qos;
private SwitchType switchType;

@SuppressWarnings("unused")
private HostNetwork() {
Expand Down Expand Up @@ -177,14 +175,6 @@ public HostNetworkQos getQos() {
return qos;
}

public SwitchType getSwitchType() {
return switchType;
}

public void setSwitchType(SwitchType switchType) {
this.switchType = switchType;
}

@Override
public String toString() {
return ToStringBuilder.forInstance(this)
Expand All @@ -205,7 +195,6 @@ public String toString() {
.append("ipv6Address", getIpv6Address())
.append("ipv6Prefix", getIpv6Prefix())
.append("ipv6Gateway", getIpv6Gateway())
.append("switchType", getSwitchType())
.build();
}
}
13 changes: 11 additions & 2 deletions .../java/org/ovirt/engine/core/common/vdscommands/HostSetupNetworksVdsCommandParameters.java
View file
Expand Up @@ -8,9 +8,11 @@
import org.ovirt.engine.core.common.action.CreateOrUpdateBond;
import org.ovirt.engine.core.common.businessentities.Entities;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.network.SwitchType;
import org.ovirt.engine.core.common.utils.ToStringBuilder;

public class HostSetupNetworksVdsCommandParameters extends VdsIdAndVdsVDSCommandParametersBase {
private SwitchType clusterSwitchType;
private List<HostNetwork> networks;
private Set<String> removedNetworks;
private List<CreateOrUpdateBond> createOrUpdateBond;
Expand All @@ -23,13 +25,15 @@ public HostSetupNetworksVdsCommandParameters(VDS host,
List<HostNetwork> networks,
Set<String> removedNetworks,
List<CreateOrUpdateBond> createOrUpdateBond,
Set<String> removedBonds) {
Set<String> removedBonds,
SwitchType clusterSwitchType) {
super(host);
this.networks = (networks == null) ? new ArrayList<HostNetwork>() : networks;
this.removedNetworks = (removedNetworks == null) ? new HashSet<String>() : removedNetworks;
this.createOrUpdateBond = (createOrUpdateBond
== null) ? new ArrayList<CreateOrUpdateBond>() : createOrUpdateBond;
this.removedBonds = (removedBonds == null) ? new HashSet<String>() : removedBonds;
this.clusterSwitchType = clusterSwitchType;
}

public HostSetupNetworksVdsCommandParameters() {
Expand Down Expand Up @@ -83,6 +87,10 @@ public void setConnectivityTimeout(int connectivityTimeout) {
this.connectivityTimeout = connectivityTimeout;
}

public SwitchType getClusterSwitchType() {
return clusterSwitchType;
}

@Override
protected ToStringBuilder appendAttributes(ToStringBuilder tsb) {
return super.appendAttributes(tsb)
Expand All @@ -91,7 +99,8 @@ protected ToStringBuilder appendAttributes(ToStringBuilder tsb) {
.append("networks", Entities.collectionToString(getNetworks(), "\t\t"))
.append("removedNetworks", getRemovedNetworks())
.append("bonds", Entities.collectionToString(getCreateOrUpdateBonds(), "\t\t"))
.append("removedBonds", getRemovedBonds());
.append("removedBonds", getRemovedBonds())
.append("clusterSwitchType", getClusterSwitchType());
}

public boolean isManagementNetworkChanged() {
Expand Down
31 changes: 21 additions & 10 deletions .../src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostSetupNetworksVDSCommand.java
View file
Expand Up @@ -9,6 +9,7 @@
import org.ovirt.engine.core.common.action.CreateOrUpdateBond;
import org.ovirt.engine.core.common.businessentities.network.Ipv4BootProtocol;
import org.ovirt.engine.core.common.businessentities.network.Ipv6BootProtocol;
import org.ovirt.engine.core.common.network.SwitchType;
import org.ovirt.engine.core.common.validation.MaskValidator;
import org.ovirt.engine.core.common.vdscommands.HostNetwork;
import org.ovirt.engine.core.common.vdscommands.HostSetupNetworksVdsCommandParameters;
Expand Down Expand Up @@ -72,12 +73,7 @@ private Map<String, Object> generateNetworks() {
attributes.put(DEFAULT_ROUTE, Boolean.TRUE);
}

if (hostNetwork.getSwitchType() != null) {
/**
* optional, specifies switch type. Legacy will be used if switch option is not passed.
*/
attributes.put(VdsProperties.SWITCH_KEY, hostNetwork.getSwitchType().getOptionValue());
}
addSwitchTypeIfSpecified(attributes);

if (hostNetwork.hasProperties()) {
attributes.put(VdsProperties.NETWORK_CUSTOM_PROPERTIES, hostNetwork.getProperties());
Expand Down Expand Up @@ -136,10 +132,7 @@ private Map<String, Object> generateBonds() {
Map<String, Object> bonds = new HashMap<>();

for (CreateOrUpdateBond bond : getParameters().getCreateOrUpdateBonds()) {
Map<String, Object> attributes = new HashMap<>();
attributes.put(SLAVES, new ArrayList<>(bond.getSlaves()));
putIfNotEmpty(attributes, BONDING_OPTIONS, bond.getBondOptions());
bonds.put(bond.getName(), attributes);
bonds.put(bond.getName(), createBondAttributes(bond));
}

for (String bond : getParameters().getRemovedBonds()) {
Expand All @@ -149,6 +142,14 @@ private Map<String, Object> generateBonds() {
return bonds;
}

private Map<String, Object> createBondAttributes(CreateOrUpdateBond bond) {
Map<String, Object> attributes = new HashMap<>();
attributes.put(SLAVES, new ArrayList<>(bond.getSlaves()));
addSwitchTypeIfSpecified(attributes);
putIfNotEmpty(attributes, BONDING_OPTIONS, bond.getBondOptions());
return attributes;
}

private Map<String, Object> generateOptions() {
Map<String, Object> options = new HashMap<>();
options.put(VdsProperties.CONNECTIVITY_CHECK, Boolean.toString(getParameters().isRollbackOnFailure()));
Expand All @@ -166,4 +167,14 @@ private static void putIfNotEmpty(Map<String, Object> map, String key, String va
map.put(key, value);
}
}

/**
* switch type is optional, specifies switch type. Legacy will be used if switch option is not set/passed.
*/
private void addSwitchTypeIfSpecified(Map<String, Object> resultMap) {
SwitchType switchType = getParameters().getClusterSwitchType();
if (switchType != null) {
resultMap.put(VdsProperties.SWITCH_KEY, switchType.getOptionValue());
}
}
}

0 comments on commit 085636c

Please sign in to comment.