Fix findbugs findings in cloud-plugin-network-ovs OvsTunnelManagerImpl.java:456, RC_REF_COMPARISON, Priority: High
Serveral low priority fixes Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/ef2ced75 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/ef2ced75 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/ef2ced75 Branch: refs/heads/ui-restyle Commit: ef2ced7564d2c8a7920458e9db482d3d09ec5ff3 Parents: cc6938e Author: Hugo Trippaers <htrippa...@schubergphilis.com> Authored: Mon Feb 10 13:09:53 2014 +0100 Committer: Hugo Trippaers <htrippa...@schubergphilis.com> Committed: Fri Feb 14 18:37:46 2014 +0100 ---------------------------------------------------------------------- .../com/cloud/network/element/OvsElement.java | 283 +++++++++---------- .../cloud/network/guru/OvsGuestNetworkGuru.java | 4 - .../com/cloud/network/ovs/OvsTunnelManager.java | 4 +- .../cloud/network/ovs/OvsTunnelManagerImpl.java | 148 +++++----- 4 files changed, 213 insertions(+), 226 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef2ced75/plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java b/plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java index ea46f93..03eeedd 100644 --- a/plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java +++ b/plugins/network-elements/ovs/src/com/cloud/network/element/OvsElement.java @@ -75,12 +75,12 @@ import com.cloud.vm.VirtualMachineProfile; import com.cloud.vm.dao.DomainRouterDao; @Local(value = {NetworkElement.class, ConnectivityProvider.class, - SourceNatServiceProvider.class, StaticNatServiceProvider.class, - PortForwardingServiceProvider.class, IpDeployer.class}) + SourceNatServiceProvider.class, StaticNatServiceProvider.class, + PortForwardingServiceProvider.class, IpDeployer.class}) public class OvsElement extends AdapterBase implements NetworkElement, - OvsElementService, ConnectivityProvider, ResourceStateAdapter, - PortForwardingServiceProvider, LoadBalancingServiceProvider, - StaticNatServiceProvider, IpDeployer { +OvsElementService, ConnectivityProvider, ResourceStateAdapter, +PortForwardingServiceProvider, LoadBalancingServiceProvider, +StaticNatServiceProvider, IpDeployer { @Inject OvsTunnelManager _ovsTunnelMgr; @Inject @@ -109,21 +109,21 @@ public class OvsElement extends AdapterBase implements NetworkElement, protected boolean canHandle(Network network, Service service) { s_logger.debug("Checking if OvsElement can handle service " - + service.getName() + " on network " + network.getDisplayText()); + + service.getName() + " on network " + network.getDisplayText()); if (network.getBroadcastDomainType() != BroadcastDomainType.Vswitch) { return false; } if (!_networkModel.isProviderForNetwork(getProvider(), network.getId())) { s_logger.debug("OvsElement is not a provider for network " - + network.getDisplayText()); + + network.getDisplayText()); return false; } if (!_ntwkSrvcDao.canProviderSupportServiceInNetwork(network.getId(), - service, Network.Provider.Ovs)) { + service, Network.Provider.Ovs)) { s_logger.debug("OvsElement can't provide the " + service.getName() - + " service on network " + network.getDisplayText()); + + " service on network " + network.getDisplayText()); return false; } @@ -132,7 +132,7 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean configure(String name, Map<String, Object> params) - throws ConfigurationException { + throws ConfigurationException { super.configure(name, params); _resourceMgr.registerResourceStateAdapter(name, this); return true; @@ -140,12 +140,12 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean implement(Network network, NetworkOffering offering, - DeployDestination dest, ReservationContext context) - throws ConcurrentOperationException, ResourceUnavailableException, - InsufficientCapacityException { + DeployDestination dest, ReservationContext context) + throws ConcurrentOperationException, ResourceUnavailableException, + InsufficientCapacityException { s_logger.debug("entering OvsElement implement function for network " - + network.getDisplayText() + " (state " + network.getState() - + ")"); + + network.getDisplayText() + " (state " + network.getState() + + ")"); if (!canHandle(network, Service.Connectivity)) { return false; @@ -155,10 +155,10 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean prepare(Network network, NicProfile nic, - VirtualMachineProfile vm, - DeployDestination dest, ReservationContext context) - throws ConcurrentOperationException, ResourceUnavailableException, - InsufficientCapacityException { + VirtualMachineProfile vm, + DeployDestination dest, ReservationContext context) + throws ConcurrentOperationException, ResourceUnavailableException, + InsufficientCapacityException { if (!canHandle(network, Service.Connectivity)) { return false; } @@ -171,16 +171,16 @@ public class OvsElement extends AdapterBase implements NetworkElement, return false; } - _ovsTunnelMgr.VmCheckAndCreateTunnel(vm, network, dest); + _ovsTunnelMgr.vmCheckAndCreateTunnel(vm, network, dest); return true; } @Override public boolean release(Network network, NicProfile nic, - VirtualMachineProfile vm, - ReservationContext context) throws ConcurrentOperationException, - ResourceUnavailableException { + VirtualMachineProfile vm, + ReservationContext context) throws ConcurrentOperationException, + ResourceUnavailableException { if (!canHandle(network, Service.Connectivity)) { return false; } @@ -192,14 +192,14 @@ public class OvsElement extends AdapterBase implements NetworkElement, return false; } - _ovsTunnelMgr.CheckAndDestroyTunnel(vm.getVirtualMachine(), network); + _ovsTunnelMgr.checkAndDestroyTunnel(vm.getVirtualMachine(), network); return true; } @Override public boolean shutdown(Network network, ReservationContext context, - boolean cleanup) throws ConcurrentOperationException, - ResourceUnavailableException { + boolean cleanup) throws ConcurrentOperationException, + ResourceUnavailableException { if (!canHandle(network, Service.Connectivity)) { return false; } @@ -208,7 +208,7 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean destroy(Network network, ReservationContext context) - throws ConcurrentOperationException, ResourceUnavailableException { + throws ConcurrentOperationException, ResourceUnavailableException { if (!canHandle(network, Service.Connectivity)) { return false; } @@ -222,8 +222,8 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean shutdownProviderInstances( - PhysicalNetworkServiceProvider provider, ReservationContext context) - throws ConcurrentOperationException, ResourceUnavailableException { + PhysicalNetworkServiceProvider provider, ReservationContext context) + throws ConcurrentOperationException, ResourceUnavailableException { return true; } @@ -275,90 +275,90 @@ public class OvsElement extends AdapterBase implements NetworkElement, method = new LbStickinessMethod(StickinessMethodType.LBCookieBased, "This is loadbalancer cookie based stickiness method."); method.addParam("cookie-name", false, "Cookie name passed in http header by the LB to the client.", false); method.addParam("mode", false, - "Valid values: insert, rewrite, prefix. Default value: insert. In the insert mode cookie will be created" + - " by the LB. In other modes, cookie will be created by the server and LB modifies it.", false); + "Valid values: insert, rewrite, prefix. Default value: insert. In the insert mode cookie will be created" + + " by the LB. In other modes, cookie will be created by the server and LB modifies it.", false); method.addParam( - "nocache", - false, - "This option is recommended in conjunction with the insert mode when there is a cache between the client" + - " and HAProxy, as it ensures that a cacheable response will be tagged non-cacheable if a cookie needs " + - "to be inserted. This is important because if all persistence cookies are added on a cacheable home page" + - " for instance, then all customers will then fetch the page from an outer cache and will all share the " + - "same persistence cookie, leading to one server receiving much more traffic than others. See also the " + - "insert and postonly options. ", - true); + "nocache", + false, + "This option is recommended in conjunction with the insert mode when there is a cache between the client" + + " and HAProxy, as it ensures that a cacheable response will be tagged non-cacheable if a cookie needs " + + "to be inserted. This is important because if all persistence cookies are added on a cacheable home page" + + " for instance, then all customers will then fetch the page from an outer cache and will all share the " + + "same persistence cookie, leading to one server receiving much more traffic than others. See also the " + + "insert and postonly options. ", + true); method.addParam( - "indirect", - false, - "When this option is specified in insert mode, cookies will only be added when the server was not reached" + - " after a direct access, which means that only when a server is elected after applying a load-balancing algorithm," + - " or after a redispatch, then the cookie will be inserted. If the client has all the required information" + - " to connect to the same server next time, no further cookie will be inserted. In all cases, when the " + - "indirect option is used in insert mode, the cookie is always removed from the requests transmitted to " + - "the server. The persistence mechanism then becomes totally transparent from the application point of view.", - true); + "indirect", + false, + "When this option is specified in insert mode, cookies will only be added when the server was not reached" + + " after a direct access, which means that only when a server is elected after applying a load-balancing algorithm," + + " or after a redispatch, then the cookie will be inserted. If the client has all the required information" + + " to connect to the same server next time, no further cookie will be inserted. In all cases, when the " + + "indirect option is used in insert mode, the cookie is always removed from the requests transmitted to " + + "the server. The persistence mechanism then becomes totally transparent from the application point of view.", + true); method.addParam( - "postonly", - false, - "This option ensures that cookie insertion will only be performed on responses to POST requests. It is an" + - " alternative to the nocache option, because POST responses are not cacheable, so this ensures that the " + - "persistence cookie will never get cached.Since most sites do not need any sort of persistence before the" + - " first POST which generally is a login request, this is a very efficient method to optimize caching " + - "without risking to find a persistence cookie in the cache. See also the insert and nocache options.", - true); + "postonly", + false, + "This option ensures that cookie insertion will only be performed on responses to POST requests. It is an" + + " alternative to the nocache option, because POST responses are not cacheable, so this ensures that the " + + "persistence cookie will never get cached.Since most sites do not need any sort of persistence before the" + + " first POST which generally is a login request, this is a very efficient method to optimize caching " + + "without risking to find a persistence cookie in the cache. See also the insert and nocache options.", + true); method.addParam( - "domain", - false, - "This option allows to specify the domain at which a cookie is inserted. It requires exactly one parameter:" + - " a valid domain name. If the domain begins with a dot, the browser is allowed to use it for any host " + - "ending with that name. It is also possible to specify several domain names by invoking this option multiple" + - " times. Some browsers might have small limits on the number of domains, so be careful when doing that. " + - "For the record, sending 10 domains to MSIE 6 or Firefox 2 works as expected.", - false); + "domain", + false, + "This option allows to specify the domain at which a cookie is inserted. It requires exactly one parameter:" + + " a valid domain name. If the domain begins with a dot, the browser is allowed to use it for any host " + + "ending with that name. It is also possible to specify several domain names by invoking this option multiple" + + " times. Some browsers might have small limits on the number of domains, so be careful when doing that. " + + "For the record, sending 10 domains to MSIE 6 or Firefox 2 works as expected.", + false); methodList.add(method); method = new LbStickinessMethod(StickinessMethodType.AppCookieBased, - "This is App session based sticky method. Define session stickiness on an existing application cookie. " + + "This is App session based sticky method. Define session stickiness on an existing application cookie. " + "It can be used only for a specific http traffic"); method.addParam("cookie-name", false, "This is the name of the cookie used by the application and which LB will " + - "have to learn for each new session. Default value: Auto geneared based on ip", false); + "have to learn for each new session. Default value: Auto geneared based on ip", false); method.addParam("length", false, "This is the max number of characters that will be memorized and checked in " + - "each cookie value. Default value:52", false); + "each cookie value. Default value:52", false); method.addParam( - "holdtime", - false, - "This is the time after which the cookie will be removed from memory if unused. The value should be in " + - "the format Example : 20s or 30m or 4h or 5d . only seconds(s), minutes(m) hours(h) and days(d) are valid," + - " cannot use th combinations like 20h30m. Default value:3h ", - false); + "holdtime", + false, + "This is the time after which the cookie will be removed from memory if unused. The value should be in " + + "the format Example : 20s or 30m or 4h or 5d . only seconds(s), minutes(m) hours(h) and days(d) are valid," + + " cannot use th combinations like 20h30m. Default value:3h ", + false); method.addParam( - "request-learn", - false, - "If this option is specified, then haproxy will be able to learn the cookie found in the request in case the server does not specify any in response. This is typically what happens with PHPSESSID cookies, or when haproxy's session expires before the application's session and the correct server is selected. It is recommended to specify this option to improve reliability", - true); + "request-learn", + false, + "If this option is specified, then haproxy will be able to learn the cookie found in the request in case the server does not specify any in response. This is typically what happens with PHPSESSID cookies, or when haproxy's session expires before the application's session and the correct server is selected. It is recommended to specify this option to improve reliability", + true); method.addParam( - "prefix", - false, - "When this option is specified, haproxy will match on the cookie prefix (or URL parameter prefix). " - + - "The appsession value is the data following this prefix. Example : appsession ASPSESSIONID len 64 timeout 3h prefix This will match the cookie ASPSESSIONIDXXXX=XXXXX, the appsession value will be XXXX=XXXXX.", - true); + "prefix", + false, + "When this option is specified, haproxy will match on the cookie prefix (or URL parameter prefix). " + + + "The appsession value is the data following this prefix. Example : appsession ASPSESSIONID len 64 timeout 3h prefix This will match the cookie ASPSESSIONIDXXXX=XXXXX, the appsession value will be XXXX=XXXXX.", + true); method.addParam( - "mode", - false, - "This option allows to change the URL parser mode. 2 modes are currently supported : - path-parameters " + - ": The parser looks for the appsession in the path parameters part (each parameter is separated by a semi-colon), " + - "which is convenient for JSESSIONID for example.This is the default mode if the option is not set. - query-string :" + - " In this mode, the parser will look for the appsession in the query string.", - false); + "mode", + false, + "This option allows to change the URL parser mode. 2 modes are currently supported : - path-parameters " + + ": The parser looks for the appsession in the path parameters part (each parameter is separated by a semi-colon), " + + "which is convenient for JSESSIONID for example.This is the default mode if the option is not set. - query-string :" + + " In this mode, the parser will look for the appsession in the query string.", + false); methodList.add(method); method = new LbStickinessMethod(StickinessMethodType.SourceBased, "This is source based Stickiness method, " + - "it can be used for any type of protocol."); + "it can be used for any type of protocol."); method.addParam("tablesize", false, "Size of table to store source ip addresses. example: tablesize=200k or 300m" + - " or 400g. Default value:200k", false); + " or 400g. Default value:200k", false); method.addParam("expire", false, "Entry in source ip table will expire after expire duration. units can be s,m,h,d ." + - " example: expire=30m 20s 50h 4d. Default value:3h", false); + " example: expire=30m 20s 50h 4d. Default value:3h", false); methodList.add(method); Gson gson = new Gson(); @@ -374,14 +374,14 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public HostVO createHostVOForConnectedAgent(HostVO host, - StartupCommand[] cmd) { + StartupCommand[] cmd) { return null; } @Override public HostVO createHostVOForDirectConnectAgent(HostVO host, - StartupCommand[] startup, ServerResource resource, - Map<String, String> details, List<String> hostTags) { + StartupCommand[] startup, ServerResource resource, + Map<String, String> details, List<String> hostTags) { if (!(startup[0] instanceof StartupOvsCommand)) { return null; } @@ -391,7 +391,7 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, - boolean isForceDeleteStorage) throws UnableDeleteHostException { + boolean isForceDeleteStorage) throws UnableDeleteHostException { if (!(host.getType() == Host.Type.L2Networking)) { return null; } @@ -405,8 +405,8 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean applyIps(Network network, - List<? extends PublicIpAddress> ipAddress, Set<Service> services) - throws ResourceUnavailableException { + List<? extends PublicIpAddress> ipAddress, Set<Service> services) + throws ResourceUnavailableException { boolean canHandle = true; for (Service service : services) { // check if Ovs can handle services except SourceNat & Firewall @@ -417,11 +417,11 @@ public class OvsElement extends AdapterBase implements NetworkElement, } if (canHandle) { List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole( - network.getId(), Role.VIRTUAL_ROUTER); + network.getId(), Role.VIRTUAL_ROUTER); if (routers == null || routers.isEmpty()) { s_logger.debug("Virtual router element doesn't need to associate ip addresses on the backend; virtual " - + "router doesn't exist in the network " - + network.getId()); + + "router doesn't exist in the network " + + network.getId()); return true; } @@ -433,15 +433,15 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean applyStaticNats(Network network, List<? extends StaticNat> rules) - throws ResourceUnavailableException { + throws ResourceUnavailableException { if (!canHandle(network, Service.StaticNat)) { return false; } List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole( - network.getId(), Role.VIRTUAL_ROUTER); + network.getId(), Role.VIRTUAL_ROUTER); if (routers == null || routers.isEmpty()) { s_logger.debug("Ovs element doesn't need to apply static nat on the backend; virtual " - + "router doesn't exist in the network " + network.getId()); + + "router doesn't exist in the network " + network.getId()); return true; } @@ -450,15 +450,15 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean applyPFRules(Network network, List<PortForwardingRule> rules) - throws ResourceUnavailableException { + throws ResourceUnavailableException { if (!canHandle(network, Service.PortForwarding)) { return false; } List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole( - network.getId(), Role.VIRTUAL_ROUTER); + network.getId(), Role.VIRTUAL_ROUTER); if (routers == null || routers.isEmpty()) { s_logger.debug("Ovs element doesn't need to apply firewall rules on the backend; virtual " - + "router doesn't exist in the network " + network.getId()); + + "router doesn't exist in the network " + network.getId()); return true; } @@ -467,25 +467,25 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public boolean applyLBRules(Network network, List<LoadBalancingRule> rules) - throws ResourceUnavailableException { + throws ResourceUnavailableException { if (canHandle(network, Service.Lb)) { if (!canHandleLbRules(rules)) { return false; } List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole( - network.getId(), Role.VIRTUAL_ROUTER); + network.getId(), Role.VIRTUAL_ROUTER); if (routers == null || routers.isEmpty()) { s_logger.debug("Virtual router elemnt doesn't need to apply firewall rules on the backend; virtual " - + "router doesn't exist in the network " - + network.getId()); + + "router doesn't exist in the network " + + network.getId()); return true; } if (!_routerMgr.applyLoadBalancingRules(network, rules, routers)) { throw new CloudRuntimeException( - "Failed to apply load balancing rules in network " - + network.getId()); + "Failed to apply load balancing rules in network " + + network.getId()); } else { return true; } @@ -500,7 +500,7 @@ public class OvsElement extends AdapterBase implements NetworkElement, rules.add(rule); if (canHandle(network, Service.Lb) && canHandleLbRules(rules)) { List<DomainRouterVO> routers = _routerDao.listByNetworkAndRole( - network.getId(), Role.VIRTUAL_ROUTER); + network.getId(), Role.VIRTUAL_ROUTER); if (routers == null || routers.isEmpty()) { return true; } @@ -511,21 +511,21 @@ public class OvsElement extends AdapterBase implements NetworkElement, @Override public List<LoadBalancerTO> updateHealthChecks(Network network, - List<LoadBalancingRule> lbrules) { + List<LoadBalancingRule> lbrules) { // TODO Auto-generated method stub return null; } private boolean canHandleLbRules(List<LoadBalancingRule> rules) { - Map<Capability, String> lbCaps = this.getCapabilities().get(Service.Lb); + Map<Capability, String> lbCaps = getCapabilities().get(Service.Lb); if (!lbCaps.isEmpty()) { String schemeCaps = lbCaps.get(Capability.LbSchemes); if (schemeCaps != null) { for (LoadBalancingRule rule : rules) { if (!schemeCaps.contains(rule.getScheme().toString())) { s_logger.debug("Scheme " + rules.get(0).getScheme() - + " is not supported by the provider " - + this.getName()); + + " is not supported by the provider " + + getName()); return false; } } @@ -539,13 +539,13 @@ public class OvsElement extends AdapterBase implements NetworkElement, for (LbStickinessPolicy stickinessPolicy : rule.getStickinessPolicies()) { List<Pair<String, String>> paramsList = stickinessPolicy - .getParams(); + .getParams(); if (StickinessMethodType.LBCookieBased.getName().equalsIgnoreCase( - stickinessPolicy.getMethodName())) { + stickinessPolicy.getMethodName())) { } else if (StickinessMethodType.SourceBased.getName() - .equalsIgnoreCase(stickinessPolicy.getMethodName())) { + .equalsIgnoreCase(stickinessPolicy.getMethodName())) { String tablesize = "200k"; // optional String expire = "30m"; // optional @@ -559,38 +559,29 @@ public class OvsElement extends AdapterBase implements NetworkElement, expire = value; } if ((expire != null) - && !containsOnlyNumbers(expire, timeEndChar)) { + && !containsOnlyNumbers(expire, timeEndChar)) { throw new InvalidParameterValueException( - "Failed LB in validation rule id: " + rule.getId() + "Failed LB in validation rule id: " + rule.getId() + " Cause: expire is not in timeformat: " + expire); } if ((tablesize != null) - && !containsOnlyNumbers(tablesize, "kmg")) { + && !containsOnlyNumbers(tablesize, "kmg")) { throw new InvalidParameterValueException( - "Failed LB in validation rule id: " - + rule.getId() - + " Cause: tablesize is not in size format: " - + tablesize); + "Failed LB in validation rule id: " + + rule.getId() + + " Cause: tablesize is not in size format: " + + tablesize); } } else if (StickinessMethodType.AppCookieBased.getName() - .equalsIgnoreCase(stickinessPolicy.getMethodName())) { - /* - * FORMAT : appsession <cookie> len <length> timeout <holdtime> - * [request-learn] [prefix] [mode - * <path-parameters|query-string>] - */ - /* example: appsession JSESSIONID len 52 timeout 3h */ - String cookieName = null; // optional + .equalsIgnoreCase(stickinessPolicy.getMethodName())) { String length = null; // optional String holdTime = null; // optional for (Pair<String, String> paramKV : paramsList) { String key = paramKV.first(); String value = paramKV.second(); - if ("cookie-name".equalsIgnoreCase(key)) - cookieName = value; if ("length".equalsIgnoreCase(key)) length = value; if ("holdtime".equalsIgnoreCase(key)) @@ -599,15 +590,15 @@ public class OvsElement extends AdapterBase implements NetworkElement, if ((length != null) && (!containsOnlyNumbers(length, null))) { throw new InvalidParameterValueException( - "Failed LB in validation rule id: " + rule.getId() + "Failed LB in validation rule id: " + rule.getId() + " Cause: length is not a number: " + length); } if ((holdTime != null) - && (!containsOnlyNumbers(holdTime, timeEndChar) && !containsOnlyNumbers( - holdTime, null))) { + && (!containsOnlyNumbers(holdTime, timeEndChar) && !containsOnlyNumbers( + holdTime, null))) { throw new InvalidParameterValueException( - "Failed LB in validation rule id: " + rule.getId() + "Failed LB in validation rule id: " + rule.getId() + " Cause: holdtime is not in timeformat: " + holdTime); } @@ -630,7 +621,7 @@ public class OvsElement extends AdapterBase implements NetworkElement, boolean matchedEndChar = false; if (str.length() < 2) return false; // atleast one numeric and one char. example: - // 3h + // 3h char strEnd = str.toCharArray()[str.length() - 1]; for (char c : endChar.toCharArray()) { if (strEnd == c) { @@ -643,7 +634,7 @@ public class OvsElement extends AdapterBase implements NetworkElement, return false; } try { - int i = Integer.parseInt(number); + Integer.parseInt(number); } catch (NumberFormatException e) { return false; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef2ced75/plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java b/plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java index 6aacbb2..8fa636d 100644 --- a/plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java +++ b/plugins/network-elements/ovs/src/com/cloud/network/guru/OvsGuestNetworkGuru.java @@ -144,10 +144,6 @@ public class OvsGuestNetworkGuru extends GuestNetworkGuru { if (network.getCidr() != null) { implemented.setCidr(network.getCidr()); } - String name = network.getName(); - if (name == null || name.isEmpty()) { - name = ((NetworkVO)network).getUuid(); - } // do we need to create switch right now? http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef2ced75/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManager.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManager.java b/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManager.java index eaf2d66..118beeb 100644 --- a/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManager.java +++ b/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManager.java @@ -26,8 +26,8 @@ public interface OvsTunnelManager extends Manager { boolean isOvsTunnelEnabled(); - public void VmCheckAndCreateTunnel(VirtualMachineProfile vm, Network nw, DeployDestination dest); + public void vmCheckAndCreateTunnel(VirtualMachineProfile vm, Network nw, DeployDestination dest); - public void CheckAndDestroyTunnel(VirtualMachine vm, Network nw); + public void checkAndDestroyTunnel(VirtualMachine vm, Network nw); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ef2ced75/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java b/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java index aa0f42e..320568b 100644 --- a/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java +++ b/plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java @@ -104,7 +104,7 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage @Override public boolean configure(String name, Map<String, Object> params) - throws ConfigurationException { + throws ConfigurationException { _executorPool = Executors.newScheduledThreadPool(10, new NamedThreadFactory("OVS")); _cleanupExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("OVS-Cleanup")); @@ -113,13 +113,13 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage @DB protected OvsTunnelInterfaceVO createInterfaceRecord(String ip, - String netmask, String mac, long hostId, String label) { + String netmask, String mac, long hostId, String label) { OvsTunnelInterfaceVO ti = null; try { ti = new OvsTunnelInterfaceVO(ip, netmask, mac, hostId, label); // TODO: Is locking really necessary here? OvsTunnelInterfaceVO lock = _tunnelInterfaceDao - .acquireInLockTable(Long.valueOf(1)); + .acquireInLockTable(Long.valueOf(1)); if (lock == null) { s_logger.warn("Cannot lock table ovs_tunnel_account"); return null; @@ -128,7 +128,7 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage _tunnelInterfaceDao.releaseFromLockTable(lock.getId()); } catch (EntityExistsException e) { s_logger.debug("A record for the interface for network " + label - + " on host id " + hostId + " already exists"); + + " on host id " + hostId + " already exists"); } return ti; } @@ -138,13 +138,13 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage if (ans.getResult()) { if (ans.getIp() != null && !("".equals(ans.getIp()))) { OvsTunnelInterfaceVO ti = createInterfaceRecord(ans.getIp(), - ans.getNetmask(), ans.getMac(), hostId, ans.getLabel()); + ans.getNetmask(), ans.getMac(), hostId, ans.getLabel()); return ti.getIp(); } } // Fetch interface failed! s_logger.warn("Unable to fetch the IP address for the GRE tunnel endpoint" - + ans.getDetails()); + + ans.getDetails()); return null; } @@ -169,22 +169,22 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage private void handleCreateTunnelAnswer(Answer[] answers) { OvsCreateTunnelAnswer r = (OvsCreateTunnelAnswer)answers[0]; String s = - String.format("(hostIP:%1$s, remoteIP:%2$s, bridge:%3$s," + "greKey:%4$s, portName:%5$s)", r.getFromIp(), r.getToIp(), r.getBridge(), r.getKey(), - r.getInPortName()); + String.format("(hostIP:%1$s, remoteIP:%2$s, bridge:%3$s," + "greKey:%4$s, portName:%5$s)", r.getFromIp(), r.getToIp(), r.getBridge(), r.getKey(), + r.getInPortName()); Long from = r.getFrom(); Long to = r.getTo(); long networkId = r.getNetworkId(); OvsTunnelNetworkVO tunnel = _tunnelNetworkDao.getByFromToNetwork(from, to, networkId); if (tunnel == null) { throw new CloudRuntimeException( - String.format("Unable find tunnelNetwork record" + - "(from=%1$s,to=%2$s, account=%3$s", - from, to, networkId)); + String.format("Unable find tunnelNetwork record" + + "(from=%1$s,to=%2$s, account=%3$s", + from, to, networkId)); } if (!r.getResult()) { tunnel.setState("FAILED"); s_logger.warn("Create GRE tunnel failed due to " + r.getDetails() - + s); + + s); } else { tunnel.setState("SUCCESS"); tunnel.setPortName(r.getInPortName()); @@ -194,31 +194,31 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage } private String getGreEndpointIP(Host host, Network nw) - throws AgentUnavailableException, OperationTimedoutException { + throws AgentUnavailableException, OperationTimedoutException { String endpointIp = null; // Fetch fefault name for network label from configuration String physNetLabel = _configDao.getValue(Config.OvsTunnelNetworkDefaultLabel.key()); Long physNetId = nw.getPhysicalNetworkId(); PhysicalNetworkTrafficType physNetTT = - _physNetTTDao.findBy(physNetId, TrafficType.Guest); + _physNetTTDao.findBy(physNetId, TrafficType.Guest); HypervisorType hvType = host.getHypervisorType(); String label = null; switch (hvType) { - case XenServer: - label = physNetTT.getXenNetworkLabel(); - if ((label != null) && (!label.equals(""))) { - physNetLabel = label; - } - break; - case KVM: - label = physNetTT.getKvmNetworkLabel(); - if ((label != null) && (!label.equals(""))) { - physNetLabel = label; - } - break; - default: - throw new CloudRuntimeException("Hypervisor " + + case XenServer: + label = physNetTT.getXenNetworkLabel(); + if ((label != null) && (!label.equals(""))) { + physNetLabel = label; + } + break; + case KVM: + label = physNetTT.getKvmNetworkLabel(); + if ((label != null) && (!label.equals(""))) { + physNetLabel = label; + } + break; + default: + throw new CloudRuntimeException("Hypervisor " + hvType.toString() + " unsupported by OVS Tunnel Manager"); } @@ -226,16 +226,16 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage // Try to fetch GRE endpoint IP address for cloud db // If not found, then find it on the hypervisor OvsTunnelInterfaceVO tunnelIface = - _tunnelInterfaceDao.getByHostAndLabel(host.getId(), - physNetLabel); + _tunnelInterfaceDao.getByHostAndLabel(host.getId(), + physNetLabel); if (tunnelIface == null) { //Now find and fetch configuration for physical interface //for network with label on target host Commands fetchIfaceCmds = - new Commands(new OvsFetchInterfaceCommand(physNetLabel)); + new Commands(new OvsFetchInterfaceCommand(physNetLabel)); s_logger.debug("Ask host " + host.getId() + - " to retrieve interface for phy net with label:" + - physNetLabel); + " to retrieve interface for phy net with label:" + + physNetLabel); Answer[] fetchIfaceAnswers = _agentMgr.send(host.getId(), fetchIfaceCmds); //And finally save it for future use endpointIp = handleFetchInterfaceAnswer(fetchIfaceAnswers, host.getId()); @@ -255,26 +255,26 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage // !! not in the case of lswitch/pvlan/(possibly)vswitch // So we now feel quite safe in converting it into a string // by calling the appropriate BroadcastDomainType method - key = Integer.valueOf(keyStr); + key = Integer.parseInt(keyStr); return key; } catch (NumberFormatException e) { s_logger.debug("Well well, how did '" + key - + "' end up in the broadcast URI for the network?"); + + "' end up in the broadcast URI for the network?"); throw new CloudRuntimeException(String.format( - "Invalid GRE key parsed from" - + "network broadcast URI (%s)", network - .getBroadcastUri().toString())); + "Invalid GRE key parsed from" + + "network broadcast URI (%s)", network + .getBroadcastUri().toString())); } } @DB - protected void CheckAndCreateTunnel(VirtualMachine instance, Network nw, DeployDestination dest) { + protected void checkAndCreateTunnel(VirtualMachine instance, Network nw, DeployDestination dest) { s_logger.debug("Creating tunnels with OVS tunnel manager"); if (instance.getType() != VirtualMachine.Type.User - && instance.getType() != VirtualMachine.Type.DomainRouter) { + && instance.getType() != VirtualMachine.Type.DomainRouter) { s_logger.debug("Will not work if you're not" - + "an instance or a virtual router"); + + "an instance or a virtual router"); return; } @@ -282,8 +282,8 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage int key = getGreKey(nw); // Find active VMs with a NIC on the target network List<UserVmVO> vms = _userVmDao.listByNetworkIdAndStates(nw.getId(), - State.Running, State.Starting, State.Stopping, State.Unknown, - State.Migrating); + State.Running, State.Starting, State.Stopping, State.Unknown, + State.Migrating); // Find routers for the network List<DomainRouterVO> routers = _routerDao.findByNetwork(nw.getId()); List<VMInstanceVO> ins = new ArrayList<VMInstanceVO>(); @@ -313,14 +313,14 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage } ta = _tunnelNetworkDao.getByFromToNetwork(rh.longValue(), - hostId, nw.getId()); + hostId, nw.getId()); // Try and create the tunnel even if a previous attempt failed if (ta == null || ta.getState().equals("FAILED")) { s_logger.debug("Attempting to create tunnel from:" + - rh.longValue() + " to:" + hostId); + rh.longValue() + " to:" + hostId); if (ta == null) { createTunnelRecord(rh.longValue(), hostId, - nw.getId(), key); + nw.getId(), key); } if (!fromHostIds.contains(rh)) { fromHostIds.add(rh); @@ -338,14 +338,14 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage String otherIp = getGreEndpointIP(rHost, nw); if (otherIp == null) throw new GreTunnelException( - "Unable to retrieve the remote " - + "endpoint for the GRE tunnel." - + "Failure is on host:" + rHost.getId()); + "Unable to retrieve the remote " + + "endpoint for the GRE tunnel." + + "Failure is on host:" + rHost.getId()); Commands cmds = new Commands( - new OvsCreateTunnelCommand(otherIp, key, - Long.valueOf(hostId), i, nw.getId(), myIp)); + new OvsCreateTunnelCommand(otherIp, key, + Long.valueOf(hostId), i, nw.getId(), myIp)); s_logger.debug("Ask host " + hostId - + " to create gre tunnel to " + i); + + " to create gre tunnel to " + i); Answer[] answers = _agentMgr.send(hostId, cmds); handleCreateTunnelAnswer(answers); noHost = false; @@ -355,9 +355,9 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage HostVO rHost = _hostDao.findById(i); String otherIp = getGreEndpointIP(rHost, nw); Commands cmds = new Commands(new OvsCreateTunnelCommand(myIp, - key, i, Long.valueOf(hostId), nw.getId(), otherIp)); + key, i, Long.valueOf(hostId), nw.getId(), otherIp)); s_logger.debug("Ask host " + i + " to create gre tunnel to " - + hostId); + + hostId); Answer[] answers = _agentMgr.send(i, cmds); handleCreateTunnelAnswer(answers); noHost = false; @@ -367,13 +367,13 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage // This will ensure VIF rules will be triggered if (noHost) { Commands cmds = new Commands(new OvsSetupBridgeCommand(key, - hostId, nw.getId())); + hostId, nw.getId())); s_logger.debug("Ask host " + hostId - + " to configure bridge for network:" + nw.getId()); + + " to configure bridge for network:" + nw.getId()); Answer[] answers = _agentMgr.send(hostId, cmds); handleSetupBridgeAnswer(answers); } - } catch (Exception e) { + } catch (GreTunnelException | OperationTimedoutException | AgentUnavailableException e) { // I really thing we should do a better handling of these exceptions s_logger.warn("Ovs Tunnel network created tunnel failed", e); } @@ -385,9 +385,9 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage } @Override - public void VmCheckAndCreateTunnel(VirtualMachineProfile vm, - Network nw, DeployDestination dest) { - CheckAndCreateTunnel(vm.getVirtualMachine(), nw, dest); + public void vmCheckAndCreateTunnel(VirtualMachineProfile vm, + Network nw, DeployDestination dest) { + checkAndCreateTunnel(vm.getVirtualMachine(), nw, dest); } @DB @@ -396,9 +396,9 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage OvsTunnelNetworkVO lock = _tunnelNetworkDao.acquireInLockTable(Long.valueOf(1)); if (lock == null) { s_logger.warn(String.format("failed to lock" + - "ovs_tunnel_account, remove record of " + - "tunnel(from=%1$s, to=%2$s account=%3$s) failed", - from, to, networkId)); + "ovs_tunnel_account, remove record of " + + "tunnel(from=%1$s, to=%2$s account=%3$s) failed", + from, to, networkId)); return; } @@ -406,8 +406,8 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage _tunnelNetworkDao.releaseFromLockTable(lock.getId()); s_logger.debug(String.format("Destroy tunnel(account:%1$s," + - "from:%2$s, to:%3$s) successful", - networkId, from, to)); + "from:%2$s, to:%3$s) successful", + networkId, from, to)); } else { s_logger.debug(String.format("Destroy tunnel(account:%1$s," + "from:%2$s, to:%3$s) failed", networkId, from, to)); } @@ -427,10 +427,10 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage _tunnelNetworkDao.releaseFromLockTable(lock.getId()); s_logger.debug(String.format("Destroy bridge for" + - "network %1$s successful", networkId)); + "network %1$s successful", networkId)); } else { s_logger.debug(String.format("Destroy bridge for" + - "network %1$s failed", networkId)); + "network %1$s failed", networkId)); } } @@ -440,7 +440,7 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage } @Override - public void CheckAndDestroyTunnel(VirtualMachine vm, Network nw) { + public void checkAndDestroyTunnel(VirtualMachine vm, Network nw) { // if (!_isEnabled) { // return; // } @@ -453,7 +453,7 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage List<DomainRouterVO> routers = _routerDao.findByNetwork(nw.getId()); for (DomainRouterVO router : routers) { - if (router.getHostId() == vm.getHostId()) { + if (router.getHostId().equals(vm.getHostId())) { return; } } @@ -471,18 +471,18 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage /* Then ask hosts have peer tunnel with me to destroy them */ List<OvsTunnelNetworkVO> peers = - _tunnelNetworkDao.listByToNetwork(vm.getHostId(), - nw.getId()); + _tunnelNetworkDao.listByToNetwork(vm.getHostId(), + nw.getId()); for (OvsTunnelNetworkVO p : peers) { // If the tunnel was not successfully created don't bother to remove it if (p.getState().equals("SUCCESS")) { cmd = new OvsDestroyTunnelCommand(p.getNetworkId(), key, - p.getPortName()); + p.getPortName()); s_logger.debug("Destroying tunnel to " + vm.getHostId() + - " from " + p.getFrom()); + " from " + p.getFrom()); ans = _agentMgr.send(p.getFrom(), cmd); handleDestroyTunnelAnswer(ans, p.getFrom(), - p.getTo(), p.getNetworkId()); + p.getTo(), p.getNetworkId()); } } } catch (Exception e) {
