[pve-devel] [PATCH-SERIES docs/qemu-server] machine version information

[pve-devel] [PATCH-SERIES docs/qemu-server] machine version information

[Fiona Ebner]

There are already some deprecated machine versions in QEMU currently,
namely 1.4-1.7 for i440fx and QEMU 8.2 will add some more. At some
point, support for these will be dropped completely, so start
informing and warning users about this.


docs:

Fiona Ebner (1):
  qm: add section about machine types

 qm.adoc | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)


qemu-server:

Fiona Ebner (6):
  machine: get current: improve naming and style
  machine: get current: make it clear that pve-version only exists for
    the current machine
  machine: get current: return early from loop if possible
  machine: get current: add flag if current machine is deprecated in
    list context
  test: migration: mock get_current_qemu_machine
  vm start: add warning about deprecated machine version

 PVE/QemuServer.pm            |  9 +++++++++
 PVE/QemuServer/Machine.pm    | 28 +++++++++++++++++-----------
 test/MigrationTest/QmMock.pm | 14 ++++++++++++++
 3 files changed, 40 insertions(+), 11 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH docs 1/1] qm: add section about machine types

[Fiona Ebner]

expanding from the two currently existing sentences. In the first one, a
typo VMs -> VM's is fixed. In the second one, "one wants to" is changed
to "you want to", because the sentence already starts with "You can" and
it's active voice.

Adds information about the machine version, rationale behind the
defaults for it and what to do for deprecated versions.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 qm.adoc | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/qm.adoc b/qm.adoc
index b7938d7..2f48969 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -139,12 +139,44 @@ snapshots) more intelligently.
 {pve} allows to boot VMs with different firmware and machine types, namely
 xref:qm_bios_and_uefi[SeaBIOS and OVMF]. In most cases you want to switch from
 the default SeaBIOS to OVMF only if you plan to use
-xref:qm_pci_passthrough[PCIe passthrough]. A VMs 'Machine Type' defines the
-hardware layout of the VM's virtual motherboard. You can choose between the
-default https://en.wikipedia.org/wiki/Intel_440FX[Intel 440FX] or the
+xref:qm_pci_passthrough[PCIe passthrough].
+
+Machine Type
+^^^^^^^^^^^^
+
+A VM's 'Machine Type' defines the hardware layout of the VM's virtual
+motherboard. You can choose between the default
+https://en.wikipedia.org/wiki/Intel_440FX[Intel 440FX] or the
 https://ark.intel.com/content/www/us/en/ark/products/31918/intel-82q35-graphics-and-memory-controller.html[Q35]
-chipset, which also provides a virtual PCIe bus, and thus may be desired if
-one wants to pass through PCIe hardware.
+chipset, which also provides a virtual PCIe bus, and thus may be
+desired if you want to pass through PCIe hardware.
+
+Each machine type is versioned in QEMU and a given QEMU binary supports many
+machine versions. New versions might bring support for new features, fixes or
+general improvements. However, they also change properties of the virtual
+hardware. To avoid sudden changes from the guest's perspective and ensure
+compatibility of the VM state, live-migration and snapshots with RAM will keep
+using the same machine version in the new QEMU instance.
+
+For Windows guests, the machine version is pinned during creation, because
+Windows is sensitive to changes in the virtual hardware - even between cold
+boots. For example, the enumeration of network devices might be different with
+different machine versions. Other OSes like Linux can usually deal with such
+changes just fine. For those, the 'Latest' machine version is used by default.
+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).
+
+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).
+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.
 
 [[qm_hard_disk]]
 Hard Disk
-- 
2.39.2

[pve-devel] [PATCH qemu-server 1/6] machine: get current: improve naming and style

[Fiona Ebner]

