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 2020/08/24 14:08:37 UTC

[GitHub] [cloudstack] RodrigoDLopez opened a new pull request #4282: Prevents null pointers when doing consecutive VM migrates

RodrigoDLopez opened a new pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282


   ## Description
   <!--- Describe your changes in detail -->
   If a VM gets consecutive migrations, a null pointer exception is thrown because these VMs do not have `host_id` or` last_host_id`; ACS clears these fields when the first migration is over.
   With that in mind, this PR prevents the respective null pointer. Additionally, it logs the right context and gives some information to the operator.
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [x] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ## Screenshots (if appropriate):
   
   ## How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   To test this, I used the `cloudmonkey` to request `migrateVirtualMachine` commands
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-679187407


   @RodrigoDLopez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez closed pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez closed pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282


   


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-694923320


   @DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-858712102


   @RodrigoDLopez can you address the conflict?
   @rhtyd @nvazquez are your comments addressed to satisfaction?


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] ravening commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-697264089


   @RodrigoDLopez I tried migrating same vm like 10 times between 5 hypervisors and never got any NPE. Also the `lost_host_id` field was not cleared.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-866863970


   not needed anymore...


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-689870590


   @blueorangutan package


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-863428399


   Thanks for testing @GabrielBrascher, @RodrigoDLopez can you test 4.15.1.0 RC2? 


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-679224243


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-1813


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-687655569


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-1905


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-689891353


   Packaging result: ✔centos7 ✖centos8 ✖debian. JID-1945


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-856968316


   @GabrielBrascher I've not tested yet and will try it tomorrow but I feel there has been a change in code that would prevent NPE that you shared,
   4.15.0.0 (failing to get clusterId for the source host as srcHost is null): https://github.com/apache/cloudstack/blob/4.15.0.0/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L2302
   4.15 branch (logic for finding source clusterId has been changed. It is now found from `host_id` or `last_host_id` or storage pool of the VMs volumes): https://github.com/apache/cloudstack/blob/4.15/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L2318


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-697931012


   Hi, @ravening can you report the methodology you used?
   * ACS version
   * How are you executing the tests? Are you migrating VMs running? Stopped VMs? Stopped VMs, and then starting them, and just then migrating again?
   
   If you take a look at the code, a null pointer will for sure happen. Just look at the lines I am changing.
   
   Before this PR, the first migration will have `host_id` or `last_host_id`, but during the migrate, ACS will set the field `last_host_id` to null using the `afterStorageMigrationCleanup` method, and on the next executions of the migrate command (without starting the VM), this field will be null and then on this line, a NPE will be launched
   https://github.com/apache/cloudstack/blob/d657ef7d5b4594b638eaf7df312b11efb99000fb/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L2282-L2283
   
   Furthermore, some of these parameters were never used for some hypervisors, seems like VMWare needs it, that is why I moved it where they will be used, and did the necessary validations providing information about the process with an appropriate log message


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] ravening commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-762379169


   > @ravening @rhtyd @harikrishna-patnala @nvazquez 
   > 
   > 
   > 
   > Hello everyone, this PR fixes a bug and is a simple change.  
   > 
   > I believe that it is in the interest of the whole community that this bug found to be solved.
   > 
   > would it be possible to receive some reviews or tests in this PR?
   
   Will review and test it this week


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-754020472


   @rhtyd, @ravenin, @DaanHoogland 
   
   Hello guys, happy new year.
   
   Is there something missing here? Everything seems to be ok with the patch. It is a good bug fix, it has no errors, and tests are passing


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez closed pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez closed pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#discussion_r493207385



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2372,18 +2368,32 @@ private void setDestinationPoolAndReallocateNetwork(StoragePool destPool, VMInst
         }
     }
 
