server24 commented on issue #3497: VMs stuck in Stopping state due to NPE at org.apache.cloudstack.network.topology.BasicNetworkVisitor.visit(BasicNetworkVisitor.java:201) URL: https://github.com/apache/cloudstack/issues/3497#issuecomment-512719626 Is this PR really fixing the issue? Your original patch was born out of the idea that DHCP entries where left in the VR when it should not. Now with your patch you are basically avoiding the command being executed, but that means it would fall back to the old behaviour before your patch in Basic Networking. Wouldn't it make sense to change it to something like this: @Override public boolean visit(final DhcpEntryRules dhcp) throws ResourceUnavailableException { final VirtualRouter router = dhcp.getRouter(); final Commands commands = new Commands(Command.OnError.Stop); final NicVO nicVo = dhcp.getNicVo(); final UserVmVO userVM = dhcp.getUserVM(); final DeployDestination destination = dhcp.getDestination(); final boolean remove = dhcp.isRemove(); // If we are just removing something, no destination is necessary; we always need the router: if (router != null && (remove || (destination != null && destination.getPod() != null && router.getPodIdToDeployIn() != null && router.getPodIdToDeployIn().longValue() == destination.getPod().getId()))) { _commandSetupHelper.createDhcpEntryCommand(router, userVM, nicVo, remove, commands); return _networkGeneralHelper.sendCommandsToRouter(router, commands); } return true; } So that if remove is TRUE it does not check for destination at all?
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
