[pve-devel] [PATCH-SERIES v8 qemu-server, common] Prevent starting a 32-bit VM using a 64-bit OVMF BIOS

[pve-devel] [PATCH-SERIES v8 qemu-server, common] Prevent starting a 32-bit VM using a 64-bit OVMF BIOS

[Filip Schauer]

This patch series prevents starting a 32-bit VM using a 64-bit OVMF BIOS
and makes the default value for 'kvm' during CPU hotplug consistent with
the rest of the code. This is a breaking change for VMs with a different
CPU architecture running on an x86_64 host.

Changes since v7:
* Rename is_native to is_native_arch to be more explicit
* Move get_default_cpu_type from Helpers to CPUConfig
* Default to host architecture when no $arch is given to get_cpu_bitness
* die with a message on hotplug of non x86_64 CPUs in print_cpu_device

Changes since v6:
* Skip the CPU bitness check if $forcecpu is set
* Take custom CPU types into account
* Add a helper for getting the default CPU type
* Unify the default value for 'kvm'
* Move is_native from PVE::QemuServer to PVE::Tools

Changes since v5:
* Refactor get_cpu_bitness to die instead of returning undef
* Pass cputype instead of the entire vm config to get_cpu_bitness

Changes since v4:
* Remove parentheses around post-if in get_cpu_bitness

Changes since v3:
* Move the cputypes_32bit list from QemuServer.pm to CPUConfig.pm
* Turn cputypes_32bit into a hash for lookup
* Create a helper get_cpu_bitness function in CPUConfig.pm
* Describe how the list of 32-bit CPU types was obtained

common:

Filip Schauer (1):
  tools: add is_native_arch to compare the CPU architecture

 src/PVE/Tools.pm | 6 ++++++
 1 file changed, 6 insertions(+)

qemu-server:

Filip Schauer (5):
  cpu config: Add helper to get the default CPU type
  Prevent starting a 32-bit VM using a 64-bit OVMF BIOS
  Move is_native from PVE::QemuServer to PVE::Tools
  Unify the default value for 'kvm'
  cpu config: die on hotplug of non x86_64 CPUs

 PVE/QemuServer.pm           | 23 +++++++------
 PVE/QemuServer/CPUConfig.pm | 64 ++++++++++++++++++++++++++++++++-----
 2 files changed, 67 insertions(+), 20 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH common 1/1] tools: add is_native_arch to compare the CPU architecture

[Filip Schauer]

Add an is_native_arch($arch) subroutine to compare a CPU architecture to
the host CPU architecture. This is brought in from PVE::QemuServer.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/Tools.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 766c809..234f16a 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -52,6 +52,7 @@ extract_param
 extract_sensitive_params
 file_copy
 get_host_arch
+is_native_arch
 O_PATH
 O_TMPFILE
 AT_EMPTY_PATH
@@ -1841,6 +1842,11 @@ sub get_host_arch {
     return $host_arch;
 }
 
