[pve-devel] [PATCH-SERIES qemu-server/manager v2 0/8] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag

[pve-devel] [PATCH-SERIES qemu-server/manager v2 0/8] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag

[Fiona Ebner]

Changes in v2 (thanks Dano and Thomas!):
* Pass running CPU configuration when using 'nested-virt'. This
  ensures that migration fails early if the flag resolves differently
  on the target.
* Describe that live migration still only works if it's the same flag.
* Drop adding non-existing link in API end point.
* Keep $supported_cpu_flags private to module and add getter method.
* Unpack @_ first at the beginning of resolve_cpu_flags().
* ui: fix function call in the CPU flag selector widget.
* ui: use simpler method to get all records of the store.
* Drop already applied patches.

The flag will automatically resolve to the flag required for the
current CPU on the host. The 'nested-virt' flag takes precedence over
the CPU-specific flag for nesting which might already be present with
custom CPU models. In that case, a warning is printed.

Also makes sure that the actual resolved flag is passed along during
live migration, since the nested-virt flag can resolve differently on
different hosts.

Moves the cpu flag descriptions to qemu-server and introduces an API
endpoint for querying. Could later be extended with further
information, for example, which flags are actually supported on the
current host (some are specific to vendor or CPU).


Dependency and build-dependency bump pve-manger -> qemu-server needed!


qemu-server:

Fiona Ebner (4):
  critic: code style: cpu config: unpack @_ first in resolve_cpu_flags()
  api: add endpoint for querying available cpu flags
  cpu config: introduce is_abstracted() helper
  cpu config: introduce vendor-agnostic 'nested-virt' CPU flag

 src/PVE/API2/Qemu/CPUFlags.pm   |  44 +++++++++
 src/PVE/API2/Qemu/Makefile      |   2 +-
 src/PVE/QemuMigrate.pm          |  11 +--
 src/PVE/QemuServer/CPUConfig.pm | 160 ++++++++++++++++++++++++++++----
 4 files changed, 188 insertions(+), 29 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/CPUFlags.pm


pve-manager:

Fiona Ebner (4):
  api: capabilities: register module for VM CPU flags
  ui: cpu flag selector: code style: use 'let' for declarations
  ui: cpu flag selector: use simpler method to get all records of the
    store
  ui: cpu flag selector: query CPU flag list via API

 PVE/API2/Capabilities.pm               |   6 ++
 www/manager6/form/VMCPUFlagSelector.js | 129 +++++++++++--------------
 2 files changed, 62 insertions(+), 73 deletions(-)


Summary over all repositories:
  6 files changed, 250 insertions(+), 102 deletions(-)

-- 
Generated by git-murpp 0.5.0


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

[pve-devel] [PATCH qemu-server v2 1/8] critic: code style: cpu config: unpack @_ first in resolve_cpu_flags()

[Fiona Ebner]

Also in preparation to add more code at the beginning of the function,
which would reduce readability further without this style fix.

