[pve-devel] [RFC manager] When adding a new hard disk, use the most used controller as suggested value

[Dominik Csapak]

comment inline

On 09/21/2016 05:44 PM, Emmanuel Kasper wrote:
> This is a complementary fix for #1105 (Create Linux VM Wizard: use scsi
> as default bus/device) and add some logic to the list of controllers
> presented in the ControllerSelector combo box
>
> Since we can have IDE, SCSI, Virtio(blk) as a controller during installation,
> based on OS detection and personal preferences, we can reasonably assume
> on 80 % of cases it will be the same controller we want to use for the
> next time we add a hardisk.
>
> This allows backward compatibility for Linux guests which were proposed a
> virtio-blk as first choice, and also helps newly created Linux VMs by proposing
> SCSI.
> ---
>  www/manager6/form/ControllerSelector.js | 54 +++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
> index 1b2946a..6dec05a 100644
> --- a/www/manager6/form/ControllerSelector.js
> +++ b/www/manager6/form/ControllerSelector.js
> @@ -17,6 +17,54 @@ Ext.define('PVE.form.ControllerSelector', {
>
>      vmconfig: {}, // used to check for existing devices
>
> +    sortByPreviousUsage: function(vmconfig, controllerList) {
> +	var sortedList = [];
> +
> +	var usedControllers = Ext.clone(PVE.form.ControllerSelector.maxIds);
> +
> +	var type;
> +	for (type in usedControllers) {
> +	    if(usedControllers.hasOwnProperty(type)) {
> +		usedControllers[type] = 0;
> +	    }
> +	}
> +
> +	var property;
> +	for (property in vmconfig) {
> +	    if (vmconfig.hasOwnProperty(property)) {
> +		if (property.match(/^ide\d+/) && !vmconfig[property].match(/media=cdrom/)) {
> +		    usedControllers.ide++;
> +		} else if (property.match(/^sata\d+/)) {
> +		    usedControllers.sata++;
> +		} else if (property.match(/^virtio\d+/)) {
> +		    usedControllers.virtio++;
> +		} else if (property.match(/^scsi\d+/)) {
> +		    usedControllers.scsi++;
> +		}
> +	    }
> +	}
> +
> +	var arrayControllers = [
> +	    {name:'ide', value:usedControllers.ide},
> +	    {name:'sata', value:usedControllers.sata},
> +	    {name:'virtio', value:usedControllers.virtio},
> +	    {name:'scsi', value:usedControllers.scsi}
> +	].sort(function compare(a, b){
> +	    return b.value - a.value;
> +	});

instead of hardcoding the interface names 4 times, it think it would  be 
better to parse the disks with e.g.
^(ide|scsi|sata|virtio)(\d+)$

and generate an array with it (which can the be sorted like above)

also we could put this regex in utils, because there is at least one 
other point in the code (BootOrderEdit) where we use a similar one,
so that when we add an interface type in the future, we only have to 
edit one regex

> +
> +	if (arrayControllers[0].value > arrayControllers[1].value ) {
> +	    // we have a favourite !
> +	    var favourite = arrayControllers[0].name;
> +	    sortedList = Ext.Array.remove(controllerList, favourite);
> +	    sortedList.unshift(favourite);
> +	    return sortedList;
> +	}
> +	else {
> +	    return controllerList;
> +	}
> +    },
> +
>      setVMConfig: function(vmconfig, autoSelect) {
>  	var me = this;
>
> @@ -30,8 +78,10 @@ Ext.define('PVE.form.ControllerSelector', {
>  		    me.down('field[name=deviceid]').setValue(2);
>  		    return;
>  		}
> -	    } else if (me.vmconfig.ostype === 'l26') {
> -		clist = ['virtio', 'ide', 'scsi', 'sata'];
> +	    } else  {
> +		// in most cases we want to add a disk to the same controller
> +		// we previously used
> +		clist = me.sortByPreviousUsage(me.vmconfig, clist);
>  	    }
>
>  	    Ext.Array.each(clist, function(controller) {
>