-    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId) {
+    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm) {
         // OfflineVmwareMigration: this should only happen on storage migration, else the guru would already have issued the command
-        final Long destClusterId = destPool.getClusterId();
-        if (srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId)) {
-            final String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
-            final String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
-            if (srcDcName != null && destDcName != null && !srcDcName.equals(destDcName)) {
-                removeStaleVmFromSource(vm, srcHost);
-            }
+        Long destClusterId = destPool.getClusterId();
+        Long srcHostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId();
+        if (srcHostId == null) {
+            s_logger.debug(String.format("No Host ID was found when doing cleanup after Vmware migration for VM with ID = [%d] and Cluster destination ID = [%d]", vm.getId(), destClusterId));
+            return;
+        }
+        HostVO srcHost = _hostDao.findById(srcHostId);
+        if (srcHost == null) {
+            s_logger.debug(String.format("When doing cleanup after Vmware migration could not find a host for the given ID = [%d]", srcHostId));
+            return;
+        }
+        Long srcClusterId = srcHost.getClusterId();
+        if (srcClusterId.equals(destClusterId)) {
+            s_logger.debug("Since the Source cluster ID [%s] is equal to the Destination cluster ID [%s] we do not need to proceed with the clean up after migration");
+            return;
+        }
+        String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
+        String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
+        if(StringUtils.isNotBlank(srcDcName) && StringUtils.isNotBlank(destDcName)){

Review comment:
       @RodrigoDLopez shouldn't we be doing a `!srcDcName.equals(destDcName)` check here?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-856707446


   I just reproduced this very issue, this PR would be nice to have (4.15.2+, I think hat for 4.15.1 it is not feasible).
   @RodrigoDLopez can you please fix the conflicts?


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-856936171


   @rhtyd we got it on a 4.15.0.0, but it seems that the codebase at 4.15.1.0 RC1 did not change at that point.
   @shwstppr I think that it can happen with any hypervisor as long as one migrate the VM, I saw this issue with KVM.
   
   Steps to reproduce:
   ~~~
   Case: migrate VM (e.g. for maintenance), keep VM stopped, try to migrate back the VM -> NullPointer
   
   1. Migrate VM from host A to host B
   2. VM stills stopped at host B
   3. Try to migrate VM back to host A
   4. Null pointer
   ~~~
   
   Stack trace:
   ```
   java.lang.NullPointerException
           at com.cloud.vm.VirtualMachineManagerImpl.migrateThroughHypervisorOrStorage(VirtualMachineManagerImpl.java:2302)
           at com.cloud.vm.VirtualMachineManagerImpl.orchestrateStorageMigration(VirtualMachineManagerImpl.java:2194)
           at com.cloud.vm.VirtualMachineManagerImpl.orchestrateStorageMigration(VirtualMachineManagerImpl.java:5625)
           at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
           at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
           at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
           at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   ```
   
   When checking the stack trace + DB it is possible to detect that the issue is indeed caused by the fact of last `last_host_id` being null, as reported by @RodrigoDLopez.


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-687650340


   @blueorangutan package
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-682226405


   <b>Trillian test result (tid-2576)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 55420 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4282-t2576-vmware-67u3.zip
   Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 78 look OK, 3 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_deploy_kubernetes_cluster | `Error` | 0.11 | test_kubernetes_clusters.py
   test_02_deploy_kubernetes_ha_cluster | `Error` | 0.05 | test_kubernetes_clusters.py
   test_04_deploy_and_upgrade_kubernetes_cluster | `Error` | 0.07 | test_kubernetes_clusters.py
   test_05_deploy_and_upgrade_kubernetes_ha_cluster | `Error` | 0.07 | test_kubernetes_clusters.py
   test_06_deploy_and_invalid_upgrade_kubernetes_cluster | `Error` | 0.06 | test_kubernetes_clusters.py
   test_07_deploy_and_scale_kubernetes_cluster | `Error` | 0.04 | test_kubernetes_clusters.py
   test_07_reboot_ssvm | `Failure` | 58.23 | test_ssvm.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 671.18 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 3977.82 | test_vpc_redundant.py
   test_04_rvpc_network_garbage_collector_nics | `Error` | 3977.85 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 0.01 | test_vpc_redundant.py
   ContextSuite context=TestVPCRedundancy>:teardown | `Error` | 0.01 | test_vpc_redundant.py
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on a change in pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#discussion_r559732436



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2372,18 +2368,32 @@ private void setDestinationPoolAndReallocateNetwork(StoragePool destPool, VMInst
         }
     }
 
-    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId) {
+    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm) {
         // OfflineVmwareMigration: this should only happen on storage migration, else the guru would already have issued the command
-        final Long destClusterId = destPool.getClusterId();
-        if (srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId)) {
-            final String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
-            final String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
-            if (srcDcName != null && destDcName != null && !srcDcName.equals(destDcName)) {
-                removeStaleVmFromSource(vm, srcHost);
-            }
+        Long destClusterId = destPool.getClusterId();
+        Long srcHostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId();
+        if (srcHostId == null) {
+            s_logger.debug(String.format("No Host ID was found when doing cleanup after Vmware migration for VM with ID = [%d] and Cluster destination ID = [%d]", vm.getId(), destClusterId));
+            return;
+        }
+        HostVO srcHost = _hostDao.findById(srcHostId);
+        if (srcHost == null) {
+            s_logger.debug(String.format("When doing cleanup after Vmware migration could not find a host for the given ID = [%d]", srcHostId));
+            return;
+        }
+        Long srcClusterId = srcHost.getClusterId();
+        if (srcClusterId.equals(destClusterId)) {

Review comment:
       Suggestion: why not reversing the logic and moving the code block from in lines 2377-2383 to this point?
   
   I'm ok anyways but I think the other way is easier to read. If you are keeping this if block please add null checks for srcClusterId and destClusterId

##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2372,18 +2368,32 @@ private void setDestinationPoolAndReallocateNetwork(StoragePool destPool, VMInst
         }
     }
 
