Fix NPE when migrating a VM that had its template removed

[Rubueno]

Description

This PR fixes an NPE when a VM had its template removed while the template still has running VMs.

Possibly fixes #8827 but not sure as I've not worked with imported VMs.

Types of changes

• [ ] 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)
• [ ] Enhancement (improves an existing feature and functionality)
• [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
• [ ] build/CI

Bug Severity

• [ ] BLOCKER
• [ ] Critical
• [ ] Major
• [ ] Minor
• [x] Trivial

How Has This Been Tested?

We have this currently running in our 4.16 environment and are bringing the change into a new release.

How did you try to break this feature and the system with this change?

It does not break but instead resolves an issue.

[gpordeus]

Hello! I think this hits the same roadblock as #7615, where it ends up migrating without copying the template and it corrupts the VM due to a missing backing file.

@JoaoJandre can you take a look?

[JoaoJandre]

Hello! I think this hits the same roadblock as #7615, where it ends up migrating without copying the template and it corrupts the VM due to a missing backing file.

@JoaoJandre can you take a look?

@Rubueno @gpordeus yeah this solution has the same pitfall of #7615. I actually have another solution for this issue (and some others that we have noticed with the VM + volume migration), but I have my hands full this week and will only be able to open the PR(s) next week.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Project coverage is 30.76%. Comparing base (08d9d06) to head (3c1d9a1). Report is 200 commits behind head on 4.19.

Files	Patch %	Lines
...torage/motion/StorageSystemDataMotionStrategy.java	0.00%	12 Missing :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #8834      +/-   ##
============================================
- Coverage     30.99%   30.76%   -0.24%     
+ Complexity    34375    34089     -286     
============================================
  Files          5355     5355              
  Lines        376686   376694       +8     
  Branches      54815    54816       +1     
============================================
- Hits         116749   115884     -865     
- Misses       244551   245532     +981     
+ Partials      15386    15278     -108     

Flag	Coverage Δ
simulator-marvin-tests	24.47% <0.00%> (-0.39%)	:arrow_down:
uitests	4.38% <ø> (ø)
unit-tests	16.57% <0.00%> (-0.01%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[DaanHoogland]

@JoaoJandre how does your #8911 relate to this? are both sensible/needed or do they conflict?

[JoaoJandre]

@JoaoJandre how does your #8911 relate to this? are both sensible/needed or do they conflict?

They do conflict, this PR changes the template copy on live migration. #8911 removes the template copy from the live migration, since #8911 always consolidates the volume, no need to copy the template.

Furthermore, I believe that this PR will have the same issue as #7615 where the VM can be corrupted.

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10078

[github-actions[bot]]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[rohityadavcloud]

@Rubueno can you fix the conflict?

[Rubueno]

Currently traveling, won't be back for another two weeks. Should this still be merged even when it results in broken VMs due to missing backing disks according to #7615?On 25 Jun 2024, at 19:55, Rohit Yadav @.***> wrote: @Rubueno can you fix the conflict?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[DaanHoogland]

@Rubueno #7615 is reported to be fixed in #9259. Should this still be merged/is it still relevant?

[Rubueno]

I don't think so then. I'll close this.