weizhouapache commented on code in PR #8371:
URL: https://github.com/apache/cloudstack/pull/8371#discussion_r1490798148
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -2798,28 +2766,39 @@ 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) {
+ // not sure if it would hurt to do it in any case, but
+ if
(Boolean.FALSE.equals(RemoveControlIpOnStop.valueIn(vm.getDataCenterId()))) {
+ final DomainRouterVO domR = _routerDao.findById(vm.getId());
+ removeNics(vm, domR);
+ }
+ }
+
+ private void removeNics(VirtualMachine vm, DomainRouterVO domR) {
+ final List<? extends Nic> routerNics = _nicDao.listByVmId(vm.getId());
+ final DataCenterVO dcVO = _dcDao.findById(vm.getDataCenterId());
+
+ for (final Nic nic : routerNics) {
+ final Network network =
_networkModel.getNetwork(nic.getNetworkId());
+
+ if (network.getTrafficType() == TrafficType.Guest &&
nic.getBroadcastUri() != null &&
nic.getBroadcastUri().getScheme().equals("pvlan")) {
Review Comment:
extract line 2791 to line 2799 to a method ?
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java:
##########
@@ -2329,6 +2329,9 @@ public Pair<Network, NicProfile> doInTransaction(final
TransactionStatus status)
final NicProfile profile = new NicProfile(nic,
network, nic.getBroadcastUri(), nic.getIsolationUri(), null, _networkModel
.isSecurityGroupSupportedInNetwork(network),
_networkModel.getNetworkTag(vmProfile.getHypervisorType(), network));
if (guru.release(profile, vmProfile,
nic.getReservationId())) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("released nic %s
on %s according to %s by guru %s, now updating record.", nic, profile,
vmProfile, guru));
Review Comment:
Maybe the message starts with an uppercase letter?
##########
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:
@DaanHoogland
did you mean, keep the comment ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -2798,28 +2766,39 @@ 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) {
+ // not sure if it would hurt to do it in any case, but
+ if
(Boolean.FALSE.equals(RemoveControlIpOnStop.valueIn(vm.getDataCenterId()))) {
+ final DomainRouterVO domR = _routerDao.findById(vm.getId());
Review Comment:
add a debug message here ?
##########
server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java:
##########
@@ -2798,28 +2766,39 @@ 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) {
+ // not sure if it would hurt to do it in any case, but
Review Comment:
remove comment ?
--
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]