-    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId) {
+    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm) {
         // OfflineVmwareMigration: this should only happen on storage migration, else the guru would already have issued the command
-        final Long destClusterId = destPool.getClusterId();
-        if (srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId)) {
-            final String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
-            final String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
-            if (srcDcName != null && destDcName != null && !srcDcName.equals(destDcName)) {
-                removeStaleVmFromSource(vm, srcHost);
-            }
+        Long destClusterId = destPool.getClusterId();
+        Long srcHostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId();
+        if (srcHostId == null) {
+            s_logger.debug(String.format("No Host ID was found when doing cleanup after Vmware migration for VM with ID = [%d] and Cluster destination ID = [%d]", vm.getId(), destClusterId));
+            return;
+        }
+        HostVO srcHost = _hostDao.findById(srcHostId);
+        if (srcHost == null) {
+            s_logger.debug(String.format("When doing cleanup after Vmware migration could not find a host for the given ID = [%d]", srcHostId));
+            return;
+        }
+        Long srcClusterId = srcHost.getClusterId();
+        if (srcClusterId.equals(destClusterId)) {
+            s_logger.debug("Since the Source cluster ID [%s] is equal to the Destination cluster ID [%s] we do not need to proceed with the clean up after migration");
+            return;
+        }
+        String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
+        String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
+        if(StringUtils.isNotBlank(srcDcName) && StringUtils.isNotBlank(destDcName)){

Review comment:
       Minor checkstyle bug: please add a space between the if and the condition and other space before the curly brace after the condition 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-791900647


   @RodrigoDLopez can you fix the conflict?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-863425025


   @rhtyd @RodrigoDLopez @shwstppr @DaanHoogland I have been running some tests on RC2 and I was not able to reproduce this issue with 4.15.1.0 RC2.
   
   This one looks to be fixed.


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-782315982


   > @RodrigoDLopez do you want this on latest or on one of the release branches (4.14 or 4.15)?
   
   For me, it's fine to be merged just on the master, since it's a nonreported bug. But if you wanna I can do the necessary changes to merge this fix on 4.14 as well
   
   @nvazquez @ravening @rhtyd @harikrishna-patnala 
   any update related to this PR? something that maybe I need to change?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] ravening commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-702394123


   > Hi @ravening, that was my fault.
   > 
   > You are doing a live migration, this NPE will occur when you: 
   > 
   > - stop an instance;
   > 
   > - migrate it with volume
   > 
   > - do not start the instance
   > 
   > - try to migrate again to another pool
   > 
   > 
   > 
   > with these steps, after the first migrate `host_id` and `last_host_id` will be set to `null`. And then, when you try to migrate it again a NPE will be thrown.
   
   @RodrigoDLopez ah ok... So you are just doing volume migration of VM...I will try these steps and let you know the result


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-679186571


   @blueorangutan package


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-681625740


   @blueorangutan test centos7 vmware-67u3


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-762735241


   @RodrigoDLopez do you want this on latest or on one of the release branches (4.14 or 4.15)? 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-689871106


   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-694922941


   @blueorangutan package


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-696020884


   @blueorangutan test matrix


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-681625999


   @shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-694949767


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2045


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-856743078


   @GabrielBrascher were you able to reproduce this with 4.15.1 RC?
   From the code changes, I feel this affects only VMware. Is that correct @RodrigoDLopez ?
   Last time I was not able to reproduce this after #4385. Can check again


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] GabrielBrascher commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-859094238


   @shwstppr thanks for bringing those changes, I might have tested with 4.15.0.0 then instead of RC1. I will take another look at it.


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-696480116


   <b>Trillian test result (tid-2818)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 56851 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4282-t2818-vmware-67u3.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 82 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-696480116


   <b>Trillian test result (tid-2818)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 56851 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4282-t2818-vmware-67u3.zip
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 82 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-762377587


   @ravening @rhtyd @harikrishna-patnala @nvazquez 
   
   Hello everyone, this PR fixes a bug and is a simple change.  
   I believe that it is in the interest of the whole community that this bug found to be solved.
   would it be possible to receive some reviews or tests in this PR?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] ravening edited a comment on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