+sub is_native_arch($) {
+    my ($arch) = @_;
+    return get_host_arch() eq $arch;
+}
+
 # Devices are: [ (12 bits minor) (12 bits major) (8 bits minor) ]
 sub dev_t_major($) {
     my ($dev_t) = @_;
-- 
2.39.2

[pve-devel] [PATCH qemu-server 1/5] cpu config: add helper to get the default CPU type

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 PVE/QemuServer/CPUConfig.pm | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index ca2946b..4be5262 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -405,7 +405,7 @@ sub print_cpu_device {
     my ($conf, $id) = @_;
 
     my $kvm = $conf->{kvm} // 1;
-    my $cpu = $kvm ? "kvm64" : "qemu64";
+    my $cpu = get_default_cpu_type('x86_64', $kvm);
     if (my $cputype = $conf->{cpu}) {
 	my $cpuconf = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cputype)
 	    or die "Cannot parse cpu description: $cputype\n";
@@ -515,10 +515,7 @@ sub parse_cpuflag_list {
 sub get_cpu_options {
     my ($conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough) = @_;
 
-    my $cputype = $kvm ? "kvm64" : "qemu64";
-    if ($arch eq 'aarch64') {
-	$cputype = 'cortex-a57';
-    }
+    my $cputype = get_default_cpu_type($arch, $kvm);
 
     my $cpu = {};
     my $custom_cpu;
@@ -719,6 +716,15 @@ sub get_cpu_from_running_vm {
     return $1;
 }
 
+sub get_default_cpu_type {
+    my ($arch, $kvm) = @_;
+
+    my $cputype = $kvm ? 'kvm64' : 'qemu64';
+    $cputype = 'cortex-a57' if $arch eq 'aarch64';
+
+    return $cputype;
+}
+
 __PACKAGE__->register();
 __PACKAGE__->init();
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server 2/5] prevent starting a 32-bit VM using a 64-bit OVMF BIOS

[Filip Schauer]

Instead of starting a VM with a 32-bit CPU type and a 64-bit OVMF image,
throw an error before starting the VM telling the user that OVMF is not
supported on 32-bit CPU types.

To obtain a list of 32-bit CPU types, refer to the builtin_x86_defs in
target/i386/cpu.c of QEMU. Exclude any entries that have the long mode
feature (CPUID_EXT2_LM).

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 PVE/QemuServer.pm           |  5 ++++-
 PVE/QemuServer/CPUConfig.pm | 39 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b45507a..33c374c 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -52,7 +52,7 @@ use PVE::QemuConfig;
 use PVE::QemuServer::Helpers qw(config_aware_timeout min_version windows_version);
 use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CGroup;
-use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options);
+use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness);
 use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
@@ -3618,6 +3618,9 @@ sub config_to_command {
     }
 
     if ($conf->{bios} && $conf->{bios} eq 'ovmf') {
+	die "OVMF (UEFI) BIOS is not supported on 32-bit CPU types\n"
+	    if !$forcecpu && get_cpu_bitness($conf->{cpu}, $arch) == 32;
+
 	my ($code_drive_str, $var_drive_str) =
 	    print_ovmf_drive_commandlines($conf, $storecfg, $vmid, $arch, $q35, $version_guard);
 	push $cmd->@*, '-drive', $code_drive_str;
diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index 4be5262..65ba43f 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -12,6 +12,7 @@ use base qw(PVE::SectionConfig Exporter);
 our @EXPORT_OK = qw(
 print_cpu_device
 get_cpu_options
+get_cpu_bitness
 );
 
 # under certain race-conditions, this module might be loaded before pve-cluster
@@ -57,6 +58,17 @@ my $depreacated_cpu_map = {
     'Icelake-Client-noTSX' => 'Icelake-Server-noTSX',
 };
 
+my $cputypes_32bit = {
+    '486' => 1,
+    'pentium' => 1,
+    'pentium2' => 1,
+    'pentium3' => 1,
+    'coreduo' => 1,
+    'athlon' => 1,
+    'kvm32' => 1,
+    'qemu32' => 1,
+};
+
 my $cpu_vendor_list = {
     # Intel CPUs
     486 => 'GenuineIntel',
@@ -725,6 +737,33 @@ sub get_default_cpu_type {
     return $cputype;
 }
 
+sub get_cpu_bitness {
+    my ($cpu_prop_str, $arch) = @_;
+
+    $arch //= get_host_arch();
+
+    my $cputype = get_default_cpu_type($arch, 0);
+
+    if ($cpu_prop_str) {
+	my $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cpu_prop_str)
+	    or die "Cannot parse cpu description: $cpu_prop_str\n";
+
+	my $cputype = $cpu->{cputype};
+
+	if (my $model = $builtin_models->{$cputype}) {
+	    $cputype = $model->{'reported-model'};
+	} elsif (is_custom_model($cputype)) {
+	    my $custom_cpu = get_custom_model($cputype);
+	    $cputype = $custom_cpu->{'reported-model'} // $cpu_fmt->{'reported-model'}->{default};
+	}
+    }
+
+    return $cputypes_32bit->{$cputype} ? 32 : 64 if $arch eq 'x86_64';
+    return 64 if $arch eq 'aarch64';
+
+    die "unsupported architecture '$arch'\n";
+}
+
 __PACKAGE__->register();
 __PACKAGE__->init();
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server 3/5] Move is_native from PVE::QemuServer to PVE::Tools

[Filip Schauer]

Move is_native from PVE::QemuServer to PVE::Tools and rename it to
is_native_arch to be more descriptive.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 PVE/QemuServer.pm | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 33c374c..7600939 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -45,7 +45,7 @@ use PVE::RPCEnvironment;
 use PVE::Storage;
 use PVE::SysFSTools;
 use PVE::Systemd;
-use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_foreach get_host_arch $IPV6RE);
+use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_foreach get_host_arch is_native_arch $IPV6RE);
 
 use PVE::QMPClient;
 use PVE::QemuConfig;
