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) {



_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to