@blueorangutan package

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

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

@blueorangutan test centos7 vmware-67u3

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

Trillian test result (tid-2576)
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:

@blueorangutan package

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

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

@blueorangutan package

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

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

@blueorangutan package

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

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

@blueorangutan test matrix

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

Trillian test result (tid-2818)
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:

@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.

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

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

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

cloudstack/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

Lines 2282 to 2283 in d657ef7

	final HostVO srcHost = _hostDao.findById(srchostId);
	final Long srcClusterId = srcHost.getClusterId();

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

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.

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

@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

@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?

@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

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

Looks good, I've left a few comments

I will make this changes. Thanks for the hint

@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?

@RodrigoDLopez can you fix the conflict?

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?

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

@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

@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.

@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

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

@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.

@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.

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

not needed anymore...

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