No functional change intended.

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

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index d9429ed4..a4bc24a5 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -19,13 +19,13 @@ sub machine_type_is_q35 {
 }
 
 sub current_from_query_machines {
-    my ($res) = @_;
+    my ($machines) = @_;
 
     my ($current, $pve_version, $default);
-    foreach my $e (@$res) {
-	$default = $e->{name} if $e->{'is-default'};
-	$current = $e->{name} if $e->{'is-current'};
-	$pve_version = $e->{'pve-version'} if $e->{'pve-version'};
+    for my $machine ($machines->@*) {
+	$default = $machine->{name} if $machine->{'is-default'};
+	$current = $machine->{name} if $machine->{'is-current'};
+	$pve_version = $machine->{'pve-version'} if $machine->{'pve-version'};
     }
 
     $current .= "+$pve_version" if $current && $pve_version;
-- 
2.39.2

[pve-devel] [PATCH qemu-server 2/6] machine: get current: make it clear that pve-version only exists for the current machine

[Fiona Ebner]

by adding a comment and grouping the code better. See the PVE QEMU
patch "PVE: Allow version code in machine type" for reference. The way
the code was written previously made it look like a bug where
$pve_version might be overwritten multiple times.

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

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index a4bc24a5..85cfb89c 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -21,14 +21,16 @@ sub machine_type_is_q35 {
 sub current_from_query_machines {
     my ($machines) = @_;
 
-    my ($current, $pve_version, $default);
+    my ($current, $default);
     for my $machine ($machines->@*) {
 	$default = $machine->{name} if $machine->{'is-default'};
-	$current = $machine->{name} if $machine->{'is-current'};
-	$pve_version = $machine->{'pve-version'} if $machine->{'pve-version'};
-    }
 
-    $current .= "+$pve_version" if $current && $pve_version;
+	if ($machine->{'is-current'}) {
+	    $current = $machine->{name};
+	    # pve-version only exists for the current machine
+	    $current .= "+$machine->{'pve-version'}" if $machine->{'pve-version'};
+	}
+    }
 
     # fallback to the default machine if current is not supported by qemu
     return $current || $default || 'pc';
-- 
2.39.2

[pve-devel] [PATCH qemu-server 3/6] machine: get current: return early from loop if possible

[Fiona Ebner]

No point iterating through the rest if we already got the current
machine.

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

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 85cfb89c..c9fc9a3d 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -29,11 +29,12 @@ sub current_from_query_machines {
 	    $current = $machine->{name};
 	    # pve-version only exists for the current machine
 	    $current .= "+$machine->{'pve-version'}" if $machine->{'pve-version'};
+	    return $current;
 	}
     }
 
     # fallback to the default machine if current is not supported by qemu
-    return $current || $default || 'pc';
+    return $default || 'pc';
 }
 
 # this only works if VM is running
-- 
2.39.2

[pve-devel] [PATCH qemu-server 4/6] machine: get current: add flag if current machine is deprecated in list context

[Fiona Ebner]

Will be used for a warning.

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

diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index c9fc9a3d..13721ae8 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -18,6 +18,7 @@ sub machine_type_is_q35 {
     return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0;
 }
 
+# In list context, also returns whether the current machine is deprecated or not.
 sub current_from_query_machines {
     my ($machines) = @_;
 
@@ -29,15 +30,17 @@ sub current_from_query_machines {
 	    $current = $machine->{name};
 	    # pve-version only exists for the current machine
 	    $current .= "+$machine->{'pve-version'}" if $machine->{'pve-version'};
-	    return $current;
+	    return wantarray ? ($current, $machine->{deprecated} ? 1 : 0) : $current;
 	}
     }
 
-    # fallback to the default machine if current is not supported by qemu
-    return $default || 'pc';
+    # fallback to the default machine if current is not supported by qemu - assume never deprecated
+    my $fallback = $default || 'pc';
+    return wantarray ? ($fallback, 0) : $fallback;
 }
 
-# this only works if VM is running
+# This only works if VM is running.
+# In list context, also returns whether the current machine is deprecated or not.
 sub get_current_qemu_machine {
     my ($vmid) = @_;
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server 5/6] test: migration: mock get_current_qemu_machine

[Fiona Ebner]

by remembering the 'forcemachine' parameter that's passed along when
starting the target instance.