Suggested-by: Daniel Kral <d.kral@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/QemuServer/CPUConfig.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index e72bdf2f..6d5a4237 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -534,9 +534,11 @@ sub print_cpu_device {
 #     ...
 # }
 sub resolve_cpu_flags {
+    my @flag_hashes = @_;
+
     my $flags = {};
 
-    for my $hash (@_) {
+    for my $hash (@flag_hashes) {
         for my $flag_name (keys %$hash) {
             my $flag = $hash->{$flag_name};
             my $old_flag = $flags->{$flag_name};
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server v2 2/8] api: add endpoint for querying available cpu flags

[Fiona Ebner]

Descriptions and ordering are taken from pve-manager's
VMCPUFlagSelector.js. The double quotes in the descriptions were
replaced with single quotes to have nicer JSON output.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Drop adding non-existing link in API end point.
* Keep $supported_cpu_flags private to module and add getter method.

 src/PVE/API2/Qemu/CPUFlags.pm   | 44 ++++++++++++++++++
 src/PVE/API2/Qemu/Makefile      |  2 +-
 src/PVE/QemuServer/CPUConfig.pm | 81 ++++++++++++++++++++++++++-------
 3 files changed, 109 insertions(+), 18 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/CPUFlags.pm

diff --git a/src/PVE/API2/Qemu/CPUFlags.pm b/src/PVE/API2/Qemu/CPUFlags.pm
new file mode 100644
index 00000000..6201654f
--- /dev/null
+++ b/src/PVE/API2/Qemu/CPUFlags.pm
@@ -0,0 +1,44 @@
+package PVE::API2::Qemu::CPUFlags;
+
+use v5.36;
+
+use PVE::RESTHandler;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::QemuServer::CPUConfig;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => 'List of available VM-specific CPU flags.',
+    permissions => { user => 'all' },
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+        },
+    },
+    returns => {
+        type => 'array',
+        items => {
+            type => 'object',
+            properties => {
+                name => {
+                    type => 'string',
+                    description => "Name of the CPU flag.",
+                },
+                description => {
+                    type => 'string',
+                    description => "Description of the CPU flag.",
+                },
+            },
+        },
+    },
+    code => sub {
+        return PVE::QemuServer::CPUConfig::get_supported_cpu_flags();
+    },
+});
+
+1;
diff --git a/src/PVE/API2/Qemu/Makefile b/src/PVE/API2/Qemu/Makefile
index 7c539702..c348af75 100644
--- a/src/PVE/API2/Qemu/Makefile
+++ b/src/PVE/API2/Qemu/Makefile
@@ -2,7 +2,7 @@ DESTDIR=
 PREFIX=/usr
 PERLDIR=$(PREFIX)/share/perl5
 
-SOURCES=Agent.pm CPU.pm HMPPerms.pm Machine.pm
+SOURCES=Agent.pm CPU.pm CPUFlags.pm HMPPerms.pm Machine.pm
 
 .PHONY: install
 install:
diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 6d5a4237..ebec42d5 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -161,21 +161,67 @@ my $cpu_vendor_list = {
     max => 'default',
 };
 
-my @supported_cpu_flags = (
-    'pcid',
-    'spec-ctrl',
-    'ibpb',
-    'ssbd',
-    'virt-ssbd',
-    'amd-ssbd',
-    'amd-no-ssb',
-    'pdpe1gb',
-    'md-clear',
-    'hv-tlbflush',
-    'hv-evmcs',
-    'aes',
-);
-my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags)]})/;
+my $supported_cpu_flags = [
+    {
+        name => 'md-clear',
+        description => "Required to let the guest OS know if MDS is mitigated correctly.",
+    },
+    {
+        name => 'pcid',
+        description =>
+            "Meltdown fix cost reduction on Westmere, Sandy-, and IvyBridge Intel CPUs.",
+    },
+    {
+        name => 'spec-ctrl',
+        description => "Allows improved Spectre mitigation with Intel CPUs.",
+    },
+    {
+        name => 'ssbd',
+        description => "Protection for 'Speculative Store Bypass' for Intel models.",
+    },
+    {
+        name => 'ibpb',
+        description => "Allows improved Spectre mitigation with AMD CPUs.",
+    },
+    {
+        name => 'virt-ssbd',
+        description => "Basis for 'Speculative Store Bypass' protection for AMD models.",
+    },
+    {
+        name => 'amd-ssbd',
+        description => "Improves Spectre mitigation performance with AMD CPUs, best used with"
+            . " 'virt-ssbd'.",
+    },
+    {
+        name => 'amd-no-ssb',
+        description => "Notifies guest OS that host is not vulnerable for Spectre on AMD CPUs.",
+    },
+    {
+        name => 'pdpe1gb',
+        description => "Allow guest OS to use 1GB size pages, if host HW supports it.",
+    },
+    {
+        name => 'hv-tlbflush',
+        description => "Improve performance in overcommitted Windows guests. May lead to guest"
+            . " bluescreens on old CPUs.",
+    },
+    {
+        name => 'hv-evmcs',
+        description => "Improve performance for nested virtualization. Only supported on Intel"
+            . " CPUs.",
+    },
+    {
+        name => 'aes',
+        description => "Activate AES instruction set for HW acceleration.",
+    },
+];
+
+sub get_supported_cpu_flags {
+    return $supported_cpu_flags;
+}
+
+my @supported_cpu_flags_names = map { $_->{name} } $supported_cpu_flags->@*;
+my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags_names)]})/;
 my $cpu_flag_any_re = qr/([+-])([a-zA-Z0-9\-_\.]+)/;
 
 our $qemu_cmdline_cpu_re = qr/^((?>[+-]?[\w\-\._=]+,?)+)$/;
