fix(xo-web/VM): SR no longer required if a migration network is selected

vatesfr · Mar 6, 2024 · f84cef7 · f84cef7
1 parent 1f856cc
commit f84cef7
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 36 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -14,6 +14,7 @@
 - [ISO SR] During ISO migration, the destination SRs were not ISO SRs [#7392](https://github.com/vatesfr/xen-orchestra/issues/7392) (PR [#7431](https://github.com/vatesfr/xen-orchestra/pull/7431))
 - [VM/Migration] Fix VDIs that were not migrated to the destination SR (PR [#7360](https://github.com/vatesfr/xen-orchestra/pull/7360))
 - [Home/VM] VMs migration from the home view will no longer execute a [Migration with Storage Motion](https://github.com/vatesfr/xen-orchestra/blob/master/docs/manage_infrastructure.md#vm-migration-with-storage-motion-vmmigrate_send) unless it is necessary [Forum#8279](https://xcp-ng.org/forum/topic/8279/getting-errors-when-migrating-4-out-5-vmguest/)(PR [#7360](https://github.com/vatesfr/xen-orchestra/pull/7360))
+- [VM/Migration] SR is no longer required if you select a migration network (PR [#7360](https://github.com/vatesfr/xen-orchestra/pull/7360))
 
 ### Packages to release
 

diff --git a/packages/xo-web/src/common/intl/messages.js b/packages/xo-web/src/common/intl/messages.js
@@ -2062,6 +2062,7 @@ const messages = {
   vmsWithDuplicatedMacAddressesMessage:
     '{nVms, number} VM{nVms, plural, one {} other {s}} contain{nVms, plural, one {s} other {}} duplicate MAC addresses or {nVms, plural, one {has} other {have}} the same MAC addresses as other running VMs. Do you want to continue?',
   ignoreVdi: 'Ignore this VDI',
+  selectDestinationSr: 'Select a destination SR',
 
   // ----- Servers -----
   enableServerErrorTitle: 'Enable server',

diff --git a/packages/xo-web/src/common/xo/choose-sr-for-each-vdis-modal/index.js b/packages/xo-web/src/common/xo/choose-sr-for-each-vdis-modal/index.js
@@ -59,22 +59,29 @@ export default class ChooseSrForEachVdisModal extends Component {
       ignorableVdis = false,
       mainSrPredicate = isSrWritable,
       placeholder,
+      required,
       srPredicate = mainSrPredicate,
       value: { mainSr, mapVdisSrs },
       vdis,
     } = props
 
     return (
       <div>
-        <SelectSr
-          onChange={this._onChangeMainSr}
-          placeholder={placeholder !== undefined ? placeholder : _('chooseSrForEachVdisModalMainSr')}
-          predicate={mainSrPredicate}
-          required
-          value={mainSr}
-        />
+        <SingleLineRow>
+          <Col size={6}>{_('selectDestinationSr')}</Col>
+          <Col size={6}>
+            <SelectSr
+              onChange={this._onChangeMainSr}
+              placeholder={placeholder !== undefined ? placeholder : _('chooseSrForEachVdisModalMainSr')}
+              predicate={mainSrPredicate}
+              required={required}
+              value={mainSr}
+            />
+          </Col>
+        </SingleLineRow>
+        {!required && <i>{_('optionalEntry')}</i>}
         <br />
-        {!isEmpty(vdis) && mainSr != null && (
+        {!isEmpty(vdis) && (
           <Collapsible buttonText={_('chooseSrForEachVdisModalSelectSr')} collapsible size='small'>
             <br />
             <Container>

diff --git a/packages/xo-web/src/common/xo/index.js b/packages/xo-web/src/common/xo/index.js
@@ -1689,17 +1689,16 @@ export const migrateVm = async (vm, host) => {
     return
   }
 
-  const { migrationNetwork, sr, targetHost } = params
+  const { sr, srRequired, targetHost } = params
 
   if (!targetHost) {
     return error(_('migrateVmNoTargetHost'), _('migrateVmNoTargetHostMessage'))
   }
 
-  // Workaround to prevent VM's VDIs from unexpectedly migrating to the default SR
-  // if migration network is defined, the SR is required.
-  if (migrationNetwork !== undefined && sr === undefined) {
+  if (srRequired && sr === undefined) {
     return error(_('migrateVmNoSr'), _('migrateVmNoSrMessage'))
   }
+  delete params.srRequired
 
   try {
     await _call('vm.migrate', { vm: vm.id, ...params })
@@ -1734,6 +1733,11 @@ export const migrateVms = vms =>
       return error(_('migrateVmNoTargetHost'), _('migrateVmNoTargetHostMessage'))
     }
 
+    if (params.srRequired && params.sr === undefined) {
+      return error(_('migrateVmNoTargetHost'), _('migrateVmNoTargetHostMessage'))
+    }
+    delete params.srRequired
+
     const { mapVmsMapVdisSrs, mapVmsMapVifsNetworks, migrationNetwork, sr, targetHost, vms } = params
     Promise.all(
       map(vms, ({ id }) =>

diff --git a/packages/xo-web/src/common/xo/migrate-vm-modal/index.js b/packages/xo-web/src/common/xo/migrate-vm-modal/index.js
@@ -122,6 +122,7 @@ export default class MigrateVmModalBody extends BaseComponent {
       migrationNetwork: this.state.migrationNetworkId,
       sr: resolveId(this.state.targetSrs.mainSr),
       targetHost: this.state.host && this.state.host.id,
+      srRequired: !this.state.doNotMigrateVdis,
     }
   }
 
@@ -222,14 +223,14 @@ export default class MigrateVmModalBody extends BaseComponent {
             </Col>
           </SingleLineRow>
         </div>
-        {host && (!doNotMigrateVdis || migrationNetworkId != null) && (
+        {host && (
           <div className={styles.groupBlock}>
             <SingleLineRow>
               <Col size={12}>
                 <ChooseSrForEachVdisModal
                   mainSrPredicate={this._getSrPredicate()}
                   onChange={this.linkState('targetSrs')}
-                  required
+                  required={!doNotMigrateVdis}
                   value={targetSrs}
                   vdis={vdis}
                 />

diff --git a/packages/xo-web/src/common/xo/migrate-vms-modal/index.js b/packages/xo-web/src/common/xo/migrate-vms-modal/index.js
@@ -108,7 +108,15 @@ export default class MigrateVmsModalBody extends BaseComponent {
       return { vms }
     }
     const { networks, pifs, vbdsByVm, vifsByVm } = this.props
-    const { doNotMigrateVdi, doNotMigrateVmVdis, migrationNetworkId, networkId, smartVifMapping, srId } = this.state
+    const {
+      doNotMigrateVdi,
+      doNotMigrateVmVdis,
+      migrationNetworkId,
+      noVdisMigration,
+      networkId,
+      smartVifMapping,
+      srId,
+    } = this.state
 
     // Map VM --> ( Map VDI --> SR )
     // - Intra-pool: a VDI will only be migrated to the selected SR if the VDI was on a local SR or if an SR was explicitly selected.
@@ -159,6 +167,7 @@ export default class MigrateVmsModalBody extends BaseComponent {
       mapVmsMapVifsNetworks,
       migrationNetwork: migrationNetworkId,
       sr: srId,
+      srRequired: !noVdisMigration,
       targetHost: host.id,
       vms,
     }
@@ -253,27 +262,11 @@ export default class MigrateVmsModalBody extends BaseComponent {
             </Col>
           </SingleLineRow>
         </div>
-        {host !== undefined && (
-          <div style={LINE_STYLE}>
-            <SingleLineRow>
-              <Col size={6}>{_('migrateVmSelectMigrationNetwork')}</Col>
-              <Col size={6}>
-                <SelectNetwork
-                  onChange={this._selectMigrationNetwork}
-                  predicate={this._getMigrationNetworkPredicate()}
-                  required={!intraPool}
-                  value={migrationNetworkId}
-                />
-              </Col>
-            </SingleLineRow>
-            {intraPool && <i>{_('optionalEntry')}</i>}
-          </div>
-        )}
-        {host && (!noVdisMigration || migrationNetworkId != null) && (
+        {host !== undefined && [
           <div key='sr' style={LINE_STYLE}>
             <SingleLineRow>
               <Col size={6}>
-                {!intraPool ? _('migrateVmsSelectSr') : _('migrateVmsSelectSrIntraPool')}{' '}
+                {_('selectDestinationSr')}{' '}
                 {(defaultSrId === undefined || !defaultSrConnectedToHost) && (
                   <Tooltip
                     content={
@@ -291,11 +284,31 @@ export default class MigrateVmsModalBody extends BaseComponent {
                 )}
               </Col>
               <Col size={6}>
-                <SelectSr onChange={this._selectSr} predicate={this._getSrPredicate()} required value={srId} />
+                <SelectSr
+                  onChange={this._selectSr}
+                  predicate={this._getSrPredicate()}
+                  required={!noVdisMigration}
+                  value={srId}
+                />
               </Col>
             </SingleLineRow>
-          </div>
-        )}
+            {noVdisMigration && <i>{_('optionalEntry')}</i>}
+          </div>,
+          <div style={LINE_STYLE} key='network'>
+            <SingleLineRow>
+              <Col size={6}>{_('migrateVmSelectMigrationNetwork')}</Col>
+              <Col size={6}>
+                <SelectNetwork
+                  onChange={this._selectMigrationNetwork}
+                  predicate={this._getMigrationNetworkPredicate()}
+                  required={!intraPool}
+                  value={migrationNetworkId}
+                />
+              </Col>
+            </SingleLineRow>
+            {intraPool && <i>{_('optionalEntry')}</i>}
+          </div>,
+        ]}
         {host && !intraPool && (
           <div key='network' style={LINE_STYLE}>
             <SingleLineRow>