weizhouapache commented on code in PR #8371:
URL: https://github.com/apache/cloudstack/pull/8371#discussion_r1487501809
##########
core/src/main/java/com/cloud/serializer/GsonHelper.java:
##########
@@ -51,6 +51,8 @@ public class GsonHelper {
GsonBuilder loggerBuilder = new GsonBuilder();
loggerBuilder.disableHtmlEscaping();
loggerBuilder.setExclusionStrategies(new
LoggingExclusionStrategy(s_logger));
+ loggerBuilder.serializeSpecialFloatingPointValues();
Review Comment:
cool
that's what I wanted to add as well
have seen some NaN errors in the log
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java:
##########
@@ -2435,7 +2438,7 @@ protected void removeNic(final VirtualMachineProfile vm,
final NicVO nic) {
for (final NetworkElement element : networkElements) {
if (providersToImplement.contains(element.getProvider())) {
if (s_logger.isDebugEnabled()) {
- s_logger.debug("Asking " + element.getName() + " to
release " + nic);
+ s_logger.debug(String.format("Asking %s to release %s,
according to %s", element.getName(), nic, nic.getReservationStrategy()));
Review Comment:
according to (????) %s
##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -166,18 +167,25 @@ public boolean release(NicProfile nic,
VirtualMachineProfile vm, String reservat
assert nic.getTrafficType() == TrafficType.Control;
HypervisorType hType = vm.getHypervisorType();
if ( ( (hType == HypervisorType.VMware) || (hType ==
HypervisorType.Hyperv) )&& isRouterVm(vm)) {
+ // for now place this in the vmware specific part, but it miught
be more generic and move up two or three lines
Review Comment:
remove comment ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -2798,28 +2766,38 @@ public void finalizeStop(final VirtualMachineProfile
profile, final Answer answe
final VirtualMachine vm = profile.getVirtualMachine();
final DomainRouterVO domR = _routerDao.findById(vm.getId());
processStopOrRebootAnswer(domR, answer);
- final List<? extends Nic> routerNics =
_nicDao.listByVmId(profile.getId());
- for (final Nic nic : routerNics) {
- final Network network =
_networkModel.getNetwork(nic.getNetworkId());
- final DataCenterVO dcVO =
_dcDao.findById(network.getDataCenterId());
-
- if (network.getTrafficType() == TrafficType.Guest &&
nic.getBroadcastUri() != null &&
nic.getBroadcastUri().getScheme().equals("pvlan")) {
- final NicProfile nicProfile = new NicProfile(nic, network,
nic.getBroadcastUri(), nic.getIsolationUri(), 0, false, "pvlan-nic");
-
- final NetworkTopology networkTopology =
_networkTopologyContext.retrieveNetworkTopology(dcVO);
- try {
- networkTopology.setupDhcpForPvlan(false, domR,
domR.getHostId(), nicProfile);
- } catch (final ResourceUnavailableException e) {
- s_logger.debug("ERROR in finalizeStop: ", e);
- }
- }
+ if
(Boolean.TRUE.equals(RemoveControlIpOnStop.valueIn(profile.getVirtualMachine().getDataCenterId())))
{
+ removeNics(vm, domR);
}
-
}
}
@Override
public void finalizeExpunge(final VirtualMachine vm) {
+ final DomainRouterVO domR = _routerDao.findById(vm.getId());
+ // not sure if it would hurt to do it in any case, but
+ if
(Boolean.FALSE.equals(RemoveControlIpOnStop.valueIn(vm.getDataCenterId()))) {
+ removeNics(vm, domR);
+ }
+ }
+
+ private void removeNics(VirtualMachine vm, DomainRouterVO domR) {
+ final List<? extends Nic> routerNics = _nicDao.listByVmId(vm.getId());
+ for (final Nic nic : routerNics) {
+ final Network network =
_networkModel.getNetwork(nic.getNetworkId());
Review Comment:
+1 with moving the query for dc out of the loop
```
final DataCenterVO dcVO = _dcDao.findById(network.getDataCenterId());
```
##########
server/src/main/java/com/cloud/network/guru/ControlNetworkGuru.java:
##########
@@ -166,18 +167,25 @@ public boolean release(NicProfile nic,
VirtualMachineProfile vm, String reservat
assert nic.getTrafficType() == TrafficType.Control;
HypervisorType hType = vm.getHypervisorType();
if ( ( (hType == HypervisorType.VMware) || (hType ==
HypervisorType.Hyperv) )&& isRouterVm(vm)) {
+ // for now place this in the vmware specific part, but it miught
be more generic and move up two or three lines
+ if
(!VirtualNetworkApplianceManager.RemoveControlIpOnStop.valueIn(vm.getVirtualMachine().getDataCenterId()))
{
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("not releasing %s from %s
with reservationId %s.", nic, vm, reservationId));
Review Comment:
add the reason in the debug message ?
##########
core/src/main/java/com/cloud/serializer/GsonHelper.java:
##########
@@ -51,6 +51,8 @@ public class GsonHelper {
GsonBuilder loggerBuilder = new GsonBuilder();
loggerBuilder.disableHtmlEscaping();
loggerBuilder.setExclusionStrategies(new
LoggingExclusionStrategy(s_logger));
+ loggerBuilder.serializeSpecialFloatingPointValues();
+ // maybe add loggerBuilder.serializeNulls(); as well?
Review Comment:
add or not ?
--
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]
