You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/07/27 19:25:15 UTC

[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #5195: Persistent Networks Resources created on host during time of connecti…

GutoVeronezi commented on a change in pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#discussion_r677734833



##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -59,12 +70,66 @@
     StoragePoolDetailsDao storagePoolDetailsDao;
     @Inject
     StorageManager storageManager;
+    @Inject
+    NetworkOfferingDao networkOfferingDao;
+    @Inject
+    HostDao hostDao;
+    @Inject
+    NetworkModel networkModel;
+    @Inject
+    ConfigurationManager configManager;
+    @Inject
+    NetworkDao networkDao;
+
 
     @Override
     public boolean hostAdded(long hostId) {
         return true;
     }
 
+    private boolean createPersistentNetworkResourcesOnHost(long hostId) {
+        HostVO host = hostDao.findById(hostId);
+        if (host != null) {

Review comment:
       We could invert this if to reduce indentation and maybe add a log.

##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -59,12 +70,66 @@
     StoragePoolDetailsDao storagePoolDetailsDao;
     @Inject
     StorageManager storageManager;
+    @Inject
+    NetworkOfferingDao networkOfferingDao;
+    @Inject
+    HostDao hostDao;
+    @Inject
+    NetworkModel networkModel;
+    @Inject
+    ConfigurationManager configManager;
+    @Inject
+    NetworkDao networkDao;
+
 
     @Override
     public boolean hostAdded(long hostId) {
         return true;
     }
 
+    private boolean createPersistentNetworkResourcesOnHost(long hostId) {
+        HostVO host = hostDao.findById(hostId);
+        if (host != null) {
+            List<NetworkVO> allPersistentNetworks = networkDao.getAllPersistentNetworksFromZone(host.getDataCenterId());
+
+            for (NetworkVO networkVO : allPersistentNetworks) {
+                NetworkOfferingVO networkOfferingVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId());
+
+                SetupPersistentNetworkCommand persistentNetworkCommand =
+                        new SetupPersistentNetworkCommand(createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, host));
+                Answer answer = agentMgr.easySend(hostId, persistentNetworkCommand);
+                if (answer == null) {
+                    throw new CloudRuntimeException("Unable to get answer to the setup persistent network command " + networkVO.getId());
+                }
+                if (!answer.getResult()) {
+                    String msg = "Unable to create L2 persistent network resources from network " + networkVO.getId() + " on the host" + hostId;

Review comment:
       We could improve this message with more context than only id. Also we could use String.format here. 

##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -137,7 +203,26 @@ public boolean hostDisconnected(long hostId, long poolId) {
 
     @Override
     public boolean hostAboutToBeRemoved(long hostId) {
-        return true;
+        // send host the cleanup persistent network resources
+        HostVO host = hostDao.findById(hostId);
+        if (host != null) {

Review comment:
       We could invert this if to reduce indentation and maybe add a log.




-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org