feat(xo-server/vm.migrate): call VM.assert_can_migrate before

[ByScripts]

Fixes #5301

Check list

Check if done, if not relevant leave unchecked.

• PR reference the relevant issue (e.g. Fixes #007 or See xoa-support#42)
• if UI changes, a screenshot has been added to the PR
• documentation updated
• CHANGELOG.unreleased.md:
  • enhancement/bug fix entry added
  • list of packages to release updated (${name} v${new version})
• I have tested added/updated features (and impacted code)
  • unit tests (e.g. cron/parse.spec.js)
  • if xo-server API changes, the corresponding test has been added to/updated on xo-server-test
  • at least manual testing

Process

1. create a PR as soon as possible
2. mark it as WiP: (Work in Progress) if not ready to be merged
3. when you want a review, add a reviewer (and only one)
4. if necessary, update your PR, and re- add a reviewer

From the Four Agreements:

1. Be impeccable with your word.
2. Don't take anything personally.
3. Don't make assumptions.
4. Always do your best.

[julien-f]

LGTM, remove draft status and I'll merge.

[olivierlambert]

It's not clear to me functionally speaking. Are we displaying the result of assert can migrate in the UI with a modal and allowing us to force anyway?

[julien-f]

@olivierlambert The result of the asset will be displayed as a migrate error.

No, this will not display a modal and allow bypassing, it was not part of the spec.

In what case bypassing the assert is relevant? When can it be wrong? From the doc, it appears to be only checking the lisence.

[olivierlambert]

The spec wasn't complete enough, I agree (it wasn't really a spec TBH). But this might have huge consequences if we push this without anyway to bypass it if needed, without knowing anything about it. Or at least, we can't merge something without more understanding on what it means (for the user functional perspective).

We should at least get someone to test this on the lab in various cases (not necessarily from XO team BTW).

[julien-f]

Ok, I propose adding this change in the server with a flag to bypass it and adding a way to bypass it in the UI in an other PR.
If the UI part is not ready for the release, we'll do a patch at the first reported issue.

Is it good for you?

[olivierlambert]

Okay let's do this. Note that "assert can migrate" will check a lot of things on XAPI's side, that's why I'm a bit concerned about the consequences (especially if we do not get the reason directly in the UI, that will raise confusion amongst our users)