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

[pve-devel] [PATCH-SERIES qemu-server/manager 0/7] VM CPU flags: 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.

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):
  cpu config: style fix: avoid multiline post-if expressions
  cpu config: style fix: avoid overly long ternary conditional
    expression
  api: add endpoint for querying available cpu flags
  cpu config: introduce vendor-agnostic 'nested-virt' CPU flag

 src/PVE/API2/Qemu/CPUFlags.pm   |  45 +++++++
 src/PVE/API2/Qemu/Makefile      |   2 +-
 src/PVE/QemuServer/CPUConfig.pm | 222 ++++++++++++++++++++++----------
 3 files changed, 199 insertions(+), 70 deletions(-)
 create mode 100644 src/PVE/API2/Qemu/CPUFlags.pm


pve-manager:

Fiona Ebner (3):
  api: capabilities: register module for VM CPU flags
  ui: cpu flag selector: code style: use 'let' for declarations
  ui: cpu flag selector: query CPU flag list via API

 PVE/API2/Capabilities.pm               |   6 ++
 www/manager6/form/VMCPUFlagSelector.js | 126 +++++++++++--------------
 2 files changed, 61 insertions(+), 71 deletions(-)


Summary over all repositories:
  5 files changed, 260 insertions(+), 141 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 1/7] cpu config: style fix: avoid multiline post-if expressions

[Fiona Ebner]

Should improve readability. No functional change intended.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/CPUConfig.pm | 98 +++++++++++++++++----------------
 1 file changed, 52 insertions(+), 46 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 786a99d8..1fdae415 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -326,19 +326,19 @@ sub validate_vm_cpu_conf {
     if (is_custom_model($cputype)) {
         # dies on unknown model
         get_custom_model($cputype);
-    } else {
-        die "Built-in cputype '$cputype' is not defined (missing 'custom-' prefix?)\n"
-            if !defined($cpu_vendor_list->{$cputype}) && !defined($builtin_models->{$cputype});
+    } elsif (!defined($cpu_vendor_list->{$cputype}) && !defined($builtin_models->{$cputype})) {
+        die "Built-in cputype '$cputype' is not defined (missing 'custom-' prefix?)\n";
     }
 
     # in a VM-specific config, certain properties are limited/forbidden
 
-    die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n"
-        if ($cpu->{flags}
-            && $cpu->{flags} !~ m/^$cpu_flag_supported_re(;$cpu_flag_supported_re)*$/);
+    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 "Property 'reported-model' not allowed in VM-specific CPU config.\n"
-        if defined($cpu->{'reported-model'});
+    if (defined($cpu->{'reported-model'})) {
+        die "Property 'reported-model' not allowed in VM-specific CPU config.\n";
+    }
 
     return $cpu;
 }