@@ -1752,7 +1752,7 @@ sub print_netdev_full {
         if length($ifname) >= 16;
 
     my $vhostparam = '';
-    if (is_native($arch)) {
+    if (is_native_arch($arch)) {
 	$vhostparam = ',vhost=on' if kernel_has_vhost_net() && $net->{model} eq 'virtio';
     }
 
@@ -3215,11 +3215,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub is_native($) {
-    my ($arch) = @_;
-    return get_host_arch() eq $arch;
-}
-
 sub get_vm_arch {
     my ($conf) = @_;
     return $conf->{arch} // get_host_arch();
@@ -3323,7 +3318,7 @@ my $Arch2Qemu = {
 };
 sub get_command_for_arch($) {
     my ($arch) = @_;
-    return '/usr/bin/kvm' if is_native($arch);
+    return '/usr/bin/kvm' if is_native_arch($arch);
 
     my $cmd = $Arch2Qemu->{$arch}
 	or die "don't know how to emulate architecture '$arch'\n";
@@ -3524,7 +3519,7 @@ sub config_to_command {
 
     my $machine_type = get_vm_machine($conf, $forcemachine, $arch, $add_pve_version);
     my $machine_version = extract_version($machine_type, $kvmver);
-    $kvm //= 1 if is_native($arch);
+    $kvm //= 1 if is_native_arch($arch);
 
     $machine_version =~ m/(\d+)\.(\d+)/;
     my ($machine_major, $machine_minor) = ($1, $2);
-- 
2.39.2

[pve-devel] [PATCH qemu-server 4/5] Unify the default value for 'kvm'

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 PVE/QemuServer.pm           | 5 +++--
 PVE/QemuServer/CPUConfig.pm | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7600939..6055d40 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3744,7 +3744,7 @@ sub config_to_command {
     if ($hotplug_features->{cpu} && min_version($machine_version, 2, 7)) {
 	push @$cmd, '-smp', "1,sockets=$sockets,cores=$cores,maxcpus=$maxcpus";
         for (my $i = 2; $i <= $vcpus; $i++)  {
-	    my $cpustr = print_cpu_device($conf,$i);
+	    my $cpustr = print_cpu_device($conf, $arch, $i);
 	    push @$cmd, '-device', $cpustr;
 	}
 
@@ -4586,9 +4586,10 @@ sub qemu_cpu_hotplug {
 	if scalar(@{$currentrunningvcpus}) != $currentvcpus;
 
     if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
+	my $arch = get_vm_arch($conf);
 
 	for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
-	    my $cpustr = print_cpu_device($conf, $i);
+	    my $cpustr = print_cpu_device($conf, $arch, $i);
 	    qemu_deviceadd($vmid, $cpustr);
 
 	    my $retry = 0;
diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index 65ba43f..7d471f4 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -5,6 +5,7 @@ use warnings;
 
 use PVE::JSONSchema;
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
+use PVE::Tools qw(is_native_arch);
 use PVE::QemuServer::Helpers qw(min_version);
 
 use base qw(PVE::SectionConfig Exporter);
@@ -414,9 +415,9 @@ sub get_custom_model {
 
 # Print a QEMU device node for a given VM configuration for hotplugging CPUs
 sub print_cpu_device {
-    my ($conf, $id) = @_;
+    my ($conf, $arch, $id) = @_;
 
-    my $kvm = $conf->{kvm} // 1;
+    my $kvm = $conf->{kvm} // is_native_arch($arch);
     my $cpu = get_default_cpu_type('x86_64', $kvm);
     if (my $cputype = $conf->{cpu}) {
 	my $cpuconf = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cputype)
-- 
2.39.2

[pve-devel] [PATCH qemu-server 5/5] cpu config: die on hotplug of non x86_64 CPUs

[Filip Schauer]

When attempting a CPU hotplug on an architecture other than x86_64, die
with a clean error instead of attempting a hotplug with a known
non-working device command line. Also move the corresponding FIXME up to
the error.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 PVE/QemuServer/CPUConfig.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
index 7d471f4..01e4515 100644
--- a/PVE/QemuServer/CPUConfig.pm
+++ b/PVE/QemuServer/CPUConfig.pm
@@ -417,6 +417,9 @@ sub get_custom_model {
 sub print_cpu_device {
     my ($conf, $arch, $id) = @_;
 
+    # FIXME: hot plugging other architectures like our unofficial arch64 support?
+    die "Hotplug of non x86_64 CPU not yet supported" if $arch != 'x86_64';
+
     my $kvm = $conf->{kvm} // is_native_arch($arch);
     my $cpu = get_default_cpu_type('x86_64', $kvm);
     if (my $cputype = $conf->{cpu}) {
@@ -441,7 +444,6 @@ sub print_cpu_device {
     my $current_core = ($id - 1) % $cores;
     my $current_socket = int(($id - 1 - $current_core)/$cores);
 
-    # FIXME: hot plugging other architectures like our unofficial arch64 support?
     return "$cpu-x86_64-cpu,id=cpu$id,socket-id=$current_socket,core-id=$current_core,thread-id=0";
 }
 
-- 
2.39.2

Re: [pve-devel] [PATCH qemu-server 4/5] Unify the default value for 'kvm'

[Filip Schauer]

This is missing a descriptive commit message. It must have accidentally
gone lost during a rebase.

Use this as the commit message:

cpu config: Unify the default value for 'kvm'

Make the default value for 'kvm' consistent, taking into account whether
the VM will run on the same CPU architecture as the host.

This would be a breaking change to CPU hotplug for VMs with a different
CPU architecture running on an x86_64 host, as in this case the default
CPU type for CPU hotplug changes from 'kvm64' to 'qemu64'. However, CPU
hotplug of non x86_64 architectures is not supported anyway, so this is
not a breaking change after all.

It should be noted that this change does alter the CPU hotplug behaviour
when emulating an x86_64 CPU on a non-x86_64 host. This is however not
officially supported in Proxmox VE.

On 21/02/2024 15:33, Filip Schauer wrote:
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>   PVE/QemuServer.pm           | 5 +++--
>   PVE/QemuServer/CPUConfig.pm | 5 +++--
>   2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 7600939..6055d40 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3744,7 +3744,7 @@ sub config_to_command {
>       if ($hotplug_features->{cpu} && min_version($machine_version, 2, 7)) {
>   	push @$cmd, '-smp', "1,sockets=$sockets,cores=$cores,maxcpus=$maxcpus";
>           for (my $i = 2; $i <= $vcpus; $i++)  {
> -	    my $cpustr = print_cpu_device($conf,$i);
> +	    my $cpustr = print_cpu_device($conf, $arch, $i);
>   	    push @$cmd, '-device', $cpustr;
>   	}
>   
> @@ -4586,9 +4586,10 @@ sub qemu_cpu_hotplug {
>   	if scalar(@{$currentrunningvcpus}) != $currentvcpus;
>   
>       if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
> +	my $arch = get_vm_arch($conf);
>   
>   	for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
> -	    my $cpustr = print_cpu_device($conf, $i);
> +	    my $cpustr = print_cpu_device($conf, $arch, $i);
>   	    qemu_deviceadd($vmid, $cpustr);
>   
>   	    my $retry = 0;
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 65ba43f..7d471f4 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -5,6 +5,7 @@ use warnings;
>   
>   use PVE::JSONSchema;
>   use PVE::Cluster qw(cfs_register_file cfs_read_file);
> +use PVE::Tools qw(is_native_arch);
>   use PVE::QemuServer::Helpers qw(min_version);
>   
>   use base qw(PVE::SectionConfig Exporter);
> @@ -414,9 +415,9 @@ sub get_custom_model {
>   
>   # Print a QEMU device node for a given VM configuration for hotplugging CPUs
>   sub print_cpu_device {
> -    my ($conf, $id) = @_;
> +    my ($conf, $arch, $id) = @_;
>   
> -    my $kvm = $conf->{kvm} // 1;
> +    my $kvm = $conf->{kvm} // is_native_arch($arch);
>       my $cpu = get_default_cpu_type('x86_64', $kvm);
>       if (my $cputype = $conf->{cpu}) {
>   	my $cpuconf = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cputype)

[pve-devel] partially-applied-series: [PATCH-SERIES v8 qemu-server, common] Prevent starting a 32-bit VM using a 64-bit OVMF BIOS

[Thomas Lamprecht]

Am 21/02/2024 um 15:33 schrieb Filip Schauer:
> This patch series prevents starting a 32-bit VM using a 64-bit OVMF BIOS
> and makes the default value for 'kvm' during CPU hotplug consistent with
> the rest of the code. This is a breaking change for VMs with a different
> CPU architecture running on an x86_64 host.
> 
> Changes since v7:
> * Rename is_native to is_native_arch to be more explicit
> * Move get_default_cpu_type from Helpers to CPUConfig
> * Default to host architecture when no $arch is given to get_cpu_bitness
> * die with a message on hotplug of non x86_64 CPUs in print_cpu_device
> 
> Changes since v6:
> * Skip the CPU bitness check if $forcecpu is set
> * Take custom CPU types into account
> * Add a helper for getting the default CPU type
> * Unify the default value for 'kvm'
> * Move is_native from PVE::QemuServer to PVE::Tools
> 
> Changes since v5:
> * Refactor get_cpu_bitness to die instead of returning undef
> * Pass cputype instead of the entire vm config to get_cpu_bitness
> 
> Changes since v4:
> * Remove parentheses around post-if in get_cpu_bitness
> 
> Changes since v3:
> * Move the cputypes_32bit list from QemuServer.pm to CPUConfig.pm
> * Turn cputypes_32bit into a hash for lookup
> * Create a helper get_cpu_bitness function in CPUConfig.pm
> * Describe how the list of 32-bit CPU types was obtained
> 
> common:
> 
> Filip Schauer (1):
>   tools: add is_native_arch to compare the CPU architecture
> 
>  src/PVE/Tools.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> qemu-server:
> 
> Filip Schauer (5):
>   cpu config: Add helper to get the default CPU type
>   Prevent starting a 32-bit VM using a 64-bit OVMF BIOS
>   Move is_native from PVE::QemuServer to PVE::Tools
>   Unify the default value for 'kvm'
>   cpu config: die on hotplug of non x86_64 CPUs
> 
>  PVE/QemuServer.pm           | 23 +++++++------
>  PVE/QemuServer/CPUConfig.pm | 64 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 67 insertions(+), 20 deletions(-)
> 


applied patch 1 to 4, with the updated commit message for the 4th one, thanks!

Patch 5 has still some issues, see reply there.

Re: [pve-devel] [PATCH qemu-server 5/5] cpu config: die on hotplug of non x86_64 CPUs

[Thomas Lamprecht]

Am 21/02/2024 um 15:33 schrieb Filip Schauer:
> When attempting a CPU hotplug on an architecture other than x86_64, die
> with a clean error instead of attempting a hotplug with a known
> non-working device command line. Also move the corresponding FIXME up to
> the error.
> 
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>  PVE/QemuServer/CPUConfig.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 7d471f4..01e4515 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -417,6 +417,9 @@ sub get_custom_model {
>  sub print_cpu_device {
>      my ($conf, $arch, $id) = @_;
>  
> +    # FIXME: hot plugging other architectures like our unofficial arch64 support?
> +    die "Hotplug of non x86_64 CPU not yet supported" if $arch != 'x86_64';

arbitrary strings need to be compared using `eq` and `ne`, as `==` and `!=` can
only be used for numerics or strings that can be interpreted to one, so this never
could work.

Interestingly the if never triggers in the `!=` case but always triggers in the
`==` case.

Anyhow please fix this and actually test it.

> +
>      my $kvm = $conf->{kvm} // is_native_arch($arch);
>      my $cpu = get_default_cpu_type('x86_64', $kvm);
>      if (my $cputype = $conf->{cpu}) {
> @@ -441,7 +444,6 @@ sub print_cpu_device {
>      my $current_core = ($id - 1) % $cores;
>      my $current_socket = int(($id - 1 - $current_core)/$cores);
>  
> -    # FIXME: hot plugging other architectures like our unofficial arch64 support?
>      return "$cpu-x86_64-cpu,id=cpu$id,socket-id=$current_socket,core-id=$current_core,thread-id=0";
>  }
>  

Re: [pve-devel] [PATCH qemu-server 5/5] cpu config: die on hotplug of non x86_64 CPUs

[Fiona Ebner]

Am 08.03.24 um 14:34 schrieb Thomas Lamprecht:
> Am 21/02/2024 um 15:33 schrieb Filip Schauer:
>> When attempting a CPU hotplug on an architecture other than x86_64, die
>> with a clean error instead of attempting a hotplug with a known
>> non-working device command line. Also move the corresponding FIXME up to
>> the error.
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>>  PVE/QemuServer/CPUConfig.pm | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
>> index 7d471f4..01e4515 100644
>> --- a/PVE/QemuServer/CPUConfig.pm
>> +++ b/PVE/QemuServer/CPUConfig.pm
>> @@ -417,6 +417,9 @@ sub get_custom_model {
>>  sub print_cpu_device {
>>      my ($conf, $arch, $id) = @_;
>>  
>> +    # FIXME: hot plugging other architectures like our unofficial arch64 support?
>> +    die "Hotplug of non x86_64 CPU not yet supported" if $arch != 'x86_64';
> 
> arbitrary strings need to be compared using `eq` and `ne`, as `==` and `!=` can
> only be used for numerics or strings that can be interpreted to one, so this never
> could work.
> 
> Interestingly the if never triggers in the `!=` case but always triggers in the
> `==` case.
> 

Because strings that cannot be parsed as a number are interpreted as 0,
so both (invalid) sides will be 0. Description of parsing rules (from
[0], wasn't able to find in official docs):

> It follows these basic rules:
> 
>     Ignore leading whitespace. This is handy when you extract a field out of columnar data and the number doesn't take up the entire column.
>     Allow for a single leading sign (+ or -)
>     Skip leading zeros (so, no octal)
>     Capture decimal ASCII digits (0,1,2,3,4,5,6,7,8,9), allowing for a single decimal point (so, no semantic versioning numbers)
>     Stop at the first non-decimal-digit character
>     Whatever you have so far is the number. If you have nothing, the number is 0.


[0]:
https://stackoverflow.com/questions/70482447/why-is-00-equal-to-0-in-perl/70496787#70496787

Re: [pve-devel] [PATCH qemu-server 5/5] cpu config: die on hotplug of non x86_64 CPUs

[Filip Schauer]

Here is a fixed patch v2:

https://lists.proxmox.com/pipermail/pve-devel/2024-March/062153.html

On 08/03/2024 14:53, Fiona Ebner wrote:
> Am 08.03.24 um 14:34 schrieb Thomas Lamprecht:
>> Am 21/02/2024 um 15:33 schrieb Filip Schauer:
>>> When attempting a CPU hotplug on an architecture other than x86_64, die
>>> with a clean error instead of attempting a hotplug with a known
>>> non-working device command line. Also move the corresponding FIXME up to
>>> the error.
>>>
>>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>>> ---
>>>   PVE/QemuServer/CPUConfig.pm | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
>>> index 7d471f4..01e4515 100644
>>> --- a/PVE/QemuServer/CPUConfig.pm
>>> +++ b/PVE/QemuServer/CPUConfig.pm
>>> @@ -417,6 +417,9 @@ sub get_custom_model {
>>>   sub print_cpu_device {
>>>       my ($conf, $arch, $id) = @_;
>>>   
>>> +    # FIXME: hot plugging other architectures like our unofficial arch64 support?
>>> +    die "Hotplug of non x86_64 CPU not yet supported" if $arch != 'x86_64';
>> arbitrary strings need to be compared using `eq` and `ne`, as `==` and `!=` can
>> only be used for numerics or strings that can be interpreted to one, so this never
>> could work.
>>
>> Interestingly the if never triggers in the `!=` case but always triggers in the
>> `==` case.
>>
> Because strings that cannot be parsed as a number are interpreted as 0,
> so both (invalid) sides will be 0. Description of parsing rules (from
> [0], wasn't able to find in official docs):
>
>> It follows these basic rules:
>>
>>      Ignore leading whitespace. This is handy when you extract a field out of columnar data and the number doesn't take up the entire column.
>>      Allow for a single leading sign (+ or -)
>>      Skip leading zeros (so, no octal)
>>      Capture decimal ASCII digits (0,1,2,3,4,5,6,7,8,9), allowing for a single decimal point (so, no semantic versioning numbers)
>>      Stop at the first non-decimal-digit character
>>      Whatever you have so far is the number. If you have nothing, the number is 0.
>
> [0]:
> https://stackoverflow.com/questions/70482447/why-is-00-equal-to-0-in-perl/70496787#70496787