GabrielBrascher commented on a change in pull request #3037: kvm: when untagged
vxlan is used, use the default guest/public bridge
URL: https://github.com/apache/cloudstack/pull/3037#discussion_r234685756
##########
File path:
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
##########
@@ -233,8 +233,8 @@ private static boolean isInterface(final String fname) {
}
if (nic.getType() == Networks.TrafficType.Guest) {
- if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan)
&& (vNetId != null) && (protocol != null) &&
(!vNetId.equalsIgnoreCase("untagged")) ||
- (nic.getBroadcastType() ==
Networks.BroadcastDomainType.Vxlan)) {
+ if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan
|| nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan)
+ && (vNetId != null) && (protocol != null) &&
(!vNetId.equalsIgnoreCase("untagged"))) {
Review comment:
@rhtyd what do you think of extracting lines 236 - 254 to a method?
example: `configureGuestBridge(NicTO nic, String guestOsType, String
nicAdapter, LibvirtVMDef.InterfaceDef intf, String vNetId, String protocol,
String trafficLabel, Integer networkRateKBps)`
Additionally, this class has some duplicated conditionals. As you are
touching those conditionals, this might be a good opportunity to extract these
conditionals to methods.
Example:
```
private isBroadcastDomainTypeVlanOrVxlan(NicTO nic) {
return nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan ||
nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan
}
```
```
private isValidProtocolAndVnetId(String vNetId, String protocol) {
return (vNetId != null) && (protocol != null) &&
(!vNetId.equalsIgnoreCase("untagged"))
}
```
Note that line 223 is also using `nic.getBroadcastType() ==
Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() ==
Networks.BroadcastDomainType.Vxlan`
The code:
`if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan ||
nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan)&& (vNetId !=
null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged")))`
would then be transformed to:
`if (isBroadcastDomainTypeVlanOrVclan(nic) &&
isValidProtocolAndVnetId(vNetId, protocol))`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