@@ -387,13 +387,15 @@ sub write_config {
     for my $model (keys %{ $cfg->{ids} }) {
         my $model_conf = $cfg->{ids}->{$model};
 
-        die
-            "internal error: tried saving built-in CPU model (or missing prefix): $model_conf->{cputype}\n"
-            if !is_custom_model($model_conf->{cputype});
+        if (!is_custom_model($model_conf->{cputype})) {
+            die "internal error: tried saving built-in CPU model (or missing prefix):"
+                . " $model_conf->{cputype}\n";
+        }
 
-        die
-            "internal error: tried saving custom cpumodel with cputype (ignoring prefix: $model_conf->{cputype}) not equal to \$cfg->ids entry ($model)\n"
-            if "custom-$model" ne $model_conf->{cputype};
+        if ("custom-$model" ne $model_conf->{cputype}) {
+            die "internal error: tried saving custom cpumodel with cputype (ignoring prefix:"
+                . " $model_conf->{cputype}) not equal to \$cfg->ids entry ($model)\n";
+        }
 
         # saved in section header
         delete $model_conf->{cputype};
@@ -654,23 +656,23 @@ sub get_cpu_options {
     my $vm_flags = parse_cpuflag_list($cpu_flag_supported_re, "manually set for VM", $cpu->{flags});
 
     my $pve_forced_flags = {};
-    $pve_forced_flags->{'enforce'} = {
-        reason => "error if requested CPU settings not available",
-        }
-        if $cputype ne 'host' && $kvm && $arch eq 'x86_64';
-    $pve_forced_flags->{'kvm'} = {
-        value => "off",
-        reason => "hide KVM virtualization from guest",
-        }
-        if $kvm_off;
+    if ($cputype ne 'host' && $kvm && $arch eq 'x86_64') {
+        $pve_forced_flags->{'enforce'} = {
+            reason => "error if requested CPU settings not available",
+        };
+    }
+    if ($kvm_off) {
+        $pve_forced_flags->{'kvm'} = {
+            value => "off",
+            reason => "hide KVM virtualization from guest",
+        };
+    }
 
     # $cputype is the "reported-model" for custom types, so we can just look up
     # the vendor in the default list
     my $cpu_vendor = $cpu_vendor_list->{$cputype};
     if ($cpu_vendor) {
-        $pve_forced_flags->{'vendor'} = {
-            value => $cpu_vendor,
-        } if $cpu_vendor ne 'default';
+        $pve_forced_flags->{'vendor'} = { value => $cpu_vendor } if $cpu_vendor ne 'default';
     } elsif ($arch ne 'aarch64') {
         die "internal error"; # should not happen
     }
@@ -714,29 +716,33 @@ sub get_pve_cpu_flags {
     my $pve_flags = {};
     my $pve_msg = "set by PVE;";
 
-    $pve_flags->{'lahf_lm'} = {
-        op => '+',
-        reason => "$pve_msg to support Windows 8.1+",
-        }
-        if $cputype eq 'kvm64' && $arch eq 'x86_64';
+    if ($cputype eq 'kvm64' && $arch eq 'x86_64') {
+        $pve_flags->{'lahf_lm'} = {
+            op => '+',
+            reason => "$pve_msg to support Windows 8.1+",
+        };
+    }
 
-    $pve_flags->{'x2apic'} = {
-        op => '-',
-        reason => "$pve_msg incompatible with Solaris",
-        }
-        if $conf->{ostype} && $conf->{ostype} eq 'solaris';
+    if ($conf->{ostype} && $conf->{ostype} eq 'solaris') {
+        $pve_flags->{'x2apic'} = {
+            op => '-',
+            reason => "$pve_msg incompatible with Solaris",
+        };
+    }
 
-    $pve_flags->{'sep'} = {
-        op => '+',
-        reason => "$pve_msg to support Windows 8+ and improve Windows XP+",
-        }
-        if $cputype eq 'kvm64' || $cputype eq 'kvm32';
+    if ($cputype eq 'kvm64' || $cputype eq 'kvm32') {
+        $pve_flags->{'sep'} = {
+            op => '+',
+            reason => "$pve_msg to support Windows 8+ and improve Windows XP+",
+        };
+    }
 
-    $pve_flags->{'rdtscp'} = {
-        op => '-',
-        reason => "$pve_msg broken on AMD Opteron",
-        }
-        if $cputype =~ m/^Opteron/;
+    if ($cputype =~ m/^Opteron/) {
+        $pve_flags->{'rdtscp'} = {
+            op => '-',
+            reason => "$pve_msg broken on AMD Opteron",
+        };
+    }
 
     if (min_version($machine_version, 2, 3) && $kvm && $arch eq 'x86_64') {
         $pve_flags->{'kvm_pv_unhalt'} = {
-- 
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 2/7] cpu config: style fix: avoid overly long ternary conditional expression

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/CPUConfig.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 1fdae415..e72bdf2f 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -636,16 +636,16 @@ sub get_cpu_options {
 
     my $pve_flags = get_pve_cpu_flags($conf, $kvm, $cputype, $arch, $machine_version);
 
-    my $hv_flags =
-        $kvm
-        ? get_hyperv_enlightenments(
+    my $hv_flags;
+    if ($kvm) {
+        $hv_flags = get_hyperv_enlightenments(
             $winversion,
             $machine_version,
             $conf->{bios},
             $gpu_passthrough,
             $hv_vendor_id,
-        )
-        : undef;
+        );
+    }
 
     my $builtin_cputype_flags =
         parse_cpuflag_list($cpu_flag_any_re, "set by builtin CPU model", $builtin_cpu->{flags});
-- 
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 3/7] 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>
---
 src/PVE/API2/Qemu/CPUFlags.pm   | 45 +++++++++++++++++++
 src/PVE/API2/Qemu/Makefile      |  2 +-
 src/PVE/QemuServer/CPUConfig.pm | 76 +++++++++++++++++++++++++--------
 3 files changed, 105 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..cc06a1d6
--- /dev/null
+++ b/src/PVE/API2/Qemu/CPUFlags.pm
@@ -0,0 +1,45 @@
+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.",
+                },
+            },
+        },
+        links => [{ rel => 'child', href => '{flag}' }],
+    },
+    code => sub {
+        return $PVE::QemuServer::CPUConfig::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 e72bdf2f..20e26ee2 100644
--- a/src/PVE/QemuServer/CPUConfig.pm
+++ b/src/PVE/QemuServer/CPUConfig.pm
@@ -161,21 +161,62 @@ 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)]})/;
+our $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.",
+    },
+];
+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 +258,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 +374,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 4/7] 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.

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

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 | 40 +++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
index 20e26ee2..dc8929c9 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 = {
 };
 
 our $supported_cpu_flags = [
+    {
+        name => 'nested-virt',
+        description => "Controls nested virtualization, namely 'svm' for AMD CPUs and 'vmx' for"
+            . " Intel CPUs.",
+    },
     {
         name => 'md-clear',
         description => "Required to let the guest OS know if MDS is mitigated correctly.",
@@ -256,8 +264,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',
@@ -578,8 +588,34 @@ sub print_cpu_device {
 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 (@_) {
         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 5/7] 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!

 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 6/7] ui: cpu flag selector: code style: use 'let' for declarations

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 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 7/7] 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. Becuase 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>
---

Happy to hear suggestions if this is too involved already! The grid
itself is a field (via mixin) and the 'state' values in the store are
coupled with the field value, so I couldn't figure out a simpler way
yet.

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

diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js
index 2dbc21dc..1bd5a60f 100644
--- a/www/manager6/form/VMCPUFlagSelector.js
+++ b/www/manager6/form/VMCPUFlagSelector.js
@@ -18,63 +18,43 @@ 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 = '';
 
         // ExtJS does not has a nice getAllRecords interface for stores :/
         store.queryBy(Ext.returnTrue).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 {
@@ -88,33 +68,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()
-            .queryBy(Ext.returnTrue)
-            .each(function (rec) {
-                rec.set('state', '=');
-            });
+        store.queryBy(Ext.returnTrue).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.adjustForValue();
+        } // if not yet loaded, the store will trigger the function
 
         let res = me.mixins.field.setValue.call(me, value);
 
@@ -184,11 +172,11 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
             },
         },
         {
-            dataIndex: 'flag',
+            dataIndex: 'name',
             width: 100,
         },
         {
-            dataIndex: 'desc',
+            dataIndex: 'description',
             cellWrap: true,
             flex: 1,
         },
@@ -197,12 +185,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