From: Fiona Ebner <f.ebner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Markus Frank <m.frank@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager v5 5/5] ui: MachineEdit with viommu checkbox
Date: Thu, 17 Aug 2023 15:59:40 +0200 [thread overview]
Message-ID: <0eecbb04-41a3-ed35-e263-fc85386cee10@proxmox.com> (raw)
In-Reply-To: <20230118135800.131382-6-m.frank@proxmox.com>
Am 18.01.23 um 14:58 schrieb Markus Frank:
> @@ -38,14 +54,27 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> if (values.version && values.version !== 'latest') {
> values.machine = values.version;
> delete values.delete;
> + } else if (values.machine === undefined && values.viommu) {
> + // set machine to pc to raise the viommu + i440fx error
> + // from qemu-server instead of a regex error
> + values.machine = "pc";
> + delete values.delete;
> }
> delete values.version;
> + if (values.viommu) {
> + values.machine += ",viommu=1";
> + }
Nit: could use PVE.Parser.printPropertyString but no big deal
> + delete values.viommu;
> + delete values.kvm;
> return values;
> },
>
> setValues: function(values) {
> let me = this;
>
> + let machineConf = PVE.Parser.parsePropertyString(values.machine, "type");
> + values.machine = machineConf.type;
> +
> me.isWindows = values.isWindows;
> if (values.machine === 'pc') {
> values.machine = '__default__';
> @@ -58,6 +87,11 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> values.version = 'pc-q35-5.1';
> }
> }
> +
> + me.lookup('kvm').setValue(values.kvm);
> + values.viommu = machineConf.viommu === '1';
Should be PVE.Parser.parseBoolean to cover all cases (i.e. also 'on',
'yes' and 'true' can be in the config)
> + me.lookup('viommu').setValue(values.viommu);
> +
> if (values.machine !== '__default__' && values.machine !== 'q35') {
> values.version = values.machine;
> values.machine = values.version.match(/q35/) ? 'q35' : '__default__';
> @@ -113,6 +147,26 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> fieldLabel: gettext('Note'),
> value: gettext('Machine version change may affect hardware layout and settings in the guest OS.'),
> },
> + {
> + xtype: 'proxmoxcheckbox',
> + fieldLabel: gettext('vIOMMU'),
> + name: 'viommu',
> + reference: 'viommu',
> + },
> + {
> + xtype: 'proxmoxcheckbox',
> + name: 'kvm',
> + reference: 'kvm',
> + hidden: true,
> + },
Why handle 'kvm' as an UI element? I guess a view model property would
be the natural place for this.
> + {
> + xtype: 'displayfield',
> + name: 'kvmQ35Hint',
> + reference: 'kvmQ35Hint',
> + userCls: 'pmx-hint',
> + value: gettext('vIOMMU needs kvm enabled and q35 firmware'),
s/firmware/machine type/
> + hidden: true,
> + },
> ],
> });
>
> @@ -135,8 +189,12 @@ Ext.define('PVE.qemu.MachineEdit', {
> me.load({
> success: function(response) {
> let conf = response.result.data;
> + if (conf.kvm === undefined || conf.kvm === null) {
> + conf.kvm = 1;
> + }
Style nit: could be one line with conf.kvm ??= 1;
> let values = {
> machine: conf.machine || '__default__',
> + kvm: conf.kvm,
> };
> values.isWindows = PVE.Utils.is_windows(conf.ostype);
> me.setValues(values);
next prev parent reply other threads:[~2023-08-17 13:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-18 13:57 [pve-devel] [PATCH qemu-server/manager/docs v5 0/5] vIOMMU-Feature Markus Frank
2023-01-18 13:57 ` [pve-devel] [PATCH qemu-server v5 1/5] tests: replaced somemachine&someothermachine with q35&pc Markus Frank
2023-08-17 11:43 ` [pve-devel] applied: " Fiona Ebner
2023-01-18 13:57 ` [pve-devel] [PATCH qemu-server v5 2/5] feature #3784: Parameter for guest vIOMMU & machine as property-string Markus Frank
2023-08-17 12:29 ` Fiona Ebner
2023-01-18 13:57 ` [pve-devel] [PATCH qemu-server v5 3/5] added test-cases for new machine-syntax & viommu Markus Frank
2023-01-18 13:57 ` [pve-devel] [PATCH docs v5 4/5] added vIOMMU documentation Markus Frank
2023-08-17 12:41 ` Fiona Ebner
2023-01-18 13:58 ` [pve-devel] [PATCH manager v5 5/5] ui: MachineEdit with viommu checkbox Markus Frank
2023-08-17 13:59 ` Fiona Ebner [this message]
2023-06-19 10:32 ` [pve-devel] [PATCH qemu-server/manager/docs v5 0/5] vIOMMU-Feature Markus Frank
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0eecbb04-41a3-ed35-e263-fc85386cee10@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=m.frank@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox