[pve-devel] [PATCH manager 2/2] ui: improve boot order editor with 'bootorder' support
Thomas Lamprecht
t.lamprecht at proxmox.com
Mon Sep 28 14:27:33 CEST 2020
On 24.09.20 16:11, Stefan Reiter wrote:
> The new 'bootorder' property can express many more scenarios than the
> old 'boot'/'bootdisk' ones. Update the editor so it can handle it.
>
> Features a grid with all supported boot devices which can be reordered
> using drag-and-drop, as well as toggled on and off with an inline
> checkbox.
>
> Support for configs still using the old properties is given, with the
> first write automatically updating the VM config to use the new one.
>
> The renderer for the Options panel is updated with support for the new
> format, and the display format for the fallback is changed to make it
> look uniform.
>
> Note that it is very well possible to disable all boot devices, in which
> case an empty 'bootorder: ' will be stored to the config file. I'm not
> sure what that would be useful for, but there's no reason to forbid it
> either, just warn the user that it's probably not what he wants.
>
It's not bad, I like it in general! I'd a few things though.
* Add a order icon ( https://fontawesome.com/v4.7.0/icon/bars has "reorder" as
alias) to rows
* Add a # column which shows the "order", i.e., similar to our firewall rules
* Use another renderer for the Options view, IMO the "A > B > C" makes people
think to hard, rather just use number, i.e., to something like
"1. scsi0, 2. Any CD-ROM, ..."#
Also, please keep the case and style of CD-ROM as is, no point in changing that.
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> www/manager6/qemu/BootOrderEdit.js | 310 +++++++++++++++++------------
> www/manager6/qemu/Options.js | 32 ++-
> 2 files changed, 210 insertions(+), 132 deletions(-)
>
> diff --git a/www/manager6/qemu/BootOrderEdit.js b/www/manager6/qemu/BootOrderEdit.js
> index 19d5d50a..b1236bbd 100644
> --- a/www/manager6/qemu/BootOrderEdit.js
> +++ b/www/manager6/qemu/BootOrderEdit.js
> @@ -1,149 +1,208 @@
> +Ext.define('pve-boot-order-entry', {
> + extend: 'Ext.data.Model',
> + fields: [
> + {name: 'name', type: 'string'},
> + {name: 'enabled', type: 'bool'},
> + {name: 'desc', type: 'string'},
> + ]
> +});
> +
> Ext.define('PVE.qemu.BootOrderPanel', {
> extend: 'Proxmox.panel.InputPanel',
> alias: 'widget.pveQemuBootOrderPanel',
> +
> vmconfig: {}, // store loaded vm config
> + store: undefined,
> + originalValue: undefined,
>
> - bootdisk: undefined,
> - selection: [],
> - list: [],
> - comboboxes: [],
> -
> - isBootDisk: function(value) {
> + isDisk: function(value, cdrom) {
> return PVE.Utils.bus_match.test(value);
> },
>
> - setVMConfig: function(vmconfig) {
> - var me = this;
> - me.vmconfig = vmconfig;
> - var order = me.vmconfig.boot || 'cdn';
> - me.bootdisk = me.vmconfig.bootdisk || undefined;
> -
> - // get the first 3 characters
> - // ignore the rest (there should never be more than 3)
> - me.selection = order.split('').slice(0,3);
> -
> - // build bootdev list
> - me.list = [];
> - Ext.Object.each(me.vmconfig, function(key, value) {
> - if (me.isBootDisk(key) &&
> - !(/media=cdrom/).test(value)) {
> - me.list.push([key, "Disk '" + key + "'"]);
> - }
> - });
> -
> - me.list.push(['d', 'CD-ROM']);
> - me.list.push(['n', gettext('Network')]);
> - me.list.push(['__none__', Proxmox.Utils.noneText]);
> -
> - me.recomputeList();
> -
> - me.comboboxes.forEach(function(box) {
> - box.resetOriginalValue();
> - });
> + isBootdev: function(dev, value) {
> + return this.isDisk(dev) ||
> + (/^net\d+/).test(dev) ||
> + (/^hostpci\d+/).test(dev) ||
> + ((/^usb\d+/).test(dev) && !(/spice/).test(value));
> },
>
> - onGetValues: function(values) {
> - var me = this;
> - var order = me.selection.join('');
> - var res = { boot: order };
> + setVMConfig: function(vmconfig) {
> + let me = this;
> + me.vmconfig = vmconfig;
>
> - if (me.bootdisk && order.indexOf('c') !== -1) {
> - res.bootdisk = me.bootdisk;
> + me.store.removeAll();
> +
> + let bootorder;
> + if (me.vmconfig.bootorder) {
> + bootorder = (/^\s*$/).test(me.vmconfig.bootorder) ? [] :
> + me.vmconfig.bootorder
> + .split(',')
> + .map(dev => ({name: dev, enabled: true}));
> } else {
> - res['delete'] = 'bootdisk';
> + // legacy style, transform to new bootorder
> + let order = me.vmconfig.boot || 'cdn';
> + let bootdisk = me.vmconfig.bootdisk || undefined;
> +
> + // get the first 3 characters
> + // ignore the rest (there should never be more than 3)
> + let orderList = order.split('').slice(0,3);
> +
> + // build bootdev list
> + bootorder = [];
> + for (let i = 0; i < orderList.length; i++) {
> + let list = [];
> + if (orderList[i] === 'c') {
> + if (bootdisk !== undefined && me.vmconfig[bootdisk]) {
> + list.push(bootdisk);
> + }
> + } else if (orderList[i] === 'd') {
> + Ext.Object.each(me.vmconfig, function(key, value) {
> + if (me.isDisk(key) && (/media=cdrom/).test(value)) {
> + list.push(key);
> + }
> + });
> + } else if (orderList[i] === 'n') {
> + Ext.Object.each(me.vmconfig, function(key, value) {
> + if ((/^net\d+/).test(key)) {
> + list.push(key);
> + }
> + });
> + }
> +
> + // Object.each iterates in random order, sort alphabetically
> + list.sort();
> + list.forEach(dev => bootorder.push({name: dev, enabled: true}));
> + }
> }
>
> + // add disabled devices as well
> + let disabled = [];
> + Ext.Object.each(me.vmconfig, function(key, value) {
> + if (me.isBootdev(key, value) &&
> + !Ext.Array.some(bootorder, x => x.name === key))
> + {
> + disabled.push(key);
> + }
> + });
> + disabled.sort();
> + disabled.forEach(dev => bootorder.push({name: dev, enabled: false}));
> +
> + bootorder.forEach(entry => {
> + entry.desc = me.vmconfig[entry.name];
> + });
> +
> + me.store.insert(0, bootorder);
> + me.store.fireEvent("update");
> + },
> +
> + calculateValue: function() {
> + let me = this;
> + return me.store.getData().items
> + .filter(x => x.data.enabled)
> + .map(x => x.data.name)
> + .join(',');
> + },
> +
> + onGetValues: function() {
> + let me = this;
> + // Note: we allow an empty value, so no 'delete' option
> + let res = { bootorder: me.calculateValue() };
> return res;
> },
>
> - recomputeSelection: function(combobox, newVal, oldVal) {
> - var me = this.up('#inputpanel');
> - me.selection = [];
> - me.comboboxes.forEach(function(item) {
> - var val = item.getValue();
> -
> - // when selecting an already selected item,
> - // switch it around
> - if ((val === newVal || (me.isBootDisk(val) && me.isBootDisk(newVal))) &&
> - item.name !== combobox.name &&
> - newVal !== '__none__') {
> - // swap items
> - val = oldVal;
> - }
> -
> - // push 'c','d' or 'n' in the array
> - if (me.isBootDisk(val)) {
> - me.selection.push('c');
> - me.bootdisk = val;
> - } else if (val === 'd' ||
> - val === 'n') {
> - me.selection.push(val);
> - }
> - });
> -
> - me.recomputeList();
> - },
> -
> - recomputeList: function(){
> - var me = this;
> - // set the correct values in the kvcomboboxes
> - var cnt = 0;
> - me.comboboxes.forEach(function(item) {
> - if (cnt === 0) {
> - // never show 'none' on first combobox
> - item.store.loadData(me.list.slice(0, me.list.length-1));
> - } else {
> - item.store.loadData(me.list);
> - }
> - item.suspendEvent('change');
> - if (cnt < me.selection.length) {
> - item.setValue((me.selection[cnt] !== 'c')?me.selection[cnt]:me.bootdisk);
> - } else if (cnt === 0){
> - item.setValue('');
> - } else {
> - item.setValue('__none__');
> - }
> - cnt++;
> - item.resumeEvent('change');
> - item.validate();
> - });
> - },
> -
> initComponent : function() {
> - var me = this;
> + let me = this;
>
> - // this has to be done here, because of
> - // the way our inputPanel class handles items
> - me.comboboxes = [
> - Ext.createWidget('proxmoxKVComboBox', {
> - fieldLabel: gettext('Boot device') + " 1",
> - labelWidth: 120,
> - name: 'bd1',
> - allowBlank: false,
> - listeners: {
> - change: me.recomputeSelection
> + // for dirty marking and reset detection
> + let inUpdate = false;
> + let marker = Ext.create('Ext.form.field.Base', {
> + hidden: true,
> + itemId: 'marker',
> + setValue: function(val) {
> + // on form reset, go back to original state
> + if (!inUpdate) {
> + me.setVMConfig(me.vmconfig);
> }
> - }),
> - Ext.createWidget('proxmoxKVComboBox', {
> - fieldLabel: gettext('Boot device') + " 2",
> - labelWidth: 120,
> - name: 'bd2',
> - allowBlank: false,
> - listeners: {
> - change: me.recomputeSelection
> +
> + // not a subclass, so no callParent; just do it manually
> + this.setRawValue(this.valueToRaw(val));
> + return this.mixins.field.setValue.call(this, val);
> + }
> + });
> +
> + let emptyWarning = Ext.create('Ext.form.field.Display', {
> + userCls: 'pmx-hint',
> + value: gettext('Warning: No devices selected, the VM will probably not boot!'),
> + });
> +
> + me.store = Ext.create('Ext.data.Store', {
> + model: 'pve-boot-order-entry',
> + listeners: {
> + update: function() {
> + this.commitChanges();
> + let val = me.calculateValue();
> + if (!marker.originalValue) {
> + marker.originalValue = val;
> + }
> + inUpdate = true;
> + marker.setValue(val);
> + inUpdate = false;
> + marker.checkDirty();
> + emptyWarning.setHidden(val !== '');
> }
> - }),
> - Ext.createWidget('proxmoxKVComboBox', {
> - fieldLabel: gettext('Boot device') + " 3",
> - labelWidth: 120,
> - name: 'bd3',
> - allowBlank: false,
> - listeners: {
> - change: me.recomputeSelection
> + }
> + });
> +
> + let grid = Ext.create('Ext.grid.Panel', {
> + store: me.store,
It seems to me that this all could be made more schematic without to much
work, could be nice.
> + margin: '0 0 5 0',
> + columns: [
> + {
> + xtype: 'checkcolumn',
> + header: gettext('Enabled'),
> + dataIndex: 'enabled',
> + width: 70,
> + sortable: false,
> + hideable: false,
> + draggable: false,
> + },
> + {
> + header: gettext('Device'),
> + dataIndex: 'name',
> + width: 70,
> + sortable: false,
> + hideable: false,
> + draggable: false,
> + },
> + {
> + header: gettext('Description'),
> + dataIndex: 'desc',
> + flex: true,
> + sortable: false,
> + hideable: false,
> + draggable: false,
> + },
> + ],
> + viewConfig: {
> + plugins: {
> + ptype: 'gridviewdragdrop',
> + dragText:gettext('Drag and drop to reorder'),
> }
> - })
> - ];
> - Ext.apply(me, { items: me.comboboxes });
> + },
> + listeners: {
> + drop: function() {
> + // doesn't fire automatically on reorder
> + me.store.fireEvent("update");
> + }
> + },
> + });
> +
> + let info = Ext.create('Ext.Component', {
> + html: gettext('Drag and drop to reorder'),
> + });
> +
> + Ext.apply(me, { items: [grid, info, emptyWarning, marker] });
> +
> me.callParent();
> }
> });
> @@ -157,9 +216,10 @@ Ext.define('PVE.qemu.BootOrderEdit', {
> }],
>
> subject: gettext('Boot Order'),
> + width: 600,
>
> initComponent : function() {
> - var me = this;
> + let me = this;
> me.callParent();
> me.load({
> success: function(response, options) {
> diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
> index 20f6ffbb..490d2f54 100644
> --- a/www/manager6/qemu/Options.js
> +++ b/www/manager6/qemu/Options.js
> @@ -86,12 +86,30 @@ Ext.define('PVE.qemu.Options', {
> bootdisk: {
> visible: false
> },
> + bootorder: {
> + visible: false
> + },
> boot: {
> header: gettext('Boot Order'),
> defaultValue: 'cdn',
> editor: caps.vms['VM.Config.Disk'] ? 'PVE.qemu.BootOrderEdit' : undefined,
> - multiKey: ['boot', 'bootdisk'],
> + multiKey: ['boot', 'bootdisk', 'bootorder'],
> renderer: function(order, metaData, record, rowIndex, colIndex, store, pending) {
> + let bootorder = me.getObjectValue('bootorder', undefined, pending);
> + if (bootorder) {
> + let list = bootorder.split(',');
> + let ret = '';
> + list.forEach(dev => {
> + if (ret !== '') {
> + ret += ' > ';
> + }
> + let desc = dev;
> + ret += desc;
> + });
> + return ret;
> + }
> +
> + // legacy style and fallback
> var i;
> var text = '';
> var bootdisk = me.getObjectValue('bootdisk', undefined, pending);
> @@ -99,20 +117,20 @@ Ext.define('PVE.qemu.Options', {
> for (i = 0; i < order.length; i++) {
> var sel = order.substring(i, i + 1);
> if (text) {
> - text += ', ';
> + text += ' > ';
> }
> if (sel === 'c') {
> if (bootdisk) {
> - text += "Disk '" + bootdisk + "'";
> + text += bootdisk;
> } else {
> - text += "Disk";
> + text += "disk";
> }
> } else if (sel === 'n') {
> - text += 'Network';
> + text += 'any net';
> } else if (sel === 'a') {
> - text += 'Floppy';
> + text += 'floppy';
> } else if (sel === 'd') {
> - text += 'CD-ROM';
> + text += 'any cdrom';
> } else {
> text += sel;
> }
>
More information about the pve-devel
mailing list