In preparation to introduce a call to get_current_qemu_machine after
starting a VM to check for machine version deprecation.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 test/MigrationTest/QmMock.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/test/MigrationTest/QmMock.pm b/test/MigrationTest/QmMock.pm
index e34686e3..fb94c58b 100644
--- a/test/MigrationTest/QmMock.pm
+++ b/test/MigrationTest/QmMock.pm
@@ -23,6 +23,7 @@ my $migrate_params = decode_json(file_get_contents("${RUN_DIR_PATH}/migrate_para
 my $nodename = $migrate_params->{target};
 
 my $kvm_exectued = 0;
+my $forcemachine;
 
 sub setup_environment {
     my $rpcenv = PVE::RPCEnvironment::init('MigrationTest::QmMock', 'cli');
@@ -56,6 +57,12 @@ $MigrationTest::Shared::qemu_server_module->mock(
     config_to_command => sub {
 	return [ 'mocked_kvm_command' ];
     },
+    vm_start_nolock => sub {
+	my ($storecfg, $vmid, $conf, $params, $migrate_opts) = @_;
+	$forcemachine = $params->{forcemachine}
+	    or die "mocked vm_start_nolock - expected 'forcemachine' parameter\n";
+	$MigrationTest::Shared::qemu_server_module->original('vm_start_nolock')->(@_);
+    },
 );
 
 my $qemu_server_helpers_module = Test::MockModule->new("PVE::QemuServer::Helpers");
@@ -65,6 +72,13 @@ $qemu_server_helpers_module->mock(
     },
 );
 
+our $qemu_server_machine_module = Test::MockModule->new("PVE::QemuServer::Machine");
+$qemu_server_machine_module->mock(
+    get_current_qemu_machine => sub {
+	return wantarray ? ($forcemachine, 0) : $forcemachine;
+    },
+);
+
 # to make sure we get valid and predictable names
 my $disk_counter = 10;
 
-- 
2.39.2

[pve-devel] [PATCH qemu-server 6/6] vm start: add warning about deprecated machine version

[Fiona Ebner]

While there already is a warning from QEMU proper, that one is not
visible as a task warning and it's not straightforward to make it be
one, because QEMU is started inside a run_fork(). It's also more
future-proof to have the detection explicit on our side and the
documentation can be referenced.

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

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index dbcd5685..a60df3a1 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6014,6 +6014,15 @@ sub vm_start_nolock {
 
     PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-start');
 
+    my ($current_machine, $is_deprecated) =
+	PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
+    if ($is_deprecated) {
+	log_warn(
+	    "current machine version '$current_machine' is deprecated - see the documentation and ".
+	    "change to a newer one",
+	);
+    }
+
     return $res;
 }
 
-- 
2.39.2

[pve-devel] applied-series: [PATCH-SERIES docs/qemu-server] machine version information

[Thomas Lamprecht]

Am 10/11/2023 um 14:24 schrieb Fiona Ebner:
> There are already some deprecated machine versions in QEMU currently,
> namely 1.4-1.7 for i440fx and QEMU 8.2 will add some more. At some
> point, support for these will be dropped completely, so start
> informing and warning users about this.
> 
> 
> docs:
> 
> Fiona Ebner (1):
>   qm: add section about machine types
> 
>  qm.adoc | 42 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> 
> qemu-server:
> 
> Fiona Ebner (6):
>   machine: get current: improve naming and style
>   machine: get current: make it clear that pve-version only exists for
>     the current machine
>   machine: get current: return early from loop if possible
>   machine: get current: add flag if current machine is deprecated in
>     list context
>   test: migration: mock get_current_qemu_machine
>   vm start: add warning about deprecated machine version
> 
>  PVE/QemuServer.pm            |  9 +++++++++
>  PVE/QemuServer/Machine.pm    | 28 +++++++++++++++++-----------
>  test/MigrationTest/QmMock.pm | 14 ++++++++++++++
>  3 files changed, 40 insertions(+), 11 deletions(-)
> 


applied, thanks!

I added a few sub-headings and references for the docs patch.
And we might need to do a bigger restructuring there, currently we describe
the options as encountered in the web UI's VM create wizard, which on it's
own is fine to do as a short overview, but we now have everything related to
every option there too. OTOH, a simple split might make some important things
harder to find, but having all cobbled together does too – as you see, no good
solution jumped into my face, so just mentioning it for the record, maybe some
body else got a better idea (or can spent more time on this), it's not a big
issue yet after all.