ravening edited a comment on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-697946918


   > Hi, @ravening can you report the methodology you used?
   > 
   > * ACS version
   > 
   > * How are you executing the tests? Are you migrating VMs running? Stopped VMs? Stopped VMs, and then starting them, and just then migrating again?
   > 
   > 
   > 
   > If you take a look at the code, a null pointer will for sure happen. Just look at the lines I am changing.
   > 
   > 
   > 
   > Before this PR, the first migration will have `host_id` or `last_host_id`, but during the migrate, ACS will set the field `last_host_id` to null using the `afterStorageMigrationCleanup` method, and on the next executions of the migrate command (without starting the VM), this field will be null and then on this line, a NPE will be launched
   > 
   > https://github.com/apache/cloudstack/blob/d657ef7d5b4594b638eaf7df312b11efb99000fb/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L2282-L2283
   > 
   > 
   > 
   > Furthermore, some of these parameters were never used for some hypervisors, seems like VMWare needs it, that is why I moved it where they will be used, and did the necessary validations providing information about the process with an appropriate log message
   
   @RodrigoDLopez im using ACS 4.14
   
   I doing regular VM migration of running VM between multiple hosts. I didn't do migration with storage. Also I'm doing migration on KVM host and not VMware


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] ravening commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-697946918


   > Hi, @ravening can you report the methodology you used?
   > 
   > * ACS version
   > 
   > * How are you executing the tests? Are you migrating VMs running? Stopped VMs? Stopped VMs, and then starting them, and just then migrating again?
   > 
   > 
   > 
   > If you take a look at the code, a null pointer will for sure happen. Just look at the lines I am changing.
   > 
   > 
   > 
   > Before this PR, the first migration will have `host_id` or `last_host_id`, but during the migrate, ACS will set the field `last_host_id` to null using the `afterStorageMigrationCleanup` method, and on the next executions of the migrate command (without starting the VM), this field will be null and then on this line, a NPE will be launched
   > 
   > https://github.com/apache/cloudstack/blob/d657ef7d5b4594b638eaf7df312b11efb99000fb/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L2282-L2283
   > 
   > 
   > 
   > Furthermore, some of these parameters were never used for some hypervisors, seems like VMWare needs it, that is why I moved it where they will be used, and did the necessary validations providing information about the process with an appropriate log message
   
   @RodrigoDLopez im using ACS 4.14
   
   I doing regular VM migration between multiple hosts. I didn't do migration with storage. Also I'm doing migration on KVM host as not VMware


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-856718947


   I'm okay to get it in 4.15.1 if it's fixing the NPE. cc @shwstppr @Pearl1594 


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-696020884


   @blueorangutan test matrix


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-696021161






