GutoVeronezi commented on a change in pull request #5810:
URL: https://github.com/apache/cloudstack/pull/5810#discussion_r779735095
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -1252,8 +1252,8 @@ public Network createGuestNetwork(CreateNetworkCmd cmd)
throws InsufficientCapac
}
}
- boolean ipv4 = true, ipv6 = false;
- if (startIP != null) {
+ boolean ipv4 = false, ipv6 = false;
+ if (org.apache.commons.lang3.StringUtils.isNoneBlank(gateway,
netmask)) {
Review comment:
@weizhouapache, recently we adopted
`org.apache.commons.lang3.StringUtils` as the default string lib. A checkstyle
validation was added to prohibit imports of
`org.apache.commons.lang.StringUtils` and `com.google.common.base.Strings`,
however it does not cover `import static`. The files that used `import static`
were modified to remove the `import static` and use the object `StringUtils`
from `lang3`, however, this file went unnoticed.
Could you remove those `import static` (`import static
org.apache.commons.lang.StringUtils.isBlank;` and `import static
org.apache.commons.lang.StringUtils.isNotBlank;`) from the begin of the file,
import the `org.apache.commons.lang3.StringUtils` and use it instead?
I observed that `com.cloud.utils.StringUtils` is already imported and would
conflict with `org.apache.commons.lang3.StringUtils`. However,
`com.cloud.utils.StringUtils` is used only twice and
`org.apache.commons.lang3.StringUtils` will be used several times, therefore,
I'd recommend to import `org.apache.commons.lang3.StringUtils` and to use the
full class name of `com.cloud.utils.StringUtils`.
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -1338,6 +1334,10 @@ public Network createGuestNetwork(CreateNetworkCmd cmd)
throws InsufficientCapac
if(isBlank(zone.getIp6Dns1()) && isBlank(zone.getIp6Dns2())) {
throw new InvalidParameterValueException("Can only create IPv6
network if the zone has IPv6 DNS! Please configure the zone IPv6 DNS1 and/or
IPv6 DNS2.");
}
+
+ if (!ipv4 && ntwkOff.getGuestType() == GuestType.Shared &&
_networkModel.isProviderForNetworkOffering(Provider.VirtualRouter,
networkOfferingId)) {
+ throw new InvalidParameterValueException("Currently IPv6-only
Shared network with Virtual Router provider is not supported");
Review comment:
```suggestion
throw new InvalidParameterValueException("Currently
IPv6-only Shared network with Virtual Router provider is not supported.");
```
##########
File path:
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -2642,7 +2642,7 @@ private Network createGuestNetwork(final long
networkOfferingId, final String na
&&
!_networkModel.areServicesSupportedByNetworkOffering(ntwkOff.getId(),
Service.SourceNat)));
if (cidr == null && ip6Cidr == null && cidrRequired) {
if (ntwkOff.getGuestType() == GuestType.Shared) {
- throw new
InvalidParameterValueException("StartIp/endIp/gateway/netmask are required when
create network of" + " type " + Network.GuestType.Shared);
+ throw new InvalidParameterValueException("gateway/netmask are
required when create network of" + " type " + Network.GuestType.Shared);
Review comment:
```suggestion
throw new
InvalidParameterValueException(String.format("Gateway/netmask are required when
creating %s networks.", Network.GuestType.Shared));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
