feat(xo-web/migration): dont force migration by default (#4364)

Fixes #2136

CHANGELOG.unreleased.md

@@ -15,6 +15,7 @@
 - [Tag] Adding a tag: ability to select from existing tags [#2810](https://github.com/vatesfr/xen-orchestra/issues/2810) (PR [#4530](https://github.com/vatesfr/xen-orchestra/pull/4530))
 - [Smart backup] Ability to manually add custom tags [#2810](https://github.com/vatesfr/xen-orchestra/issues/2810) (PR [#4648](https://github.com/vatesfr/xen-orchestra/pull/4648))
 - [Proxy] Ability to backup VMs via registered proxy [#4254](https://github.com/vatesfr/xen-orchestra/issues/4254) (PR [#4495](https://github.com/vatesfr/xen-orchestra/pull/4495))
+- [VM/Migrate] Ask user before forcing migration [#2136](https://github.com/vatesfr/xen-orchestra/issues/2136) (PR [#4364](https://github.com/vatesfr/xen-orchestra/pull/4364))
 
 ### Bug fixes
 
@@ -29,5 +30,6 @@
 >
 > Rule of thumb: add packages on top.
 
+- xo-common v0.3.0
 - xo-server v5.55.0
 - xo-web v5.55.0

packages/xo-common/src/api-errors.js

@@ -172,3 +172,11 @@ export const patchPrecheckFailed = create(20, ({ errorType, patch }) => ({
   },
   message: `patch precheck failed: ${errorType}`,
 }))
+
+export const operationFailed = create(21, ({ objectId, code }) => ({
+  data: {
+    objectId,
+    code,
+  },
+  message: 'operation failed',
+}))

packages/xo-server/src/api/vm.js

@@ -6,6 +6,7 @@ import {
   forbiddenOperation,
   invalidParameters,
   noSuchObject,
+  operationFailed,
   unauthorized,
 } from 'xo-common/api-errors'
 
@@ -453,6 +454,7 @@ export async function migrate({
   mapVdisSrs,
   mapVifsNetworks,
   migrationNetwork,
+  force,
 }) {
   let mapVdisSrsXapi, mapVifsNetworksXapi
   const permissions = []
@@ -480,24 +482,29 @@ export async function migrate({
 
   await this.checkPermissions(this.user.id, permissions)
 
-  await this.getXapi(vm).migrateVm(
+  await this.getXapi(vm)
-    vm._xapiId,
+    .migrateVm(vm._xapiId, this.getXapi(host), host._xapiId, {
-    this.getXapi(host),
-    host._xapiId,
-    {
       sr: sr && this.getObject(sr, 'SR')._xapiId,
       migrationNetworkId:
         migrationNetwork != null ? migrationNetwork._xapiId : undefined,
       mapVifsNetworks: mapVifsNetworksXapi,
       mapVdisSrs: mapVdisSrsXapi,
-    }
+      force,
-  )
+    })
+    .catch(error => {
+      if (error?.code !== undefined) {
+        throw operationFailed({ objectId: vm.id, code: error.code })
+      }
+      throw error
+    })
 }
 
 migrate.params = {
   // Identifier of the VM to migrate.
   vm: { type: 'string' },
 
+  force: { type: 'boolean', optional: true },
+
   // Identifier of the host to migrate to.
   targetHost: { type: 'string' },
 

packages/xo-server/src/xapi/index.js

@@ -1139,6 +1139,7 @@ export default class Xapi extends XapiBase {
       sr,
       mapVdisSrs,
       mapVifsNetworks,
+      force = false,
     }
   ) {
     // VDIs/SRs mapping
@@ -1186,7 +1187,7 @@ export default class Xapi extends XapiBase {
         vdis,
         vifsMap,
         {
-          force: 'true',
+          force: force ? 'true' : 'false',
         }
         // FIXME: missing param `vgu_map`, it does not cause issues ATM but it
         // might need to be changed one day.
@@ -1440,7 +1441,7 @@ export default class Xapi extends XapiBase {
     vmId,
     hostXapi,
     hostId,
-    { sr, migrationNetworkId, mapVifsNetworks, mapVdisSrs } = {}
+    { force = false, mapVdisSrs, mapVifsNetworks, migrationNetworkId, sr } = {}
   ) {
     const vm = this.getObject(vmId)
     const host = hostXapi.getObject(hostId)
@@ -1460,11 +1461,12 @@ export default class Xapi extends XapiBase {
         sr,
         mapVdisSrs,
         mapVifsNetworks,
+        force,
       })
     } else {
       try {
         await this.callAsync('VM.pool_migrate', vm.$ref, host.$ref, {
-          force: 'true',
+          force: force ? 'true' : 'false',
         })
       } catch (error) {
         if (error.code !== 'VM_REQUIRES_SR') {
@@ -1472,7 +1474,7 @@ export default class Xapi extends XapiBase {
         }
 
         // Retry using motion storage.
-        await this._migrateVmWithStorageMotion(vm, hostXapi, host, {})
+        await this._migrateVmWithStorageMotion(vm, hostXapi, host, { force })
       }
     }
   }

packages/xo-web/src/common/intl/messages.js

@@ -1621,6 +1621,9 @@ const messages = {
   deleteVmBlockedModalTitle: 'Blocked operation',
   deleteVmBlockedModalMessage:
     'Removing the VM is a blocked operation. Would you like to remove it anyway?',
+  forceVmMigrateModalTitle: 'Force migration',
+  forceVmMigrateModalMessage:
+    'The VM is incompatible with the CPU features of the destination host. Would you like to force it anyway?',
   migrateVmModalTitle: 'Migrate VM',
   migrateVmSelectHost: 'Select a destination host:',
   migrateVmSelectMigrationNetwork: 'Select a migration network:',

packages/xo-web/src/common/xo/index.js

@@ -1,7 +1,6 @@
 import asap from 'asap'
 import cookies from 'cookies-js'
 import fpSortBy from 'lodash/fp/sortBy'
-import Icon from 'icon'
 import pFinally from 'promise-toolbox/finally'
 import React from 'react'
 import reflect from 'promise-toolbox/reflect'
@@ -1285,19 +1284,44 @@ export const deleteSnapshots = vms =>
   )
 
 import MigrateVmModalBody from './migrate-vm-modal' // eslint-disable-line import/first
-export const migrateVm = (vm, host) =>
+export const migrateVm = async (vm, host) => {
-  confirm({
+  let params
-    title: _('migrateVmModalTitle'),
+  try {
-    body: <MigrateVmModalBody vm={vm} host={host} />,
+    params = await confirm({
-  }).then(params => {
+      title: _('migrateVmModalTitle'),
-    if (!params.targetHost) {
+      body: <MigrateVmModalBody vm={vm} host={host} />,
-      return error(
+    })
-        _('migrateVmNoTargetHost'),
+  } catch (error) {
-        _('migrateVmNoTargetHostMessage')
+    return
-      )
+  }
+
+  if (!params.targetHost) {
+    return error(_('migrateVmNoTargetHost'), _('migrateVmNoTargetHostMessage'))
+  }
+
+  try {
+    await _call('vm.migrate', { vm: vm.id, ...params })
+  } catch (error) {
+    // https://developer-docs.citrix.com/projects/citrix-hypervisor-management-api/en/latest/api-ref-autogen-errors/#vmincompatiblewiththishost
+    if (
+      error != null &&
+      error.data !== undefined &&
+      error.data.code === 'VM_INCOMPATIBLE_WITH_THIS_HOST'
+    ) {
+      // Retry with force.
+      try {
+        await confirm({
+          body: _('forceVmMigrateModalMessage'),
+          title: _('forceVmMigrateModalTitle'),
+        })
+      } catch (error) {
+        return
+      }
+      return _call('vm.migrate', { vm: vm.id, force: true, ...params })
     }
-    return _call('vm.migrate', { vm: vm.id, ...params })
+    throw error
-  }, noop)
+  }
+}
 
 import MigrateVmsModalBody from './migrate-vms-modal' // eslint-disable-line import/first
 export const migrateVms = vms =>