[pve-devel] [PATCH qemu/qemu-server/docs v2 00/18] adapt to changes in QEMU machine version deprecation/removal

[pve-devel] [PATCH qemu/qemu-server/docs v2 00/18] adapt to changes in QEMU machine version deprecation/removal

[Fiona Ebner]

Changes in v2:
* change deprecation logic into QEMU itself rather than just weakening
  the warning there (so no need to change the deprecation logic in
  qemu-server anymore)
* get rid of outdated information from "Update to a Newer Machine
  Version" section in docs

In QEMU commit a35f8577a0 ("include/hw: add macros for deprecation &
removal of versioned machines"), a new machine version deprecation and
removal policy was introduced. After only 3 years a machine version
will be deprecated while being removed after 6 years.

The deprecation is a bit early considering major PVE releases are
approximately every 2 years. This means that a deprecation warning can
already happen for a machine version that was introduced during the
previous major release. This would scare users for no good reason, so
avoid deprecating machine versions in PVE too early and define a
baseline of machine versions that will be supported throughout a
single major PVE release.

The new policy takes effect only in QEMU 10.1, see QEMU commit
c9fd2d9a48 ("include/hw: temporarily disable deletion of versioned
machine types"). Machine versions <=2.3 were already deprecated for a
while, with PVE also warning about it since commit dec371d9 ("vm
start: add warning about deprecated machine version") in qemu-server
8.0.8. These have been dropped in QEMU 9.1, so the baseline for PVE 8
is 2.4.

This series is intended to allow broader QEMU 9.1 rollout. Still
required in addition to this series before PVE 9:
* wiki article about what to look out for when changing machine
  version
* checks in pve8to9 script giving errors/warnings when machine version
  is too old

qemu:

Fiona Ebner (1):
  adapt machine version deprecation for Proxmox VE

 ...e-version-deprecation-for-Proxmox-VE.patch | 137 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 138 insertions(+)
 create mode 100644 debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch


qemu-server:

Fiona Ebner (16):
  machine: drop unused parameter from assert_valid_machine_property()
    helper
  move get_command_for_arch() helper to helpers module
  helpers: improve name for variable for mapping arch to binary
  move kvm_user_version() function to helpers module
  move get_vm_arch() helper to helpers module
  machine: add default_machine_for_arch() helper
  move get_installed_machine_version() helper to machine module
  move windows_get_pinned_machine_version() function to machine module
  move get_vm_machine() function to machine module
  move meta information handling to its own module
  machine: get vm machine: fallback to creation QEMU version for windows
    starting with 9.1
  machine: add check_and_pin_machine_string() helper
  api: update vm config: pin machine version when switching to windows
    os type
  machine: log informational line when pinning machine version for
    Windows guest
  machine: rename machine_version() function to
    machine_version_at_least()
  machine: code cleanup: avoid superfluous augmented assignment operator

 PVE/API2/Qemu.pm                 |  44 +++++---
 PVE/QemuServer.pm                | 184 +++----------------------------
 PVE/QemuServer/Helpers.pm        |  49 ++++++++
 PVE/QemuServer/Machine.pm        | 107 +++++++++++++++++-
 PVE/QemuServer/Makefile          |   1 +
 PVE/QemuServer/MetaInfo.pm       |  47 ++++++++
 test/run_config2command_tests.pl |   3 +-
 7 files changed, 248 insertions(+), 187 deletions(-)
 create mode 100644 PVE/QemuServer/MetaInfo.pm


pve-docs:

Fiona Ebner (1):
  qm: machine version: document support in PVE

 qm.adoc | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)


Summary over all repositories:
  10 files changed, 417 insertions(+), 195 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 v2 01/18] adapt machine version deprecation for Proxmox VE

[Fiona Ebner]

In commit a35f8577a0 ("include/hw: add macros for deprecation &
removal of versioned machines"), a new machine version deprecation and
removal policy was introduced. After only 3 years a machine version
will be deprecated while being removed after 6 years.

The deprecation is a bit early considering major PVE releases are
approximately every 2 years. This means that a deprecation warning can
already happen for a machine version that was introduced during the
previous major release. This would scare users for no good reason, so
avoid deprecating machine versions in PVE too early and define a
baseline of machine versions that will be supported throughout a
single major PVE release.

Reported-by: Martin Maurer <martin@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Rather than just weakening the warning from the QEMU side, adapt the
  deprecation logic for PVE already here.

 ...e-version-deprecation-for-Proxmox-VE.patch | 137 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 138 insertions(+)
 create mode 100644 debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch

diff --git a/debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch b/debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
new file mode 100644
index 0000000..6c1a73a
--- /dev/null
+++ b/debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
@@ -0,0 +1,137 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner@proxmox.com>
+Date: Fri, 3 Jan 2025 14:03:12 +0100
+Subject: [PATCH] adapt machine version deprecation for Proxmox VE
+
+In commit a35f8577a0 ("include/hw: add macros for deprecation &
+removal of versioned machines"), a new machine version deprecation and
+removal policy was introduced. After only 3 years a machine version
+will be deprecated while being removed after 6 years.
+
+The deprecation is a bit early considering major PVE releases are
+approximately every 2 years. This means that a deprecation warning can
+already happen for a machine version that was introduced during the
+previous major release. This would scare users for no good reason, so
+avoid deprecating machine versions in PVE too early and define a
+baseline of machine versions that will be supported throughout a
+single major PVE release.
+
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ include/hw/boards.h | 78 +++++++++++++++++++++++++++++----------------
+ 1 file changed, 51 insertions(+), 27 deletions(-)
+
+diff --git a/include/hw/boards.h b/include/hw/boards.h
+index 5cddeb7fcb..b1e7787499 100644
+--- a/include/hw/boards.h
++++ b/include/hw/boards.h
+@@ -607,42 +607,66 @@ struct MachineState {
+ 
+ 
+ /*
+- * How many years/major releases for each phase
+- * of the life cycle. Assumes use of versioning
+- * scheme where major is bumped each year
++ * Baseline of machine versions that are still considered supported throughout
++ * current major Proxmox VE release. Machine versions older than this are
++ * considered to be deprecated in Proxmox VE.
++ *
++ * Machine versions older than 6 years are removed just like in upstream QEMU.
++ * (policy takes effect with QEMU 10.1). Assumes yearly major QEMU release.
++ *
++ * QEMU release cylce N.0 in ~April, N.1 in ~August, N.2 in ~December
++ * Debian/PVE release cylce ~every two years in summer
++ *
++ * PVE - last QEMU - machine versions dropped - baseline
++ *   8         9.2              2.3 and older        2.4
++ *   9        11.2              5.2 and older        6.0
++ *  10        13.2              7.2 and older        8.0
++ */
++#define MACHINE_VER_BASELINE_PVE_MAJOR 2
++#define MACHINE_VER_BASELINE_PVE_MINOR 4
++#define MACHINE_VER_DELETION_MAJOR (QEMU_VERSION_MAJOR - 6)
++#define MACHINE_VER_DELETION_MINOR QEMU_VERSION_MINOR
++
++/*
++ * Proxmox VE needs to support the baseline throughout a major PVE release. So
++ * a QEMU release where the baseline is already deleted cannot be used.
++ * Removal policy after 6 years takes effect with QEMU 10.1.
+  */
+-#define MACHINE_VER_DELETION_MAJOR 6
+-#define MACHINE_VER_DEPRECATION_MAJOR 3
++#if ((QEMU_VERSION_MAJOR > 10) || ((QEMU_VERSION_MAJOR == 10) && (QEMU_VERSION_MINOR >= 1)))
++#if ((MACHINE_VER_BASELINE_PVE_MAJOR < MACHINE_VER_DELETION_MAJOR) || \
++     ((MACHINE_VER_BASELINE_PVE_MAJOR == MACHINE_VER_DELETION_MAJOR) && \
++      (MACHINE_VER_BASELINE_PVE_MINOR < MACHINE_VER_DELETION_MINOR)))
++#error "Baseline machine version needed by Proxmox VE not supported anymore by this QEMU release"
++#endif
++#endif
+ 
+ /*
+  * Expands to a static string containing a deprecation
+  * message for a versioned machine type
+  */
+ #define MACHINE_VER_DEPRECATION_MSG \
+-    "machines more than " stringify(MACHINE_VER_DEPRECATION_MAJOR) \
+-    " years old are subject to deletion after " \
+-    stringify(MACHINE_VER_DELETION_MAJOR) " years"
+-
+-#define _MACHINE_VER_IS_EXPIRED_IMPL(cutoff, major, minor) \
+-    (((QEMU_VERSION_MAJOR - major) > cutoff) || \
+-     (((QEMU_VERSION_MAJOR - major) == cutoff) && \
+-      (QEMU_VERSION_MINOR - minor) >= 0))
+-
+-#define _MACHINE_VER_IS_EXPIRED2(cutoff, major, minor) \
+-    _MACHINE_VER_IS_EXPIRED_IMPL(cutoff, major, minor)
+-#define _MACHINE_VER_IS_EXPIRED3(cutoff, major, minor, micro) \
+-    _MACHINE_VER_IS_EXPIRED_IMPL(cutoff, major, minor)
+-#define _MACHINE_VER_IS_EXPIRED4(cutoff, major, minor, _unused, tag) \
+-    _MACHINE_VER_IS_EXPIRED_IMPL(cutoff, major, minor)
+-#define _MACHINE_VER_IS_EXPIRED5(cutoff, major, minor, micro, _unused, tag)   \
+-    _MACHINE_VER_IS_EXPIRED_IMPL(cutoff, major, minor)
+-
+-#define _MACHINE_IS_EXPIRED(cutoff, ...) \
++    "old machine version is subject to deletion during current major Proxmox VE release"
++
++#define _MACHINE_VER_IS_EXPIRED_IMPL(baseline_major, baseline_minor, major, minor) \
++    ((major < baseline_major) || \
++     ((major == baseline_major) && \
++      (minor < baseline_minor)))
++
++#define _MACHINE_VER_IS_EXPIRED2(baseline_major, baseline_minor, major, minor) \
++    _MACHINE_VER_IS_EXPIRED_IMPL(baseline_major, baseline_minor, major, minor)
++#define _MACHINE_VER_IS_EXPIRED3(baseline_major, baseline_minor, major, minor, micro) \
++    _MACHINE_VER_IS_EXPIRED_IMPL(baseline_major, baseline_minor, major, minor)
++#define _MACHINE_VER_IS_EXPIRED4(baseline_major, baseline_minor, major, minor, _unused, tag) \
++    _MACHINE_VER_IS_EXPIRED_IMPL(baseline_major, baseline_minor, major, minor)
++#define _MACHINE_VER_IS_EXPIRED5(baseline_major, baseline_minor, major, minor, micro, _unused, tag)   \
++    _MACHINE_VER_IS_EXPIRED_IMPL(baseline_major, baseline_minor, major, minor)
++
++#define _MACHINE_IS_EXPIRED(baseline_major, baseline_minor, ...) \
+     _MACHINE_VER_PICK(__VA_ARGS__, \
+                       _MACHINE_VER_IS_EXPIRED5, \
+                       _MACHINE_VER_IS_EXPIRED4, \
+                       _MACHINE_VER_IS_EXPIRED3, \
+-                      _MACHINE_VER_IS_EXPIRED2) (cutoff, __VA_ARGS__)
++                      _MACHINE_VER_IS_EXPIRED2) (baseline_major, baseline_minor, __VA_ARGS__)
+ 
+ /*
+  * Evaluates true when a machine type with (major, minor)
+@@ -651,7 +675,7 @@ struct MachineState {
+  * lifecycle rules
+  */
+ #define MACHINE_VER_IS_DEPRECATED(...) \
+-    _MACHINE_IS_EXPIRED(MACHINE_VER_DEPRECATION_MAJOR, __VA_ARGS__)
++    _MACHINE_IS_EXPIRED(MACHINE_VER_BASELINE_PVE_MAJOR, MACHINE_VER_BASELINE_PVE_MINOR, __VA_ARGS__)
+ 
+ /*
+  * Evaluates true when a machine type with (major, minor)
+@@ -660,7 +684,7 @@ struct MachineState {
+  * lifecycle rules
+  */
+ #define MACHINE_VER_SHOULD_DELETE(...) \
+-    _MACHINE_IS_EXPIRED(MACHINE_VER_DELETION_MAJOR, __VA_ARGS__)
++    _MACHINE_IS_EXPIRED(MACHINE_VER_DELETION_MAJOR, MACHINE_VER_DELETION_MINOR, __VA_ARGS__)
+ 
+ /*
+  * Sets the deprecation reason for a versioned machine based
diff --git a/debian/patches/series b/debian/patches/series
index 0b48878..9ebf335 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -61,3 +61,4 @@ pve/0048-PVE-backup-fixup-error-handling-for-fleecing.patch
 pve/0049-PVE-backup-factor-out-setting-up-snapshot-access-for.patch
 pve/0050-PVE-backup-save-device-name-in-device-info-structure.patch
 pve/0051-PVE-backup-include-device-name-in-error-when-setting.patch
+pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
-- 
2.39.5



_______________________________________________
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 02/18] machine: drop unused parameter from assert_valid_machine_property() helper

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm          | 4 ++--
 PVE/QemuServer.pm         | 2 +-
 PVE/QemuServer/Machine.pm | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8392400e..52425ee8 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1250,7 +1250,7 @@ __PACKAGE__->register_method({
 			    $conf->{machine} = PVE::QemuServer::Machine::print_machine($machine_conf);
 			}
 		    }
-		    PVE::QemuServer::Machine::assert_valid_machine_property($conf, $machine_conf);
+		    PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
 
 		    $conf->{lock} = 'import' if $live_import_mapping;
 
@@ -2113,7 +2113,7 @@ my $update_vm_api  = sub {
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} elsif ($opt eq 'machine') {
 		    my $machine_conf = PVE::QemuServer::Machine::parse_machine($param->{$opt});
-		    PVE::QemuServer::Machine::assert_valid_machine_property($conf, $machine_conf);
+		    PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} elsif ($opt eq 'cipassword') {
 		    if (!PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 5cde94a1..bc38fb1b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4193,7 +4193,7 @@ sub config_to_command {
     $machine_type_min .= "+pve$required_pve_version";
     push @$machineFlags, "type=${machine_type_min}";
 
-    PVE::QemuServer::Machine::assert_valid_machine_property($conf, $machine_conf);
+    PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
 
     if (my $viommu = $machine_conf->{viommu}) {
 	if ($viommu eq 'intel') {
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index a072ac29..7f03ef20 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -56,7 +56,7 @@ sub print_machine {
 }
 
 sub assert_valid_machine_property {
-    my ($conf, $machine_conf) = @_;
+    my ($machine_conf) = @_;
     my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0;
     if ($machine_conf->{viommu} && $machine_conf->{viommu} eq "intel" && !$q35) {
 	die "to use Intel vIOMMU please set the machine type to q35\n";
-- 
2.39.5



_______________________________________________
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 03/18] move get_command_for_arch() helper to helpers module

[Fiona Ebner]

Cannot use the is_native_arch() helper inside the function anymore,
to avoid a cyclic dependency between the 'CPUConfig' and 'Helpers'
modules, inline it.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm         | 19 +++----------------
 PVE/QemuServer/Helpers.pm | 13 +++++++++++++
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index bc38fb1b..ce962b7a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1199,7 +1199,7 @@ my $kvm_mtime = {};
 sub kvm_user_version {
     my ($binary) = @_;
 
-    $binary //= get_command_for_arch(get_host_arch()); # get the native arch by default
+    $binary //= PVE::QemuServer::Helpers::get_command_for_arch(get_host_arch()); # get the native arch by default
     my $st = stat($binary);
 
     my $cachedmtime = $kvm_mtime->{$binary} // -1;
@@ -3397,19 +3397,6 @@ sub get_ovmf_files($$$) {
     return ($ovmf_code, $ovmf_vars);
 }
 
-my $Arch2Qemu = {
-    aarch64 => '/usr/bin/qemu-system-aarch64',
-    x86_64 => '/usr/bin/qemu-system-x86_64',
-};
-sub get_command_for_arch($) {
-    my ($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";
-    return $cmd;
-}
-
 # To use query_supported_cpu_flags and query_understood_cpu_flags to get flags
 # to use in a QEMU command line (-cpu element), first array_intersect the result
 # of query_supported_ with query_understood_. This is necessary because:
@@ -3444,7 +3431,7 @@ sub query_supported_cpu_flags {
 	$arch eq "aarch64";
 
     my $kvm_supported = defined(kvm_version());
-    my $qemu_cmd = get_command_for_arch($arch);
+    my $qemu_cmd = PVE::QemuServer::Helpers::get_command_for_arch($arch);
     my $fakevmid = -1;
     my $pidfile = PVE::QemuServer::Helpers::pidfile_name($fakevmid);
 
@@ -3634,7 +3621,7 @@ sub config_to_command {
     my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
 
     my $arch = get_vm_arch($conf);
-    my $kvm_binary = get_command_for_arch($arch);
+    my $kvm_binary = PVE::QemuServer::Helpers::get_command_for_arch($arch);
     my $kvmver = kvm_user_version($kvm_binary);
 
     if (!$kvmver || $kvmver !~ m/^(\d+)\.(\d+)/ || $1 < 5) {
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 72a46a0a..d58bad2b 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -19,6 +19,19 @@ windows_version
 
 my $nodename = PVE::INotify::nodename();
 
+my $Arch2Qemu = {
+    aarch64 => '/usr/bin/qemu-system-aarch64',
+    x86_64 => '/usr/bin/qemu-system-x86_64',
+};
+sub get_command_for_arch($) {
+    my ($arch) = @_;
+    return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch
+
+    my $cmd = $Arch2Qemu->{$arch}
+	or die "don't know how to emulate architecture '$arch'\n";
+    return $cmd;
+}
+
 # Paths and directories
 
 our $var_run_tmpdir = "/var/run/qemu-server";
-- 
2.39.5



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

Re: [pve-devel] [PATCH qemu-server v2 03/18] move get_command_for_arch() helper to helpers module

[Thomas Lamprecht]

Am 16.01.25 um 12:50 schrieb Fiona Ebner:
> diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
> index 72a46a0a..d58bad2b 100644
> --- a/PVE/QemuServer/Helpers.pm
> +++ b/PVE/QemuServer/Helpers.pm
> @@ -19,6 +19,19 @@ windows_version
>  
>  my $nodename = PVE::INotify::nodename();
>  
> +my $Arch2Qemu = {
> +    aarch64 => '/usr/bin/qemu-system-aarch64',
> +    x86_64 => '/usr/bin/qemu-system-x86_64',
> +};
> +sub get_command_for_arch($) {
> +    my ($arch) = @_;
> +    return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch


This introduces a test/runtime error:

Undefined subroutine &PVE::QemuServer::Helpers::get_host_arch called at ../PVE/QemuServer/Helpers.pm line 28.

It's fixed again in 05/18 "move kvm_user_version() function to helpers module",
the import should be added here already.


_______________________________________________
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 04/18] helpers: improve name for variable for mapping arch to binary

[Fiona Ebner]

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

diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index d58bad2b..be10a92a 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -19,7 +19,7 @@ windows_version
 
 my $nodename = PVE::INotify::nodename();
 
-my $Arch2Qemu = {
+my $arch_to_qemu_binary = {
     aarch64 => '/usr/bin/qemu-system-aarch64',
     x86_64 => '/usr/bin/qemu-system-x86_64',
 };
@@ -27,7 +27,7 @@ sub get_command_for_arch($) {
     my ($arch) = @_;
     return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch
 
-    my $cmd = $Arch2Qemu->{$arch}
+    my $cmd = $arch_to_qemu_binary->{$arch}
 	or die "don't know how to emulate architecture '$arch'\n";
     return $cmd;
 }
-- 
2.39.5



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

Re: [pve-devel] [PATCH qemu-server v2 04/18] helpers: improve name for variable for mapping arch to binary

[Thomas Lamprecht]

Am 16.01.25 um 12:50 schrieb Fiona Ebner:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuServer/Helpers.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
> index d58bad2b..be10a92a 100644
> --- a/PVE/QemuServer/Helpers.pm
> +++ b/PVE/QemuServer/Helpers.pm
> @@ -19,7 +19,7 @@ windows_version
>  
>  my $nodename = PVE::INotify::nodename();
>  
> -my $Arch2Qemu = {
> +my $arch_to_qemu_binary = {
>      aarch64 => '/usr/bin/qemu-system-aarch64',
>      x86_64 => '/usr/bin/qemu-system-x86_64',
>  };
> @@ -27,7 +27,7 @@ sub get_command_for_arch($) {
>      my ($arch) = @_;
>      return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch
>  
> -    my $cmd = $Arch2Qemu->{$arch}
> +    my $cmd = $arch_to_qemu_binary->{$arch}
>  	or die "don't know how to emulate architecture '$arch'\n";
>      return $cmd;
>  }

could be squashed into the previous patch.


_______________________________________________
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 05/18] move kvm_user_version() function to helpers module

[Fiona Ebner]

Add an export, since the function is rather commonly used (in
particular inlined in function calls, where prefixing with the module
name would hurt readability) and there won't be much potential for
confusion name-wise.

This was the only user of stat(), so remove the File::stat include.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm                 |  2 +-
 PVE/QemuServer.pm                | 32 +-------------------------------
 PVE/QemuServer/Helpers.pm        | 31 +++++++++++++++++++++++++++++++
 test/run_config2command_tests.pl |  3 ++-
 4 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 52425ee8..868508e7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -854,7 +854,7 @@ sub assert_scsi_feature_compatibility {
 
     my $machine_type = PVE::QemuServer::get_vm_machine($conf, undef, $conf->{arch});
     my $machine_version = PVE::QemuServer::Machine::extract_version(
-	$machine_type, PVE::QemuServer::kvm_user_version());
+	$machine_type, PVE::QemuServer::Helpers::kvm_user_version());
     my $drivetype = PVE::QemuServer::Drive::get_scsi_device_type(
 	$drive, $storecfg, $machine_version);
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ce962b7a..fd1feada 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -10,7 +10,6 @@ use Fcntl;
 use File::Basename;
 use File::Copy qw(copy);
 use File::Path;
-use File::stat;
 use Getopt::Long;
 use IO::Dir;
 use IO::File;
@@ -51,7 +50,7 @@ use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_for
 
 use PVE::QMPClient;
 use PVE::QemuConfig;
-use PVE::QemuServer::Helpers qw(config_aware_timeout min_version windows_version);
+use PVE::QemuServer::Helpers qw(config_aware_timeout min_version kvm_user_version windows_version);
 use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CGroup;
 use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object);
@@ -1193,35 +1192,6 @@ sub kvm_version {
     return $kvm_api_version;
 }
 
-my $kvm_user_version = {};
-my $kvm_mtime = {};
-
-sub kvm_user_version {
-    my ($binary) = @_;
-
-    $binary //= PVE::QemuServer::Helpers::get_command_for_arch(get_host_arch()); # get the native arch by default
-    my $st = stat($binary);
-
-    my $cachedmtime = $kvm_mtime->{$binary} // -1;
-    return $kvm_user_version->{$binary} if $kvm_user_version->{$binary} &&
-	$cachedmtime == $st->mtime;
-
-    $kvm_user_version->{$binary} = 'unknown';
-    $kvm_mtime->{$binary} = $st->mtime;
-
-    my $code = sub {
-	my $line = shift;
-	if ($line =~ m/^QEMU( PC)? emulator version (\d+\.\d+(\.\d+)?)(\.\d+)?[,\s]/) {
-	    $kvm_user_version->{$binary} = $2;
-	}
-    };
-
-    eval { run_command([$binary, '--version'], outfunc => $code); };
-    warn $@ if $@;
-
-    return $kvm_user_version->{$binary};
-
-}
 my sub extract_version {
     my ($machine_type, $version) = @_;
     $version = kvm_user_version() if !defined($version);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index be10a92a..07b2ff6e 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -8,11 +8,13 @@ use JSON;
 
 use PVE::INotify;
 use PVE::ProcFSTools;
+use PVE::Tools qw(get_host_arch);
 
 use base 'Exporter';
 our @EXPORT_OK = qw(
 min_version
 config_aware_timeout
+kvm_user_version
 parse_number_sets
 windows_version
 );
@@ -32,6 +34,35 @@ sub get_command_for_arch($) {
     return $cmd;
 }
 
+my $kvm_user_version = {};
+my $kvm_mtime = {};
+
+sub kvm_user_version {
+    my ($binary) = @_;
+
+    $binary //= get_command_for_arch(get_host_arch()); # get the native arch by default
+    my $st = stat($binary);
+
+    my $cachedmtime = $kvm_mtime->{$binary} // -1;
+    return $kvm_user_version->{$binary} if $kvm_user_version->{$binary} &&
+	$cachedmtime == $st->mtime;
+
+    $kvm_user_version->{$binary} = 'unknown';
+    $kvm_mtime->{$binary} = $st->mtime;
+
+    my $code = sub {
+	my $line = shift;
+	if ($line =~ m/^QEMU( PC)? emulator version (\d+\.\d+(\.\d+)?)(\.\d+)?[,\s]/) {
+	    $kvm_user_version->{$binary} = $2;
+	}
+    };
+
+    eval { PVE::Tools::run_command([$binary, '--version'], outfunc => $code); };
+    warn $@ if $@;
+
+    return $kvm_user_version->{$binary};
+}
+
 # Paths and directories
 
 our $var_run_tmpdir = "/var/run/qemu-server";
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 5308b1fc..2feebd4a 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -15,6 +15,7 @@ use PVE::SysFSTools;
 
 use PVE::QemuConfig;
 use PVE::QemuServer;
+use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Monitor;
 use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer::CPUConfig;
@@ -72,7 +73,7 @@ my $base_env = {
 	}
     },
     vmid => 8006,
-    real_qemu_version => PVE::QemuServer::kvm_user_version(), # not yet mocked
+    real_qemu_version => PVE::QemuServer::Helpers::kvm_user_version(), # not yet mocked
 };
 
 my $pci_devs = [
-- 
2.39.5



_______________________________________________
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 06/18] move get_vm_arch() helper to helpers module

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm          |  4 ++--
 PVE/QemuServer.pm         | 13 ++++---------
 PVE/QemuServer/Helpers.pm |  5 +++++
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 868508e7..2840de1b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1198,7 +1198,7 @@ __PACKAGE__->register_method({
 
 	    my $realcmd = sub {
 		my $conf = $param;
-		my $arch = PVE::QemuServer::get_vm_arch($conf);
+		my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
 
 		for my $opt (sort keys $param->%*) {
 		    next if $opt !~ m/^scsi\d+$/;
@@ -2034,7 +2034,7 @@ my $update_vm_api  = sub {
 		$conf = PVE::QemuConfig->load_config($vmid); # update/reload
 		next if defined($conf->{pending}->{$opt}) && ($param->{$opt} eq $conf->{pending}->{$opt}); # skip if nothing changed
 
-		my $arch = PVE::QemuServer::get_vm_arch($conf);
+		my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
 
 		if (PVE::QemuServer::is_valid_drivename($opt)) {
 		    # old drive
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index fd1feada..164cb0e8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,11 +3271,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub get_vm_arch {
-    my ($conf) = @_;
-    return $conf->{arch} // get_host_arch();
-}
-
 my $default_machines = {
     x86_64 => 'pc',
     aarch64 => 'virt',
@@ -3590,7 +3585,7 @@ sub config_to_command {
 
     my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
 
-    my $arch = get_vm_arch($conf);
+    my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
     my $kvm_binary = PVE::QemuServer::Helpers::get_command_for_arch($arch);
     my $kvmver = kvm_user_version($kvm_binary);
 
@@ -4670,7 +4665,7 @@ sub qemu_cpu_hotplug {
 	if scalar(@{$currentrunningvcpus}) != $currentvcpus;
 
     if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
-	my $arch = get_vm_arch($conf);
+	my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
 
 	for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
 	    my $cpustr = print_cpu_device($conf, $arch, $i);
@@ -4912,7 +4907,7 @@ sub vmconfig_hotplug_pending {
     my ($vmid, $conf, $storecfg, $selection, $errors) = @_;
 
     my $defaults = load_defaults();
-    my $arch = get_vm_arch($conf);
+    my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
     my $machine_type = get_vm_machine($conf, undef, $arch);
 
     # commit values which do not have any impact on running VM first
@@ -8493,7 +8488,7 @@ sub qemu_use_old_bios_files {
 sub get_efivars_size {
     my ($conf, $efidisk) = @_;
 
-    my $arch = get_vm_arch($conf);
+    my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
     $efidisk //= $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef;
     my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
     my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 07b2ff6e..8e9f4fc0 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -34,6 +34,11 @@ sub get_command_for_arch($) {
     return $cmd;
 }
 
+sub get_vm_arch {
+    my ($conf) = @_;
+    return $conf->{arch} // get_host_arch();
+}
+
 my $kvm_user_version = {};
 my $kvm_mtime = {};
 
-- 
2.39.5



_______________________________________________
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 07/18] machine: add default_machine_for_arch() helper

[Fiona Ebner]

There are already other places where 'aarch64' and 'x86_64' are
checked to be the only valid architectures, for example, the
get_command_for_arch() helper, so the new error scenario for an
unknown arch should not cause any regressions.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm         |  9 ++-------
 PVE/QemuServer/Machine.pm | 12 ++++++++++++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 164cb0e8..82795767 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,11 +3271,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-my $default_machines = {
-    x86_64 => 'pc',
-    aarch64 => 'virt',
-};
-
 sub get_installed_machine_version {
     my ($kvmversion) = @_;
     $kvmversion = kvm_user_version() if !defined($kvmversion);
@@ -3320,7 +3315,7 @@ sub get_vm_machine {
 	    $machine = windows_get_pinned_machine_version($machine, '5.1', $kvmversion);
 	}
 	$arch //= 'x86_64';
-	$machine ||= $default_machines->{$arch};
+	$machine ||= PVE::QemuServer::Machine::default_machine_for_arch($arch);
 	my $pvever = PVE::QemuServer::Machine::get_pve_version($kvmversion);
 	$machine .= "+pve$pvever";
     }
@@ -3386,7 +3381,7 @@ sub query_supported_cpu_flags {
     my ($arch) = @_;
 
     $arch //= get_host_arch();
-    my $default_machine = $default_machines->{$arch};
+    my $default_machine = PVE::QemuServer::Machine::default_machine_for_arch($arch);
 
     my $flags = {};
 
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 7f03ef20..5a039244 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -55,6 +55,18 @@ sub print_machine {
     return print_property_string($machine_conf, $machine_fmt);
 }
 
+my $default_machines = {
+    x86_64 => 'pc',
+    aarch64 => 'virt',
+};
+
+sub default_machine_for_arch {
+    my ($arch) = @_;
+
+    my $machine = $default_machines->{$arch} or die "unsupported architecture '$arch'\n";
+    return $machine;
+}
+
 sub assert_valid_machine_property {
     my ($machine_conf) = @_;
     my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0;
-- 
2.39.5



_______________________________________________
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 08/18] move get_installed_machine_version() helper to machine module

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm         | 9 +--------
 PVE/QemuServer/Machine.pm | 7 +++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 82795767..ee7bb017 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,13 +3271,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub get_installed_machine_version {
-    my ($kvmversion) = @_;
-    $kvmversion = kvm_user_version() if !defined($kvmversion);
-    $kvmversion =~ m/^(\d+\.\d+)/;
-    return $1;
-}
-
 sub windows_get_pinned_machine_version {
     my ($machine, $base_version, $kvmversion) = @_;
 
@@ -3285,7 +3278,7 @@ sub windows_get_pinned_machine_version {
     if (!defined($base_version) ||
 	!PVE::QemuServer::Machine::can_run_pve_machine_version($base_version, $kvmversion)
     ) {
-	$pin_version = get_installed_machine_version($kvmversion);
+	$pin_version = PVE::QemuServer::Machine::get_installed_machine_version($kvmversion);
     }
     if (!$machine || $machine eq 'pc') {
 	$machine = "pc-i440fx-$pin_version";
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 5a039244..075554f4 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -186,4 +186,11 @@ sub qemu_machine_pxe {
     return $machine;
 }
 
+sub get_installed_machine_version {
+    my ($kvmversion) = @_;
+    $kvmversion = PVE::QemuServer::Helpers::kvm_user_version() if !defined($kvmversion);
+    $kvmversion =~ m/^(\d+\.\d+)/;
+    return $1;
+}
+
 1;
-- 
2.39.5



_______________________________________________
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 09/18] move windows_get_pinned_machine_version() function to machine module

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm          |  3 ++-
 PVE/QemuServer.pm         | 24 +-----------------------
 PVE/QemuServer/Machine.pm | 20 ++++++++++++++++++++
 3 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2840de1b..8d0d788f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1246,7 +1246,8 @@ __PACKAGE__->register_method({
 		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
 			# always pin Windows' machine version on create, they get to easily confused
 			if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
-			    $machine_conf->{type} = PVE::QemuServer::windows_get_pinned_machine_version($machine);
+			    $machine_conf->{type} =
+				PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine);
 			    $conf->{machine} = PVE::QemuServer::Machine::print_machine($machine_conf);
 			}
 		    }
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ee7bb017..989fafbd 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,28 +3271,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub windows_get_pinned_machine_version {
-    my ($machine, $base_version, $kvmversion) = @_;
-
-    my $pin_version = $base_version;
-    if (!defined($base_version) ||
-	!PVE::QemuServer::Machine::can_run_pve_machine_version($base_version, $kvmversion)
-    ) {
-	$pin_version = PVE::QemuServer::Machine::get_installed_machine_version($kvmversion);
-    }
-    if (!$machine || $machine eq 'pc') {
-	$machine = "pc-i440fx-$pin_version";
-    } elsif ($machine eq 'q35') {
-	$machine = "pc-q35-$pin_version";
-    } elsif ($machine eq 'virt') {
-	$machine = "virt-$pin_version";
-    } else {
-	warn "unknown machine type '$machine', not touching that!\n";
-    }
-
-    return $machine;
-}
-
 sub get_vm_machine {
     my ($conf, $forcemachine, $arch) = @_;
 
@@ -3305,7 +3283,7 @@ sub get_vm_machine {
 	# layout which confuses windows quite a bit and may result in various regressions..
 	# see: https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
 	if (windows_version($conf->{ostype})) {
-	    $machine = windows_get_pinned_machine_version($machine, '5.1', $kvmversion);
+	    $machine = PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine, '5.1', $kvmversion);
 	}
 	$arch //= 'x86_64';
 	$machine ||= PVE::QemuServer::Machine::default_machine_for_arch($arch);
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 075554f4..04c77ed5 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -193,4 +193,24 @@ sub get_installed_machine_version {
     return $1;
 }
 
+sub windows_get_pinned_machine_version {
+    my ($machine, $base_version, $kvmversion) = @_;
+
+    my $pin_version = $base_version;
+    if (!defined($base_version) || !can_run_pve_machine_version($base_version, $kvmversion)) {
+	$pin_version = get_installed_machine_version($kvmversion);
+    }
+    if (!$machine || $machine eq 'pc') {
+	$machine = "pc-i440fx-$pin_version";
+    } elsif ($machine eq 'q35') {
+	$machine = "pc-q35-$pin_version";
+    } elsif ($machine eq 'virt') {
+	$machine = "virt-$pin_version";
+    } else {
+	warn "unknown machine type '$machine', not touching that!\n";
+    }
+
+    return $machine;
+}
+
 1;
-- 
2.39.5



_______________________________________________
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 10/18] move get_vm_machine() function to machine module

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm          |  2 +-
 PVE/QemuServer.pm         | 38 ++------------------------------------
 PVE/QemuServer/Machine.pm | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8d0d788f..868049cb 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -852,7 +852,7 @@ sub assert_scsi_feature_compatibility {
 
     my $drive = PVE::QemuServer::Drive::parse_drive($opt, $drive_attributes, 1);
 
-    my $machine_type = PVE::QemuServer::get_vm_machine($conf, undef, $conf->{arch});
+    my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, undef, $conf->{arch});
     my $machine_version = PVE::QemuServer::Machine::extract_version(
 	$machine_type, PVE::QemuServer::Helpers::kvm_user_version());
     my $drivetype = PVE::QemuServer::Drive::get_scsi_device_type(
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 989fafbd..04d251e0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,40 +3271,6 @@ sub vga_conf_has_spice {
     return $1 || 1;
 }
 
-sub get_vm_machine {
-    my ($conf, $forcemachine, $arch) = @_;
-
-    my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
-    my $machine = $forcemachine || $machine_conf->{type};
-
-    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
-	my $kvmversion //= kvm_user_version();
-	# we must pin Windows VMs without a specific version to 5.1, as 5.2 fixed a bug in ACPI
-	# layout which confuses windows quite a bit and may result in various regressions..
-	# see: https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
-	if (windows_version($conf->{ostype})) {
-	    $machine = PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine, '5.1', $kvmversion);
-	}
-	$arch //= 'x86_64';
-	$machine ||= PVE::QemuServer::Machine::default_machine_for_arch($arch);
-	my $pvever = PVE::QemuServer::Machine::get_pve_version($kvmversion);
-	$machine .= "+pve$pvever";
-    }
-
-    if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) {
-	my $is_pxe = $machine =~ m/^(.*?)\.pxe$/;
-	$machine = $1 if $is_pxe;
-
-	# for version-pinned machines that do not include a pve-version (e.g.
-	# pc-q35-4.1), we assume 0 to keep them stable in case we bump
-	$machine .= '+pve0';
-
-	$machine .= '.pxe' if $is_pxe;
-    }
-
-    return $machine;
-}
-
 sub get_ovmf_files($$$) {
     my ($arch, $efidisk, $smm) = @_;
 
@@ -3560,7 +3526,7 @@ sub config_to_command {
 	die "Detected old QEMU binary ('$kvmver', at least 5.0 is required)\n";
     }
 
-    my $machine_type = get_vm_machine($conf, $forcemachine, $arch);
+    my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, $forcemachine, $arch);
     my $machine_version = extract_version($machine_type, $kvmver);
     $kvm //= 1 if is_native_arch($arch);
 
@@ -4874,7 +4840,7 @@ sub vmconfig_hotplug_pending {
 
     my $defaults = load_defaults();
     my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
-    my $machine_type = get_vm_machine($conf, undef, $arch);
+    my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, undef, $arch);
 
     # commit values which do not have any impact on running VM first
     # Note: those option cannot raise errors, we we do not care about
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 04c77ed5..915913c0 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -213,4 +213,38 @@ sub windows_get_pinned_machine_version {
     return $machine;
 }
 
+sub get_vm_machine {
+    my ($conf, $forcemachine, $arch) = @_;
+
+    my $machine_conf = parse_machine($conf->{machine});
+    my $machine = $forcemachine || $machine_conf->{type};
+
+    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
+	my $kvmversion //= PVE::QemuServer::Helpers::kvm_user_version();
+	# we must pin Windows VMs without a specific version to 5.1, as 5.2 fixed a bug in ACPI
+	# layout which confuses windows quite a bit and may result in various regressions..
+	# see: https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
+	if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
+	    $machine = windows_get_pinned_machine_version($machine, '5.1', $kvmversion);
+	}
+	$arch //= 'x86_64';
+	$machine ||= default_machine_for_arch($arch);
+	my $pvever = get_pve_version($kvmversion);
+	$machine .= "+pve$pvever";
+    }
+
+    if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) {
+	my $is_pxe = $machine =~ m/^(.*?)\.pxe$/;
+	$machine = $1 if $is_pxe;
+
+	# for version-pinned machines that do not include a pve-version (e.g.
+	# pc-q35-4.1), we assume 0 to keep them stable in case we bump
+	$machine .= '+pve0';
+
+	$machine .= '.pxe' if $is_pxe;
+    }
+
+    return $machine;
+}
+
 1;
-- 
2.39.5



_______________________________________________
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 11/18] move meta information handling to its own module

[Fiona Ebner]

Like this, it can be used by modules that cannot depend on
QemuServer.pm without creating a cyclic dependency.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm           |  3 ++-
 PVE/QemuServer.pm          | 42 +++-------------------------------
 PVE/QemuServer/Makefile    |  1 +
 PVE/QemuServer/MetaInfo.pm | 47 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 40 deletions(-)
 create mode 100644 PVE/QemuServer/MetaInfo.pm

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 868049cb..60b8a4cc 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -35,6 +35,7 @@ use PVE::QemuServer::ImportDisk;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
+use PVE::QemuServer::MetaInfo;
 use PVE::QemuServer::PCI;
 use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer::USB;
@@ -1205,7 +1206,7 @@ __PACKAGE__->register_method({
 		    assert_scsi_feature_compatibility($opt, $conf, $storecfg, $param->{$opt});
 		}
 
-		$conf->{meta} = PVE::QemuServer::new_meta_info_string();
+		$conf->{meta} = PVE::QemuServer::MetaInfo::new_meta_info_string();
 
 		my $vollist = [];
 		eval {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 04d251e0..6f2a9aed 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -57,6 +57,7 @@ use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitne
 use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format 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);
+use PVE::QemuServer::MetaInfo;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port parse_hostpci);
 use PVE::QemuServer::QMPHelpers qw(qemu_deviceadd qemu_devicedel qemu_objectadd qemu_objectdel);
@@ -281,21 +282,6 @@ my $rng_fmt = {
     },
 };
 
-my $meta_info_fmt = {
-    'ctime' => {
-	type => 'integer',
-	description => "The guest creation timestamp as UNIX epoch time",
-	minimum => 0,
-	optional => 1,
-    },
-    'creation-qemu' => {
-	type => 'string',
-	description => "The QEMU (machine) version from the time this VM was created.",
-	pattern => '\d+(\.\d+)+',
-	optional => 1,
-    },
-};
-
 my $confdesc = {
     onboot => {
 	optional => 1,
@@ -729,7 +715,7 @@ EODESCR
     },
     meta => {
 	type => 'string',
-	format => $meta_info_fmt,
+	format => $PVE::QemuServer::MetaInfo::meta_info_fmt,
 	description => "Some (read-only) meta-information about this guest.",
 	optional => 1,
     },
@@ -2058,32 +2044,10 @@ sub parse_rng {
     return $res;
 }
 
-sub parse_meta_info {
-    my ($value) = @_;
-
-    return if !$value;
-
-    my $res = eval { parse_property_string($meta_info_fmt, $value) };
-    warn $@ if $@;
-    return $res;
-}
-
-sub new_meta_info_string {
-    my () = @_; # for now do not allow to override any value
-
-    return PVE::JSONSchema::print_property_string(
-	{
-	    'creation-qemu' => kvm_user_version(),
-	    ctime => "". int(time()),
-	},
-	$meta_info_fmt
-    );
-}
-
 sub qemu_created_version_fixups {
     my ($conf, $forcemachine, $kvmver) = @_;
 
-    my $meta = parse_meta_info($conf->{meta}) // {};
+    my $meta = PVE::QemuServer::MetaInfo::parse_meta_info($conf->{meta}) // {};
     my $forced_vers = PVE::QemuServer::Machine::extract_version($forcemachine);
 
     # check if we need to apply some handling for VMs that always use the latest machine version but
diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
index 89d12091..18fd13ea 100644
--- a/PVE/QemuServer/Makefile
+++ b/PVE/QemuServer/Makefile
@@ -7,6 +7,7 @@ SOURCES=PCI.pm		\
 	Helpers.pm	\
 	Monitor.pm	\
 	Machine.pm	\
+	MetaInfo.pm	\
 	CPUConfig.pm	\
 	CGroup.pm	\
 	Drive.pm	\
diff --git a/PVE/QemuServer/MetaInfo.pm b/PVE/QemuServer/MetaInfo.pm
new file mode 100644
index 00000000..a8cb6c5e
--- /dev/null
+++ b/PVE/QemuServer/MetaInfo.pm
@@ -0,0 +1,47 @@
+package PVE::QemuServer::MetaInfo;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema;
+
+use PVE::QemuServer::Helpers;
+
+our $meta_info_fmt = {
+    'ctime' => {
+	type => 'integer',
+	description => "The guest creation timestamp as UNIX epoch time",
+	minimum => 0,
+	optional => 1,
+    },
+    'creation-qemu' => {
+	type => 'string',
+	description => "The QEMU (machine) version from the time this VM was created.",
+	pattern => '\d+(\.\d+)+',
+	optional => 1,
+    },
+};
+
+sub parse_meta_info {
+    my ($value) = @_;
+
+    return if !$value;
+
+    my $res = eval { PVE::JSONSchema::parse_property_string($meta_info_fmt, $value) };
+    warn $@ if $@;
+    return $res;
+}
+
+sub new_meta_info_string {
+    my () = @_; # for now do not allow to override any value
+
+    return PVE::JSONSchema::print_property_string(
+	{
+	    'creation-qemu' => PVE::QemuServer::Helpers::kvm_user_version(),
+	    ctime => "". int(time()),
+	},
+	$meta_info_fmt,
+    );
+}
+
+1;
-- 
2.39.5



_______________________________________________
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 12/18] machine: get vm machine: fallback to creation QEMU version for windows starting with 9.1

[Fiona Ebner]

Starting from QEMU 9.1, pin to the creation version instead. Support
for machine version 5.1 is expected to drop with QEMU 11.1 and it
would still be good to handle Windows VMs that do not have explicit
machine version for whatever reason. For example, explicitly setting
the machine without a version on the CLI/API after creation is one way
to end up with such a machine.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer/Machine.pm | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 915913c0..679d69c7 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use PVE::QemuServer::Helpers;
+use PVE::QemuServer::MetaInfo;
 use PVE::QemuServer::Monitor;
 use PVE::JSONSchema qw(get_standard_option parse_property_string print_property_string);
 
@@ -221,11 +222,23 @@ sub get_vm_machine {
 
     if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
 	my $kvmversion //= PVE::QemuServer::Helpers::kvm_user_version();
-	# we must pin Windows VMs without a specific version to 5.1, as 5.2 fixed a bug in ACPI
-	# layout which confuses windows quite a bit and may result in various regressions..
+	# we must pin Windows VMs without a specific version and no meta info about creation QEMU to
+	# 5.1, as 5.2 fixed a bug in ACPI layout which confuses windows quite a bit and may result
+	# in various regressions..
 	# see: https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
+	# Starting from QEMU 9.1, pin to the creation version instead. Support for 5.1 is expected
+	# to drop with QEMU 11.1 and it would still be good to handle Windows VMs that do not have
+	# an explicit machine version for whatever reason.
 	if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
-	    $machine = windows_get_pinned_machine_version($machine, '5.1', $kvmversion);
+	    my $base_version = '5.1';
+	    # TODO PVE 10 - die early if there is a Windows VM both without explicit machine version
+	    # and without meta info.
+	    if (my $meta = PVE::QemuServer::MetaInfo::parse_meta_info($conf->{meta})) {
+		$base_version = $meta->{'creation-qemu'}
+		    if PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1);
+		($base_version) = ($base_version =~ m/^(\d+.\d+)/); # need only major.minor
+	    }
+	    $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion);
 	}
 	$arch //= 'x86_64';
 	$machine ||= default_machine_for_arch($arch);
-- 
2.39.5



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

Re: [pve-devel] [PATCH qemu-server v2 12/18] machine: get vm machine: fallback to creation QEMU version for windows starting with 9.1

[Thomas Lamprecht]

nit, if there will be a v3 I'd adapt the subject to:

machine: fallback to creation QEMU version for windows starting with 9.1

The additional tag did not provide much useful additional info IMO and
made the subject rather long.

Am 16.01.25 um 12:50 schrieb Fiona Ebner:
> Starting from QEMU 9.1, pin to the creation version instead. Support
> for machine version 5.1 is expected to drop with QEMU 11.1 and it
> would still be good to handle Windows VMs that do not have explicit
> machine version for whatever reason. For example, explicitly setting
> the machine without a version on the CLI/API after creation is one way
> to end up with such a machine.

But wouldn't the machine version jump from 5.1 to creation-QEMU with your
change for any VM that was created via CLI/API without explicit machine
version but with the code that adds the creation QEMU meta-info already
deployed?



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

Re: [pve-devel] [PATCH qemu-server v2 12/18] machine: get vm machine: fallback to creation QEMU version for windows starting with 9.1

[Fiona Ebner]

Am 17.01.25 um 10:19 schrieb Thomas Lamprecht:
> nit, if there will be a v3 I'd adapt the subject to:
> 
> machine: fallback to creation QEMU version for windows starting with 9.1
> 
> The additional tag did not provide much useful additional info IMO and
> made the subject rather long.
> 

Ack

> Am 16.01.25 um 12:50 schrieb Fiona Ebner:
>> Starting from QEMU 9.1, pin to the creation version instead. Support
>> for machine version 5.1 is expected to drop with QEMU 11.1 and it
>> would still be good to handle Windows VMs that do not have explicit
>> machine version for whatever reason. For example, explicitly setting
>> the machine without a version on the CLI/API after creation is one way
>> to end up with such a machine.
> 
> But wouldn't the machine version jump from 5.1 to creation-QEMU with your
> change for any VM that was created via CLI/API without explicit machine
> version but with the code that adds the creation QEMU meta-info already
> deployed?

No, the version will still be 5.1, except if the creation-QEMU version
is >= 9.1.

>> +	    my $base_version = '5.1';
>> +	    # TODO PVE 10 - die early if there is a Windows VM both without explicit machine version
>> +	    # and without meta info.
>> +	    if (my $meta = PVE::QemuServer::MetaInfo::parse_meta_info($conf->{meta})) {
>> +		$base_version = $meta->{'creation-qemu'}
>> +		    if PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1);
>> +		($base_version) = ($base_version =~ m/^(\d+.\d+)/); # need only major.minor
>> +	    }
>> +	    $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion);

The conditional might be better written up-front, not as a post-if and
also cover the regex matching expression.


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

Re: [pve-devel] [PATCH qemu-server v2 12/18] machine: get vm machine: fallback to creation QEMU version for windows starting with 9.1

[Thomas Lamprecht]

Am 17.01.25 um 10:27 schrieb Fiona Ebner:
> Am 17.01.25 um 10:19 schrieb Thomas Lamprecht:
>> But wouldn't the machine version jump from 5.1 to creation-QEMU with your
>> change for any VM that was created via CLI/API without explicit machine
>> version but with the code that adds the creation QEMU meta-info already
>> deployed?
> 
> No, the version will still be 5.1, except if the creation-QEMU version
> is >= 9.1.

ack, thanks for the hint. 
>>> +	    my $base_version = '5.1';
>>> +	    # TODO PVE 10 - die early if there is a Windows VM both without explicit machine version
>>> +	    # and without meta info.
>>> +	    if (my $meta = PVE::QemuServer::MetaInfo::parse_meta_info($conf->{meta})) {
>>> +		$base_version = $meta->{'creation-qemu'}
>>> +		    if PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1);
>>> +		($base_version) = ($base_version =~ m/^(\d+.\d+)/); # need only major.minor
>>> +	    }
>>> +	    $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion);
> 
> The conditional might be better written up-front, not as a post-if and
> also cover the regex matching expression.

Ah yes, that probably would have helped me grasping this quicker.


_______________________________________________
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 13/18] machine: add check_and_pin_machine_string() helper

[Fiona Ebner]

Extract the logic for guest OS-type dependent machine version pinning
into a dedicated helper, so it can be re-used.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm          | 14 +++-----------
 PVE/QemuServer/Machine.pm | 16 ++++++++++++++++
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 60b8a4cc..b1410df6 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1242,17 +1242,9 @@ __PACKAGE__->register_method({
 			$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
 		    }
 
-		    my $machine_conf = PVE::QemuServer::Machine::parse_machine($conf->{machine});
-		    my $machine = $machine_conf->{type};
-		    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
-			# always pin Windows' machine version on create, they get to easily confused
-			if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
-			    $machine_conf->{type} =
-				PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine);
-			    $conf->{machine} = PVE::QemuServer::Machine::print_machine($machine_conf);
-			}
-		    }
-		    PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
+		    # always pin Windows' machine version on create, they get confused too easily
+		    $conf->{machine} = PVE::QemuServer::Machine::check_and_pin_machine_string(
+			$conf->{machine}, $conf->{ostype});
 
 		    $conf->{lock} = 'import' if $live_import_mapping;
 
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 679d69c7..cf00da6d 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -260,4 +260,20 @@ sub get_vm_machine {
     return $machine;
 }
 
+sub check_and_pin_machine_string {
+    my ($machine_string, $ostype) = @_;
+
+    my $machine_conf = parse_machine($machine_string);
+    my $machine = $machine_conf->{type};
+    if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
+	# always pin Windows' machine version on create, they get confused too easily
+	if (PVE::QemuServer::Helpers::windows_version($ostype)) {
+	    $machine_conf->{type} = windows_get_pinned_machine_version($machine);
+	}
+    }
+
+    assert_valid_machine_property($machine_conf);
+    return print_machine($machine_conf);
+}
+
 1;
-- 
2.39.5



_______________________________________________
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 14/18] api: update vm config: pin machine version when switching to windows os type

[Fiona Ebner]

During virtual machine creation, the machine version is pinned when
the guest OS is Windows. The same should be done when the guest OS
type is newly set to Windows for consistency.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b1410df6..8acd8d9f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2109,6 +2109,24 @@ my $update_vm_api  = sub {
 		    my $machine_conf = PVE::QemuServer::Machine::parse_machine($param->{$opt});
 		    PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
 		    $conf->{pending}->{$opt} = $param->{$opt};
+		} elsif ($opt eq 'ostype') {
+		    # Check if machine version pinning is needed when switching OS type, just like
+		    # upon creation. Skip if 'machine' is explicitly set or removed at the same time
+		    # to honor the users request. While it should be enough to look at $modified,
+		    # because 'machine' is sorted before 'ostype', be explicit just to be sure.
+		    if (
+			!defined($param->{machine})
+			&& !defined($conf->{pending}->{machine})
+			&& !$modified->{machine} # detects deletion
+		    ) {
+			eval {
+			    $conf->{pending}->{machine} =
+				PVE::QemuServer::Machine::check_and_pin_machine_string(
+				    $conf->{machine}, $param->{ostype});
+			};
+			print "automatic pinning of machine version failed - $@" if $@;
+		    }
+		    $conf->{pending}->{$opt} = $param->{$opt};
 		} elsif ($opt eq 'cipassword') {
 		    if (!PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
 			# Same logic as in cloud-init (but with the regex fixed...)
-- 
2.39.5



_______________________________________________
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 15/18] machine: log informational line when pinning machine version for Windows guest

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer/Machine.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index cf00da6d..9b18cf6e 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -269,6 +269,7 @@ sub check_and_pin_machine_string {
 	# always pin Windows' machine version on create, they get confused too easily
 	if (PVE::QemuServer::Helpers::windows_version($ostype)) {
 	    $machine_conf->{type} = windows_get_pinned_machine_version($machine);
+	    print "pinning machine type to '$machine_conf->{type}' for Windows guest OS\n";
 	}
     }
 
-- 
2.39.5



_______________________________________________
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 16/18] machine: rename machine_version() function to machine_version_at_least()

[Fiona Ebner]

The old name does not make it clear what exactly the function does.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm         | 4 ++--
 PVE/QemuServer/Machine.pm | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6f2a9aed..58d23533 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4532,7 +4532,7 @@ sub qemu_cpu_hotplug {
 
     if ($vcpus < $currentvcpus) {
 
-	if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
+	if (PVE::QemuServer::Machine::machine_version_at_least($machine_type, 2, 7)) {
 
 	    for (my $i = $currentvcpus; $i > $vcpus; $i--) {
 		qemu_devicedel($vmid, "cpu$i");
@@ -4560,7 +4560,7 @@ sub qemu_cpu_hotplug {
     die "vcpus in running vm does not match its configuration\n"
 	if scalar(@{$currentrunningvcpus}) != $currentvcpus;
 
-    if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
+    if (PVE::QemuServer::Machine::machine_version_at_least($machine_type, 2, 7)) {
 	my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
 
 	for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 9b18cf6e..6398e756 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -136,7 +136,7 @@ sub extract_version {
     return;
 }
 
-sub machine_version {
+sub machine_version_at_least {
     my ($machine_type, $major, $minor, $pve) = @_;
 
     return PVE::QemuServer::Helpers::min_version(
-- 
2.39.5



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

Re: [pve-devel] [PATCH qemu-server v2 16/18] machine: rename machine_version() function to machine_version_at_least()

[Thomas Lamprecht]

Am 16.01.25 um 12:50 schrieb Fiona Ebner:
> The old name does not make it clear what exactly the function does.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuServer.pm         | 4 ++--
>  PVE/QemuServer/Machine.pm | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6f2a9aed..58d23533 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4532,7 +4532,7 @@ sub qemu_cpu_hotplug {
>  
>      if ($vcpus < $currentvcpus) {
>  
> -	if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
> +	if (PVE::QemuServer::Machine::machine_version_at_least($machine_type, 2, 7)) {
>  
>  	    for (my $i = $currentvcpus; $i > $vcpus; $i--) {
>  		qemu_devicedel($vmid, "cpu$i");
> @@ -4560,7 +4560,7 @@ sub qemu_cpu_hotplug {
>      die "vcpus in running vm does not match its configuration\n"
>  	if scalar(@{$currentrunningvcpus}) != $currentvcpus;
>  
> -    if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
> +    if (PVE::QemuServer::Machine::machine_version_at_least($machine_type, 2, 7)) {
>  	my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
>  
>  	for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> index 9b18cf6e..6398e756 100644
> --- a/PVE/QemuServer/Machine.pm
> +++ b/PVE/QemuServer/Machine.pm
> @@ -136,7 +136,7 @@ sub extract_version {
>      return;
>  }
>  
> -sub machine_version {
> +sub machine_version_at_least {

maybe use the chance and prefix with is_ to make this even more clear, i.e.
is_version_at_least or to be very verbose: is_version_greater_or_equal (no
hard feelings there though).


>      my ($machine_type, $major, $minor, $pve) = @_;
>  
>      return PVE::QemuServer::Helpers::min_version(



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

Re: [pve-devel] [PATCH qemu-server v2 16/18] machine: rename machine_version() function to machine_version_at_least()

[Fiona Ebner]

Am 17.01.25 um 13:39 schrieb Thomas Lamprecht:
> Am 16.01.25 um 12:50 schrieb Fiona Ebner:
>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
>> index 9b18cf6e..6398e756 100644
>> --- a/PVE/QemuServer/Machine.pm
>> +++ b/PVE/QemuServer/Machine.pm
>> @@ -136,7 +136,7 @@ sub extract_version {
>>      return;
>>  }
>>  
>> -sub machine_version {
>> +sub machine_version_at_least {
> 
> maybe use the chance and prefix with is_ to make this even more clear, i.e.
> is_version_at_least or to be very verbose: is_version_greater_or_equal (no
> hard feelings there though).

Okay, but I'd rather not drop the "machine" from the function name. We
have a lot of different versions floating around so even though this is
already in the machine module, I'd still like to mention it explicitly
in the function name to reduce potential for accidental re-use for other
kinds of versions.


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

Re: [pve-devel] [PATCH qemu-server v2 16/18] machine: rename machine_version() function to machine_version_at_least()

[Thomas Lamprecht]

Am 17.01.25 um 14:33 schrieb Fiona Ebner:
> Am 17.01.25 um 13:39 schrieb Thomas Lamprecht:
>> Am 16.01.25 um 12:50 schrieb Fiona Ebner:
>>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
>>> index 9b18cf6e..6398e756 100644
>>> --- a/PVE/QemuServer/Machine.pm
>>> +++ b/PVE/QemuServer/Machine.pm
>>> @@ -136,7 +136,7 @@ sub extract_version {
>>>      return;
>>>  }
>>>  
>>> -sub machine_version {
>>> +sub machine_version_at_least {
>>
>> maybe use the chance and prefix with is_ to make this even more clear, i.e.
>> is_version_at_least or to be very verbose: is_version_greater_or_equal (no
>> hard feelings there though).
> 
> Okay, but I'd rather not drop the "machine" from the function name. We
> have a lot of different versions floating around so even though this is
> already in the machine module, I'd still like to mention it explicitly
> in the function name to reduce potential for accidental re-use for other
> kinds of versions.

Yeah, I actually agree here 100%, that was just an oversight on my side
when writing this.


_______________________________________________
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 17/18] machine: code cleanup: avoid superfluous augmented assignment operator

[Fiona Ebner]

Suggested by perlcritic.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer/Machine.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 6398e756..7b7479d1 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -221,7 +221,7 @@ sub get_vm_machine {
     my $machine = $forcemachine || $machine_conf->{type};
 
     if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
-	my $kvmversion //= PVE::QemuServer::Helpers::kvm_user_version();
+	my $kvmversion = PVE::QemuServer::Helpers::kvm_user_version();
 	# we must pin Windows VMs without a specific version and no meta info about creation QEMU to
 	# 5.1, as 5.2 fixed a bug in ACPI layout which confuses windows quite a bit and may result
 	# in various regressions..
-- 
2.39.5



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

[pve-devel] [PATCH docs v2 18/18] qm: machine version: document support in PVE

[Fiona Ebner]

Elaborate on new QEMU machine version removal policy and how PVE will
support machine versions. Make sure to also mention the years, so that
users immediately have a good idea for how long it will be.

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

Changes in v2:
* get rid of outdated information from "Update to a Newer Machine
  Version" section
* drop sentence about early pre-deprecation warning (doesn't exist
  anymore)

 qm.adoc | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/qm.adoc b/qm.adoc
index 94fdd4e..9d8becf 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -173,19 +173,42 @@ This means that after a fresh start, the newest machine version supported by the
 QEMU binary is used (e.g. the newest machine version QEMU 8.1 supports is
 version 8.1 for each machine type).
 
+Starting with QEMU 10.1, machine versions will be removed from upstream QEMU
+after 6 years. In {pve}, major releases happen approximately every 2 years, so a
+major {pve} release will support machine versions from approximately two
+previous major {pve} releases, more details below.
+
+Before upgrading to a new major {pve} release, you should update VM
+configurations to avoid all machine versions that will be dropped during the
+next major {pve} release. This ensures that the guests can still be used
+throughout that release. See the section
+xref:qm_machine_update[Update to a Newer Machine Version].
+
+The following table shows the expected baselines of supported machine versions
+for the current and upcoming major {pve} releases (best guesses):
+
+[width="100%",cols=">s,>,>s,2*>",options="header"]
+|===============================================================================================
+| {pve} | active development | supported baseline | dropped during life cycle | last QEMU binary
+|     8 |          2023-2025 |                2.4 |             2.3 and older |              9.2
+|     9 |          2025-2027 |                6.0 |             5.2 and older |             11.2
+|    10 |          2027-2029 |                8.0 |             7.2 and older |             13.2
+|===============================================================================================
+
+NOTE: Support for {pve} releases is longer than active development, but no new
+QEMU binary versions will be added after active development, just backports and
+fixes for existing binary versions.
+
 [[qm_machine_update]]
 
 Update to a Newer Machine Version
 +++++++++++++++++++++++++++++++++
 
-Very old machine versions might become deprecated in QEMU. For example, this is
-the case for versions 1.4 to 1.7 for the i440fx machine type. It is expected
-that support for these machine versions will be dropped at some point. If you
-see a deprecation warning, you should change the machine version to a newer one.
-Be sure to have a working backup first and be prepared for changes to how the
-guest sees hardware. In some scenarios, re-installing certain drivers might be
-required. You should also check for snapshots with RAM that were taken with
-these machine versions (i.e. the `runningmachine` configuration entry).
+If you see a deprecation warning, you should change the machine version to a
+newer one. Be sure to have a working backup first and be prepared for changes to
+how the guest sees hardware. In some scenarios, re-installing certain drivers
+might be required. You should also check for snapshots with RAM that were taken
+with these machine versions (i.e. the `runningmachine` configuration entry).
 Unfortunately, there is no way to change the machine version of a snapshot, so
 you'd need to load the snapshot to salvage any data from it.
 
-- 
2.39.5



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

Re: [pve-devel] [PATCH docs v2 18/18] qm: machine version: document support in PVE

[Thomas Lamprecht]

Am 16.01.25 um 12:50 schrieb Fiona Ebner:
> Elaborate on new QEMU machine version removal policy and how PVE will
> support machine versions. Make sure to also mention the years, so that
> users immediately have a good idea for how long it will be.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes in v2:
> * get rid of outdated information from "Update to a Newer Machine
>   Version" section
> * drop sentence about early pre-deprecation warning (doesn't exist
>   anymore)
> 
>  qm.adoc | 39 +++++++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/qm.adoc b/qm.adoc
> index 94fdd4e..9d8becf 100644
> --- a/qm.adoc
> +++ b/qm.adoc
> @@ -173,19 +173,42 @@ This means that after a fresh start, the newest machine version supported by the
>  QEMU binary is used (e.g. the newest machine version QEMU 8.1 supports is
>  version 8.1 for each machine type).
>  

Maybe make this a separate section and add a `QEMU Machine Version Depreaction`
heading, or the like, here?

> +Starting with QEMU 10.1, machine versions will be removed from upstream QEMU

opinionated nit: s/will be/are/

> +after 6 years. In {pve}, major releases happen approximately every 2 years, so a
> +major {pve} release will support machine versions from approximately two
> +previous major {pve} releases, more details below.

I'd drop the ", more details below." or if you really think it helps then link
to the respective paragraph, as else I as reader get distracted with searching
where this is explained exactly.

> +
> +Before upgrading to a new major {pve} release, you should update VM
> +configurations to avoid all machine versions that will be dropped during the
> +next major {pve} release. This ensures that the guests can still be used
> +throughout that release. See the section
> +xref:qm_machine_update[Update to a Newer Machine Version].
> +
> +The following table shows the expected baselines of supported machine versions
> +for the current and upcoming major {pve} releases (best guesses):

Maybe avoid a table with all releases listed (and the note below) and instead
write a specific example, e.g. for PVE 9, mixed with a description how the
decision/cut-off process works – the table can be surely nice for some, but
probably also not trivial to interpret for all users, and it might get outdated,
especially if we got something like total 5 year (LTS) life span, which is not
very likely very soon, but also not completely out of the question.

Or just a list of the expected base-line, maybe we could generate that through
some small perl script for current - 1, current and current + 1 release and
include that here? Just as an idea, I do not want to inflate the scope to much ^^

> +
> +[width="100%",cols=">s,>,>s,2*>",options="header"]
> +|===============================================================================================
> +| {pve} | active development | supported baseline | dropped during life cycle | last QEMU binary

nit: if, I'd title-case the column titles, I normally use AP style for that and
if unsure I run the title through [0].

[0]: https://titlecaseconverter.com/

Also, "dropped during life cycle" seems a bit redundant to me, a sentence above
or below that states that all machine versions below the baseline are not
supported anymore would be enough.

> +|     8 |          2023-2025 |                2.4 |             2.3 and older |              9.2
> +|     9 |          2025-2027 |                6.0 |             5.2 and older |             11.2
> +|    10 |          2027-2029 |                8.0 |             7.2 and older |             13.2
> +|===============================================================================================


> +
> +NOTE: Support for {pve} releases is longer than active development, but no new
> +QEMU binary versions will be added after active development, just backports and
> +fixes for existing binary versions.
> +
>  [[qm_machine_update]]
>  
>  Update to a Newer Machine Version
>  +++++++++++++++++++++++++++++++++
>  
> -Very old machine versions might become deprecated in QEMU. For example, this is
> -the case for versions 1.4 to 1.7 for the i440fx machine type. It is expected
> -that support for these machine versions will be dropped at some point. If you
> -see a deprecation warning, you should change the machine version to a newer one.
> -Be sure to have a working backup first and be prepared for changes to how the
> -guest sees hardware. In some scenarios, re-installing certain drivers might be
> -required. You should also check for snapshots with RAM that were taken with
> -these machine versions (i.e. the `runningmachine` configuration entry).
> +If you see a deprecation warning, you should change the machine version to a
> +newer one. Be sure to have a working backup first and be prepared for changes to
> +how the guest sees hardware. In some scenarios, re-installing certain drivers
> +might be required. You should also check for snapshots with RAM that were taken
> +with these machine versions (i.e. the `runningmachine` configuration entry).
>  Unfortunately, there is no way to change the machine version of a snapshot, so
>  you'd need to load the snapshot to salvage any data from it.
>  



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