----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-763206900


   > Looks good, I've left a few comments
   
   I will make this changes. Thanks for the hint


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-696021161


   @DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware67, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] rafaelweingartner commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
rafaelweingartner commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-866882776


   @RodrigoDLopez why is this one not needed anymore? Was it addressed somewhere else?


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on a change in pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on a change in pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#discussion_r493782120



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -2372,18 +2368,32 @@ private void setDestinationPoolAndReallocateNetwork(StoragePool destPool, VMInst
         }
     }
 
-    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm, HostVO srcHost, Long srcClusterId) {
+    private void afterStorageMigrationVmwareVMcleanup(StoragePool destPool, VMInstanceVO vm) {
         // OfflineVmwareMigration: this should only happen on storage migration, else the guru would already have issued the command
-        final Long destClusterId = destPool.getClusterId();
-        if (srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId)) {
-            final String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
-            final String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
-            if (srcDcName != null && destDcName != null && !srcDcName.equals(destDcName)) {
-                removeStaleVmFromSource(vm, srcHost);
-            }
+        Long destClusterId = destPool.getClusterId();
+        Long srcHostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId();
+        if (srcHostId == null) {
+            s_logger.debug(String.format("No Host ID was found when doing cleanup after Vmware migration for VM with ID = [%d] and Cluster destination ID = [%d]", vm.getId(), destClusterId));
+            return;
+        }
+        HostVO srcHost = _hostDao.findById(srcHostId);
+        if (srcHost == null) {
+            s_logger.debug(String.format("When doing cleanup after Vmware migration could not find a host for the given ID = [%d]", srcHostId));
+            return;
+        }
+        Long srcClusterId = srcHost.getClusterId();
+        if (srcClusterId.equals(destClusterId)) {
+            s_logger.debug("Since the Source cluster ID [%s] is equal to the Destination cluster ID [%s] we do not need to proceed with the clean up after migration");
+            return;
+        }
+        String srcDcName = _clusterDetailsDao.getVmwareDcName(srcClusterId);
+        String destDcName = _clusterDetailsDao.getVmwareDcName(destClusterId);
+        if(StringUtils.isNotBlank(srcDcName) && StringUtils.isNotBlank(destDcName)){

Review comment:
       Hi @rhtyd, on line 2385 we check if the srcClusterId is equal to destClusterId and if so, ACS will show a log message give information about this and do nothing




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-687650442


   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] RodrigoDLopez commented on pull request #4282: Prevents null pointers when doing consecutive VM migrates

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #4282:
URL: https://github.com/apache/cloudstack/pull/4282#issuecomment-700736138


   Hi @ravening, that was my fault.
   You are doing a live migration, this NPE will occur when you: 
   - stop an instance;
   - migrate it with volume
   - do not start the instance
   - try to migrate again to another pool
   
   with these steps, after the first migrate `host_id` and `last_host_id` will be set to `null`. And then, when you try to migrate it again a NPE will be thrown.


----------------------------------------------------------------
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:
users@infra.apache.org