[pve-devel] [PATCH manager] ui: migrate: add checkbox to enable offline migration with local resources

[Thomas Lamprecht]

On 10/7/19 1:40 PM, Tim Marx wrote:
> Added to make use of [0] and because it does make sense for non HA vm's
> as well, in accordance with #2241.
> 
> [0] pve-ha-manager: 6e8b0c225405da9472f56fe5c94c94b204259caa
> Signed-off-by: Tim Marx <t.marx at proxmox.com>
> ---
>  www/manager6/window/Migrate.js | 63 +++++++++++++++++++++++++++-------
>  1 file changed, 50 insertions(+), 13 deletions(-)
> 
> diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
> index 02ef6bfd..3bab88bd 100644
> --- a/www/manager6/window/Migrate.js
> +++ b/www/manager6/window/Migrate.js
> @@ -25,7 +25,9 @@ Ext.define('PVE.window.Migrate', {
>  		preconditions: [],
>  		'with-local-disks': 0,
>  		mode: undefined,
> -		allowedNodes: undefined
> +		allowedNodes: undefined,
> +		withLocalResources: false,

allowLocalResources or overwriteLocalResourceCheck ?

> +		hasLocalResources: false
>  	    }
>  
>  	},
> @@ -48,6 +50,14 @@ Ext.define('PVE.window.Migrate', {
>  		    } else {
>  			return true;
>  		    }
> +	    },
> +	    setLocalResourceCheckboxHidden: function(get) {
> +		if (get('running') || !get('migration.hasLocalResources') ||
> +		    Proxmox.UserName !== 'root at pam') {
> +		    return true;
> +		} else {
> +		    return false;
> +		}
>  	    }
>  	}
>      },
> @@ -121,6 +131,10 @@ Ext.define('PVE.window.Migrate', {
>  		params.targetstorage = values.targetstorage;
>  	    }
>  
> +	    if (vm.get('migration.withLocalResources')) {
> +		params['force'] = 1;
> +	    }
> +
>  	    Proxmox.Utils.API2Request({
>  		params: params,
>  		url: '/nodes/' + vm.get('nodename') + '/' + vm.get('vmtype') + '/' + vm.get('vmid') + '/migrate',
> @@ -144,11 +158,10 @@ Ext.define('PVE.window.Migrate', {
>  
>  	},
>  
> -	checkMigratePreconditions: function() {
> +	checkMigratePreconditions: function(reset) {

cane we get a more telling name than "reset" ? cannot possibly tell what this
could do at all when just looking at the method name and this parameter name.

>  	    var me = this,
>  		vm = me.getViewModel();
>  
> -
>  	    var vmrec = PVE.data.ResourceStore.findRecord('vmid', vm.get('vmid'),
>  			0, false, false, true);
>  	    if (vmrec && vmrec.data && vmrec.data.running) {
> @@ -156,9 +169,9 @@ Ext.define('PVE.window.Migrate', {
>  	    }
>  
>  	    if (vm.get('vmtype') === 'qemu') {
> -		me.checkQemuPreconditions();
> +		me.checkQemuPreconditions(reset);
>  	    } else {
> -		me.checkLxcPreconditions();
> +		me.checkLxcPreconditions(reset);
>  	    }
>  	    me.lookup('pveNodeSelector').disallowedNodes = [vm.get('nodename')];
>  
> @@ -170,7 +183,7 @@ Ext.define('PVE.window.Migrate', {
>  
>  	},
>  
> -	checkQemuPreconditions: function() {
> +	checkQemuPreconditions: function(reset) {
>  	    var me = this,
>  		vm = me.getViewModel(),
>  		migrateStats;
> @@ -193,6 +206,7 @@ Ext.define('PVE.window.Migrate', {
>  		    // Get migration object from viewmodel to prevent
>  		    // to many bind callbacks
>  		    var migration = vm.get('migration');
> +		    if (reset) migration.possible = true;
>  		    migration.preconditions = [];
>  
>  		    if (migrateStats.allowed_nodes) {
> @@ -212,11 +226,22 @@ Ext.define('PVE.window.Migrate', {
>  		    }
>  
>  		    if (migrateStats.local_resources.length) {
> -			migration.possible = false;
> -			migration.preconditions.push({
> -			    text: 'Can\'t migrate VM with local resources: '+ migrateStats.local_resources.join(', '),
> -			    severity: 'error'
> -			});
> +			migration.hasLocalResources = true;
> +			if(!migration.withLocalResources || vm.get('running')){
> +			    migration.possible = false;
> +			    migration.preconditions.push({
> +				text: Ext.String.format('Can\'t migrate VM with local resources: {0}',
> +				migrateStats.local_resources.join(', ')),
> +				severity: 'error'
> +			    });
> +			} else {
> +			    migration.preconditions.push({
> +				text: Ext.String.format('Migrate VM with local resources: {0}. ' +
> +				'This might fail if resources aren\'t available on the target node.',
> +				migrateStats.local_resources.join(', ')),
> +				severity: 'warning'
> +			    });
> +			}
>  		    }
>  
>  		    if (migrateStats.local_disks.length) {
> @@ -252,7 +277,7 @@ Ext.define('PVE.window.Migrate', {
>  		}
>  	    });
>  	},
> -	checkLxcPreconditions: function() {
> +	checkLxcPreconditions: function(reset) {
>  	    var me = this,
>  		vm = me.getViewModel();
>  	    if (vm.get('running')) {
> @@ -324,7 +349,19 @@ Ext.define('PVE.window.Migrate', {
>  			    bind: {
>  				hidden: '{setStorageselectorHidden}'
>  			    }
> -		    }]
> +		    },
> +		    {
> +			xtype: 'proxmoxcheckbox',
> +			name: 'withLocalResources',
> +			fieldLabel: gettext('With local resources'),

hmm, maybe this is better suited for a two column spanning checkbox using
a boxLabel, which is rightAligned?`As then we could be a bit more expressive
here, alà 
"Overwrite local resources unavailable check"

Alternatively we could call this just "Force" and have above in a tool tip?

> +			bind: {
> +			    hidden: '{setLocalResourceCheckboxHidden}',
> +			    value: '{migration.withLocalResources}'
> +			},
> +			listeners: {
> +			    change: {fn: 'checkMigratePreconditions', extraArg: true}
> +			}
> +		}]
>  		}
>  	    ]
>  	},
>