@@ -217,7 +263,7 @@ my $cpu_fmt = {
         description => "List of additional CPU flags separated by ';'. Use '+FLAG' to enable,"
             . " '-FLAG' to disable a flag. Custom CPU models can specify any flag supported by"
             . " QEMU/KVM, VM-specific flags must be from the following set for security reasons: "
-            . join(', ', @supported_cpu_flags),
+            . join(', ', @supported_cpu_flags_names),
         format_description => '+FLAG[;-FLAG...]',
         type => 'string',
         pattern => qr/$cpu_flag_any_re(;$cpu_flag_any_re)*/,
@@ -333,7 +379,8 @@ sub validate_vm_cpu_conf {
     # in a VM-specific config, certain properties are limited/forbidden
 
     if ($cpu->{flags} && $cpu->{flags} !~ m/^$cpu_flag_supported_re(;$cpu_flag_supported_re)*$/) {
-        die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n";
+        die "VM-specific CPU flags must be a subset of: "
+            . join(', ', @supported_cpu_flags_names) . "\n";
     }
 
     if (defined($cpu->{'reported-model'})) {
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server v2 3/8] cpu config: introduce is_abstracted() helper

[Fiona Ebner]

The helper reports whether the CPU configuration cannot be determined
from the configuration alone. Currently, this means using a custom
model, because that depends on the contents of the custom model
configuration. The next commit will add an abstract flag that depends
on the host.

Note that parse_property_string() returns a hash reference or fails,
so there is no need to check its return value for evaluating to true
in boolean context. Even if the check were necessary, having no result
should be treated as a failure, not quietly ignored.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/QemuMigrate.pm          | 11 +++--------
 src/PVE/QemuServer/CPUConfig.pm | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 78954c20..063fd6f2 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -226,14 +226,9 @@ sub prepare {
 
         $self->{forcemachine} = PVE::QemuServer::Machine::qemu_machine_pxe($vmid, $conf);
 
-        # To support custom CPU types, we keep QEMU's "-cpu" parameter intact.
-        # Since the parameter itself contains no reference to a custom model,
-        # this makes migration independent of changes to "cpu-models.conf".
-        if ($conf->{cpu}) {
-            my $cpuconf = PVE::JSONSchema::parse_property_string('pve-cpu-conf', $conf->{cpu});
-            if ($cpuconf && PVE::QemuServer::CPUConfig::is_custom_model($cpuconf->{cputype})) {
-                $self->{forcecpu} = PVE::QemuServer::CPUConfig::get_cpu_from_running_vm($pid);
-            }
+        # To support abstracted CPU configurations, keep QEMU's "-cpu" parameter intact.
+        if ($conf->{cpu} && PVE::QemuServer::CPUConfig::is_abstracted($conf->{cpu})) {
+            $self->{forcecpu} = PVE::QemuServer::CPUConfig::get_cpu_from_running_vm($pid);
         }
 
         # Do not treat a suspended VM as paused, as it might wake up
diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index ebec42d5..4cbd7b48 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -567,6 +567,31 @@ sub print_cpu_device {
     return "$cpu-x86_64-cpu,id=cpu$id,socket-id=$current_socket,core-id=$current_core,thread-id=0";
 }
 
+=head3 is_abstracted
+
+    if (is_abstracted($conf->{cpu})) {
+        # better check running CPU of QEMU instance
+    }
+
+Check if the configured CPU is abstracted in any way, meaning that the actual CPU model cannot be
+determined from the configuration alone. Possible abstractions:
+
+=over
+
+=item custom model: can change with updates to the custom model configuration
+
+=back
+
+=cut
+
+sub is_abstracted {
+    my ($cpu_property_string) = @_;
+
+    my $cpu_conf = PVE::JSONSchema::parse_property_string('pve-cpu-conf', $cpu_property_string);
+
+    return is_custom_model($cpu_conf->{cputype});
+}
+
 # Resolves multiple arrays of hashes representing CPU flags with metadata to a
 # single string in QEMU "-cpu" compatible format. Later arrays have higher
 # priority.
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server v2 4/8] cpu config: introduce vendor-agnostic 'nested-virt' CPU flag

[Fiona Ebner]

The flag will automatically resolve to the flag required for the
current CPU on the host. The 'nested-virt' flag takes precendence over
the CPU-specific flag for nesting which might already be present with
custom CPU models. In that case, a warning is printed.

Also make sure that the actual resolved flag is passed along during
live migration, since the nested-virt flag can resolve differently on
different hosts.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Pass running CPU configuration when using 'nested-virt'. This
  ensures that migration fails early if the flag resolves differently
  on the target.
* Describe that live migration still only works if it's the same flag.

I was thinking about using special characters in the flag name or some
custom-/pve-/special- prefix to distinguish from regular flags, but
decided against it in the end, because I got the gut feeling it might
cause more confusion than it helps. Users who are interested in
details will hopefully read the description and for others, having the
flag name be direct and descriptive is better.

 src/PVE/QemuServer/CPUConfig.pm | 52 +++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 4cbd7b48..8daafd10 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -7,7 +7,10 @@ use JSON;
 
 use PVE::JSONSchema;
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
+use PVE::ProcFSTools;
+use PVE::RESTEnvironment qw(log_warn);
 use PVE::Tools qw(run_command get_host_arch);
+
 use PVE::QemuServer::Helpers qw(min_version);
 
 use base qw(PVE::SectionConfig Exporter);
@@ -162,6 +165,11 @@ my $cpu_vendor_list = {
 };
 
 my $supported_cpu_flags = [
+    {
+        name => 'nested-virt',
+        description => "Controls nested virtualization, namely 'svm' for AMD CPUs and 'vmx' for"
+            . " Intel CPUs. Live migration still only works if it's the same flag on both sides.",
+    },
     {
         name => 'md-clear',
         description => "Required to let the guest OS know if MDS is mitigated correctly.",
@@ -261,8 +269,10 @@ my $cpu_fmt = {
     },
     flags => {
         description => "List of additional CPU flags separated by ';'. Use '+FLAG' to enable,"
-            . " '-FLAG' to disable a flag. Custom CPU models can specify any flag supported by"
-            . " QEMU/KVM, VM-specific flags must be from the following set for security reasons: "
+            . " '-FLAG' to disable a flag. There is a special 'nested-virt' shorthand which"
+            . " controls nested virtualization for the current CPU ('svm' for AMD and 'vmx' for"
+            . " Intel). Custom CPU models can specify any flag supported by QEMU/KVM, VM-specific"
+            . " flags must be from the following set for security reasons: "
             . join(', ', @supported_cpu_flags_names),
         format_description => '+FLAG[;-FLAG...]',
         type => 'string',
@@ -580,6 +590,8 @@ determined from the configuration alone. Possible abstractions:
 
 =item custom model: can change with updates to the custom model configuration
 
+=item abstract flag: for example, nested-virt changes with the host
+
 =back
 
 =cut
@@ -589,7 +601,15 @@ sub is_abstracted {
 
     my $cpu_conf = PVE::JSONSchema::parse_property_string('pve-cpu-conf', $cpu_property_string);
 
-    return is_custom_model($cpu_conf->{cputype});
+    return 1 if is_custom_model($cpu_conf->{cputype});
+
+    return if !$cpu_conf->{flags};
+    for my $flag (split(";", $cpu_conf->{flags})) {
+        if ($flag =~ $cpu_flag_supported_re) {
+            return 1 if $2 eq 'nested-virt';
+        }
+    }
+    return;
 }
 
 # Resolves multiple arrays of hashes representing CPU flags with metadata to a
@@ -610,8 +630,34 @@ sub resolve_cpu_flags {
 
     my $flags = {};
 
+    my $nested_flag;
+    my $nested_flag_resolved;
+    my $resolve_nested_flag = sub {
+        if (!$nested_flag_resolved) {
+            my $host_cpu_flags = PVE::ProcFSTools::read_cpuinfo()->{flags};
+            if ($host_cpu_flags =~ m/\s(svm|vmx)\s/) {
+                $nested_flag = $1;
+            } else {
+                log_warn("ignoring 'nested-virt' CPU flag - unable to resolve from host CPU flags");
+            }
+            $nested_flag_resolved = 1;
+        }
+        return $nested_flag;
+    };
+
     for my $hash (@flag_hashes) {
         for my $flag_name (keys %$hash) {
+            if ($flag_name eq 'nested-virt') {
+                my $nested_flag_name = $resolve_nested_flag->() or next;
+                if ($hash->{$nested_flag_name}) {
+                    warn "warning: CPU flag '$flag_name' overrides '$nested_flag_name'\n";
+                } else {
+                    print "CPU flag '$flag_name' resolved to '$nested_flag_name'\n";
+                }
+                $hash->{$nested_flag_name} = delete($hash->{$flag_name});
+                $flag_name = $nested_flag_name;
+            }
+
             my $flag = $hash->{$flag_name};
             my $old_flag = $flags->{$flag_name};
 
-- 
2.47.3



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

[pve-devel] [PATCH manager v2 5/8] api: capabilities: register module for VM CPU flags

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Depends and build-depends on new qemu-server!

No changes in v2.

 PVE/API2/Capabilities.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/API2/Capabilities.pm b/PVE/API2/Capabilities.pm
index d45c548e..a027142b 100644
--- a/PVE/API2/Capabilities.pm
+++ b/PVE/API2/Capabilities.pm
@@ -7,6 +7,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
 
 use PVE::API2::Qemu::CPU;
+use PVE::API2::Qemu::CPUFlags;
 use PVE::API2::Qemu::Machine;
 use PVE::API2::NodeCapabilities::Qemu::Migration;
 
@@ -17,6 +18,11 @@ __PACKAGE__->register_method({
     path => 'qemu/cpu',
 });
 
+__PACKAGE__->register_method({
+    subclass => "PVE::API2::Qemu::CPUFlags",
+    path => 'qemu/cpu-flags',
+});
+
 __PACKAGE__->register_method({
     subclass => "PVE::API2::Qemu::Machine",
     path => 'qemu/machines',
-- 
2.47.3



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

[pve-devel] [PATCH manager v2 6/8] ui: cpu flag selector: code style: use 'let' for declarations

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v2.

 www/manager6/form/VMCPUFlagSelector.js | 28 +++++++++++++-------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 874068cf..2dbc21dc 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -66,13 +66,13 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     },
 
     getValue: function () {
-        var me = this;
-        var store = me.getStore();
-        var flags = '';
+        let me = this;
+        let store = me.getStore();
+        let flags = '';
 
         // ExtJS does not has a nice getAllRecords interface for stores :/
         store.queryBy(Ext.returnTrue).each(function (rec) {
-            var s = rec.get('state');
+            let s = rec.get('state');
             if (s && s !== '=') {
                 let f = rec.get('flag');
                 if (flags === '') {
@@ -89,8 +89,8 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     },
 
     setValue: function (value) {
-        var me = this;
-        var store = me.getStore();
+        let me = this;
+        let store = me.getStore();
 
         me.value = value || '';
 
@@ -102,12 +102,12 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
                 rec.set('state', '=');
             });
 
-        var flags = value ? value.split(';') : [];
+        let flags = value ? value.split(';') : [];
         flags.forEach(function (flag) {
-            var sign = flag.substr(0, 1);
+            let sign = flag.substr(0, 1);
             flag = flag.substr(1);
 
-            var rec = store.findRecord('flag', flag, 0, false, true, true);
+            let rec = store.findRecord('flag', flag, 0, false, true, true);
             if (rec !== null) {
                 rec.set('state', sign);
             } else {
@@ -116,7 +116,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
         });
         store.reload();
 
-        var res = me.mixins.field.setValue.call(me, value);
+        let res = me.mixins.field.setValue.call(me, value);
 
         return res;
     },
@@ -142,7 +142,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             dataIndex: 'state',
             width: 95,
             onWidgetAttach: function (column, widget, record) {
-                var val = record.get('state') || '=';
+                let val = record.get('state') || '=';
                 widget.down('[inputValue=' + val + ']').setValue(true);
                 // TODO: disable if selected CPU model and flag are incompatible
             },
@@ -154,10 +154,10 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
                 value: '=',
                 listeners: {
                     change: function (f, value) {
-                        var v = Object.values(value)[0];
+                        let v = Object.values(value)[0];
                         f.getWidgetRecord().set('state', v);
 
-                        var view = this.up('grid');
+                        let view = this.up('grid');
                         view.dirty = view.getValue() !== view.originalValue;
                         view.checkDirty();
                         //view.checkChange();
@@ -195,7 +195,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     ],
 
     initComponent: function () {
-        var me = this;
+        let me = this;
 
         // static class store, thus gets not recreated, so ensure defaults are set!
         me.getStore().data.forEach(function (v) {
-- 
2.47.3



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

[pve-devel] [PATCH manager v2 7/8] ui: cpu flag selector: use simpler method to get all records of the store

[Fiona Ebner]

The Ext.data.Store class has a getData() method for getting all
records, which is simpler than doing queryBy(Ext.returnTrue).

Suggested-by: Daniel Kral <d.kral@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 www/manager6/form/VMCPUFlagSelector.js | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 2dbc21dc..630f79f5 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -70,8 +70,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
         let store = me.getStore();
         let flags = '';
 
-        // ExtJS does not has a nice getAllRecords interface for stores :/
-        store.queryBy(Ext.returnTrue).each(function (rec) {
+        store.getData().each(function (rec) {
             let s = rec.get('state');
             if (s && s !== '=') {
                 let f = rec.get('flag');
@@ -96,11 +95,7 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
 
         me.unkownFlags = [];
 
-        me.getStore()
-            .queryBy(Ext.returnTrue)
-            .each(function (rec) {
-                rec.set('state', '=');
-            });
+        me.getStore().getData().each((rec) => rec.set('state', '='));
 
         let flags = value ? value.split(';') : [];
         flags.forEach(function (flag) {
-- 
2.47.3



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

[pve-devel] [PATCH manager v2 8/8] ui: cpu flag selector: query CPU flag list via API

[Fiona Ebner]

There is coupling between the store and value for the top-level grid,
which has a field mixin. Because of this, the setValue() method needs
to update the 'state' value for the records in the store accordingly.
This can however only be done if the store is already loaded, so the
store gets a refresh() callback. It also remembers which value it was
already adjusted for for potential subsequent setValue() calls.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* ui: fix function name adjustStoreForValue() for caller in setValue()
* Fix typo in commit message 'Becuase' -> 'Because'.

 www/manager6/form/VMCPUFlagSelector.js | 96 +++++++++++---------------
 1 file changed, 42 insertions(+), 54 deletions(-)

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 630f79f5..922fcfa6 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -18,62 +18,42 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
 
     store: {
         type: 'store',
-        fields: ['flag', { name: 'state', defaultValue: '=' }, 'desc'],
-        data: [
-            // FIXME: let qemu-server host this and autogenerate or get from API call??
-            {
-                flag: 'md-clear',
-                desc: 'Required to let the guest OS know if MDS is mitigated correctly',
-            },
-            {
-                flag: 'pcid',
-                desc: 'Meltdown fix cost reduction on Westmere, Sandy-, and IvyBridge Intel CPUs',
-            },
-            { flag: 'spec-ctrl', desc: 'Allows improved Spectre mitigation with Intel CPUs' },
-            { flag: 'ssbd', desc: 'Protection for "Speculative Store Bypass" for Intel models' },
-            { flag: 'ibpb', desc: 'Allows improved Spectre mitigation with AMD CPUs' },
-            {
-                flag: 'virt-ssbd',
-                desc: 'Basis for "Speculative Store Bypass" protection for AMD models',
-            },
-            {
-                flag: 'amd-ssbd',
-                desc: 'Improves Spectre mitigation performance with AMD CPUs, best used with "virt-ssbd"',
-            },
-            {
-                flag: 'amd-no-ssb',
-                desc: 'Notifies guest OS that host is not vulnerable for Spectre on AMD CPUs',
-            },
-            {
-                flag: 'pdpe1gb',
-                desc: 'Allow guest OS to use 1GB size pages, if host HW supports it',
-            },
-            {
-                flag: 'hv-tlbflush',
-                desc: 'Improve performance in overcommitted Windows guests. May lead to guest bluescreens on old CPUs.',
-            },
-            {
-                flag: 'hv-evmcs',
-                desc: 'Improve performance for nested virtualization. Only supported on Intel CPUs.',
-            },
-            { flag: 'aes', desc: 'Activate AES instruction set for HW acceleration.' },
-        ],
+        fields: ['name', { name: 'state', defaultValue: '=' }, 'description'],
+        autoLoad: true,
+        proxy: {
+            type: 'proxmox',
+            url: '/api2/json/nodes/localhost/capabilities/qemu/cpu-flags',
+        },
         listeners: {
             update: function () {
                 this.commitChanges();
             },
+            refresh: function (store, eOpts) {
+                let me = this;
+                let view = me.view;
+
+                if (store.adjustedForValue !== view.value) {
+                    view.adjustStoreForValue();
+                }
+            },
         },
+        adjustedForValue: undefined,
     },
 
     getValue: function () {
         let me = this;
         let store = me.getStore();
+
+        if (!store.isLoaded()) {
+            return me.value;
+        }
+
         let flags = '';
 
         store.getData().each(function (rec) {
             let s = rec.get('state');
             if (s && s !== '=') {
-                let f = rec.get('flag');
+                let f = rec.get('name');
                 if (flags === '') {
                     flags = s + f;
                 } else {
@@ -87,29 +67,41 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
         return flags;
     },
 
-    setValue: function (value) {
+    // Adjusts the store for the current value and determines the unkown flags based on what the
+    // store does not know.
+    adjustStoreForValue: function () {
         let me = this;
         let store = me.getStore();
-
-        me.value = value || '';
+        let value = me.value;
 
         me.unkownFlags = [];
 
-        me.getStore().getData().each((rec) => rec.set('state', '='));
+        store.getData().each((rec) => rec.set('state', '='));
 
         let flags = value ? value.split(';') : [];
         flags.forEach(function (flag) {
             let sign = flag.substr(0, 1);
             flag = flag.substr(1);
 
-            let rec = store.findRecord('flag', flag, 0, false, true, true);
+            let rec = store.findRecord('name', flag, 0, false, true, true);
             if (rec !== null) {
                 rec.set('state', sign);
             } else {
                 me.unkownFlags.push(flag);
             }
         });
-        store.reload();
+
+        store.adjustedForValue = value;
+    },
+
+    setValue: function (value) {
+        let me = this;
+
+        me.value = value || '';
+
+        if (me.getStore().isLoaded()) {
+            me.adjustStoreForValue();
+        } // if not yet loaded, the store will trigger the function
 
         let res = me.mixins.field.setValue.call(me, value);
 
@@ -179,11 +171,11 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             },
         },
         {
-            dataIndex: 'flag',
+            dataIndex: 'name',
             width: 100,
         },
         {
-            dataIndex: 'desc',
+            dataIndex: 'description',
             cellWrap: true,
             flex: 1,
         },
@@ -192,12 +184,8 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
     initComponent: function () {
         let me = this;
 
-        // static class store, thus gets not recreated, so ensure defaults are set!
-        me.getStore().data.forEach(function (v) {
-            v.state = '=';
-        });
-
         me.value = me.originalValue = '';
+        me.store.view = me;
 
         me.callParent(arguments);
     },
-- 
2.47.3



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