[pve-devel] [PATCH manager 5/6 v2] gui: add qemu/PCIEdit.js

[Thomas Lamprecht]

On 11/20/18 5:13 PM, Dominik Csapak wrote:
> this patch adds the PCIEdit window and InputPanel
> uses PCISelector and MDevSelector
> 
> when we detect an iommugroup of -1, we put a warning on top
> to inform the user that IOMMU is not activated (but let him add
> the devices regardless, so that he can use it after IOMMU is
> activated)
> 
> also puts a warning if he selects a device that shares an iommugroup
> with a different device (but not the same device with different
> function). that detection is not perfect, but we cannot do really
> better
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  www/manager6/Makefile        |   1 +
>  www/manager6/qemu/PCIEdit.js | 227 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 228 insertions(+)
>  create mode 100644 www/manager6/qemu/PCIEdit.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index d900ec88..d005d714 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -139,6 +139,7 @@ JSSRC= 				                 	\
>  	qemu/Config.js					\
>  	qemu/CreateWizard.js				\
>  	qemu/USBEdit.js					\
> +	qemu/PCIEdit.js					\
>  	qemu/SerialEdit.js					\
>  	qemu/AgentIPView.js				\
>  	qemu/CloudInit.js				\
> diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
> new file mode 100644
> index 00000000..993da7bf
> --- /dev/null
> +++ b/www/manager6/qemu/PCIEdit.js
> @@ -0,0 +1,227 @@
> +Ext.define('PVE.qemu.PCIInputPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +
> +    onlineHelp: 'qm_pci_passthrough',
> +
> +    setVMConfig: function(vmconfig) {
> +	var me = this;
> +	me.vmconfig = vmconfig;
> +
> +	var hostpci = me.vmconfig[me.confid] || '';
> +
> +	var values = PVE.Parser.parsePropertyString(hostpci, 'host');
> +	if (values.host && values.host.length < 6) { // 00:00 format not 00:00.0
> +	    values.host += ".0";
> +	    values.multifunction = true;
> +	}
> +	values['x-vga'] = PVE.Parser.parseBoolean(values['x-vga'], 0);
> +	values.pcie = PVE.Parser.parseBoolean(values.pcie, 0);
> +	values.rombar = PVE.Parser.parseBoolean(values.rombar, 1);
> +
> +	me.setValues(values);
> +	if (!me.vmconfig.machine || me.vmconfig.machine.indexOf('q35') === -1) {
> +	    // machine is not set to some variant of q35, so we disable pcie
> +	    var pcie = me.down('field[name=pcie]');
> +	    pcie.setDisabled(true);
> +	    pcie.setBoxLabel(gettext('Q35 only'));
> +	}
> +    },
> +
> +    onGetValues: function(values) {
> +	var me = this;
> +	var ret = {};
> +	if(!me.confid) {
> +	    var i;
> +	    for (i = 0; i < 5; i++) {
> +		if (!me.vmconfig['hostpci' +  i.toString()]) {
> +		    me.confid = 'hostpci' + i.toString();
> +		    break;
> +		}
> +	    }
> +	}
> +	if (values.multifunction) {
> +	    // modify host to skip the '.X'
> +	    values.host = values.host.substring(0,5);
> +	    delete values.multifunction;
> +	}
> +
> +	if (values.rombar) {
> +	    delete values.rombar;
> +	} else {
> +	    values.rombar = 0;
> +	}
> +
> +	ret[me.confid] = PVE.Parser.printPropertyString(values, 'host');
> +	return ret;
> +    },
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.nodename = me.pveSelNode.data.node;
> +	if (!me.nodename) {
> +	    throw "no node name specified";
> +	}
> +
> +	me.column1 = [
> +	    {
> +		xtype: 'pvePCISelector',
> +		fieldLabel: gettext('Device'),
> +		name: 'host',
> +		nodename: me.nodename,
> +		allowBlank: false,
> +		onLoadCallBack: function(store, records, success) {
> +		    if (!success || !records.length) {
> +			return;
> +		    }
> +
> +		    var first = records[0];
> +		    if (first.data.iommugroup === -1) {
> +			// no iommu groups
> +			var warning = Ext.create('Ext.form.field.Display', {
> +			    columnWidth: 1,
> +			    padding: '0 0 10 0',
> +			    value: 'No IOMMU detected, please activate it.' +
> +				   'See Documentation for further information.',
> +			    userCls: 'pve-hint'
> +			});
> +			me.items.insert(0, warning);
> +			me.updateLayout(); // insert does not trigger that
> +		    }
> +		},
> +		listeners: {
> +		    change: function(pcisel, value) {
> +			if (!value) {
> +			    return;
> +			}
> +			var pcidev = pcisel.getStore().getById(value);
> +			var mdevfield = me.down('field[name=mdev]');
> +			mdevfield.setDisabled(!pcidev || !pcidev.data.mdev);
> +			if (!pcidev) {
> +			    return;
> +			}
> +			var id = pcidev.data.id.substring(0,5); // 00:00
> +			var iommu = pcidev.data.iommugroup;
> +			// try to find out if there are more devices
> +			// in that iommu group
> +			if (iommu !== -1) {
> +			    var count = 0;
> +			    pcisel.getStore().each(function(record) {
> +				if (record.data.iommugroup === iommu &&
> +				    record.data.id.substring(0,5) !== id)
> +				{
> +				    count++;
> +				    return false;
> +				}
> +			    });
> +			    var warning = me.down('#iommuwarning');
> +			    if (count && !warning) {
> +				warning = Ext.create('Ext.form.field.Display', {
> +				    columnWidth: 1,
> +				    padding: '0 0 10 0',
> +				    itemId: 'iommuwarning',
> +				    value: 'The selected Device is not in a seperate' +
> +					   'IOMMU group, make sure this is intended.',
> +				    userCls: 'pve-hint'
> +				});
> +				me.items.insert(0, warning);
> +				me.updateLayout(); // insert does not trigger that
> +			    } else if (!count && warning) {
> +				me.remove(warning);
> +			    }
> +			}
> +			if (pcidev.data.mdev) {
> +			    mdevfield.setPciID(value);
> +			}
> +		    }
> +		}
> +	    },
> +	    {
> +		xtype: 'proxmoxcheckbox',
> +		fieldLabel: gettext('All Functions'),
> +		name: 'multifunction'
> +	    }
> +	];
> +
> +	me.column2 = [
> +	    {
> +		xtype: 'pveMDevSelector',
> +		name: 'mdev',
> +		disabled: true,
> +		fieldLabel: gettext('MDev Type'),
> +		nodename: me.nodename,
> +		listeners: {
> +		    change: function(field, value) {
> +			var mf = me.down('field[name=multifunction]');
> +			if (!!value) {
> +			    mf.setValue(false);
> +			}
> +			mf.setDisabled(!!value);
> +		    }
> +		}
> +	    },
> +	    {
> +		xtype: 'proxmoxcheckbox',
> +		fieldLabel: gettext('Primary GPU'),
> +		name: 'x-vga'
> +	    }
> +	];
> +
> +	me.advancedColumn1 = [
> +	    {
> +		xtype: 'proxmoxcheckbox',
> +		fieldLabel: 'ROM-Bar',
> +		name: 'rombar'
> +	    },
> +	    {
> +		xtype: 'proxmoxtextfield',
> +		fieldLabel: 'ROM-File',
> +		name: 'romfile'
> +	    }

this feels just a bit raw to me, and I do not see the editing point at all.

You need to go to the CLI to copy a ROM there or get it's file path/name anyway.
And nowhere, neither docs nor UI it's hinted that this is a relative path to
'/usr/share/kvm' (at least I did not see anything when searching after rombar)
I'd just remove it for now, or make it read only display (if set at all).

> +	];
> +
> +	me.advancedColumn2 = [
> +	    {
> +		xtype: 'proxmoxcheckbox',
> +		fieldLabel: 'PCI-Express',
> +		name: 'pcie'
> +	    }
> +	];
> +
> +	me.callParent();
> +    }
> +});
> +
> +Ext.define('PVE.qemu.PCIEdit', {
> +    extend: 'Proxmox.window.Edit',
> +
> +    vmconfig: undefined,
> +
> +    isAdd: true,
> +
> +    subject: gettext('PCI Device'),
> +
> +
> +    initComponent : function() {
> +	var me = this;
> +
> +	me.isCreate = !me.confid;
> +
> +	var ipanel = Ext.create('PVE.qemu.PCIInputPanel', {
> +	    confid: me.confid,
> +	    pveSelNode: me.pveSelNode
> +	});
> +
> +	Ext.apply(me, {
> +	    items: [ ipanel ]
> +	});
> +
> +	me.callParent();
> +
> +	me.load({
> +	    success: function(response) {
> +		ipanel.setVMConfig(response.result.data);
> +	    }
> +	});
> +    }
> +});
>