From f289e04cbf3271d06651cb7fdf3c145e0cdebac0 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Sun, 6 Apr 2025 19:02:00 +0200 Subject: [PATCH] pve machine versions: unify information into single hash and rework use sites It's a bit nicer to avoid multiple hashes, this keeps the information also closer together, albeit that alone wasn't the reason for doing this because this (hopefully) will never grow that big, rather it should encourage adding changes entries until we test for that and it also makes handling these entries a tiny bit more natural as no wrapper counter for-loops doing integer string interpolation are required. While at it expand the comment, might be even a tad to verbose now, and add a new helper to get these per-machine revisions. Signed-off-by: Thomas Lamprecht --- PVE/API2/Qemu/Machine.pm | 23 +++++++------- PVE/QemuServer/Machine.pm | 63 +++++++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 35 deletions(-) diff --git a/PVE/API2/Qemu/Machine.pm b/PVE/API2/Qemu/Machine.pm index d5af0d2c..05b47cc9 100644 --- a/PVE/API2/Qemu/Machine.pm +++ b/PVE/API2/Qemu/Machine.pm @@ -59,27 +59,26 @@ __PACKAGE__->register_method({ my $raw = file_get_contents('/usr/share/kvm/machine-versions-x86_64.json'); my $machines = from_json($raw, { utf8 => 1 }); - my $to_add = []; - + my $pve_machines = []; for my $machine ($machines->@*) { - my $base_version = $machine->{version}; - my $pvever = PVE::QemuServer::Machine::get_pve_version($base_version); - for (my $i = 1; $i <= $pvever; $i++) { - my $version = $base_version . "+pve$i"; + my $pve_machine = PVE::QemuServer::Machine::get_machine_pve_revisions($machine->{version}) or next; + + for my $pve_revision (sort keys $pve_machine->{revisions}->%*) { my $entry = { - id => $machine->{id} . "+pve$i", + id => $machine->{id} . $pve_revision, type => $machine->{type}, - version => $version, + version => $machine->{version} . $pve_revision, }; - my $desc = PVE::QemuServer::Machine::get_pve_version_description($version); - $entry->{changes} = $desc if defined($desc); + if (defined(my $changes = $pve_machine->{revisions}->{$pve_revision})) { + $entry->{changes} = $changes; + } - push $to_add->@*, $entry; + push $pve_machines->@*, $entry; } } - return [ sort { $b->{id} cmp $a->{id} } ($machines->@*, $to_add->@*) ]; # merge & sort + return [ sort { $b->{id} cmp $a->{id} } ($machines->@*, $pve_machines->@*) ]; # merge & sort }; die "could not load supported machine versions - $@\n" if $@; return $supported_machine_list; diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index 2056aad0..b04cf99d 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -8,28 +8,37 @@ use PVE::QemuServer::MetaInfo; use PVE::QemuServer::Monitor; use PVE::JSONSchema qw(get_standard_option parse_property_string print_property_string); -# Bump this for VM HW layout changes during a release (where the QEMU machine -# version stays the same) +# The PVE machine versions allow rolling out (incompatibel) changes to the hardware layout and/or +# the QEMU command of a VM without requiring a newer QEMU upstream machine version. +# +# To use this find the newest available QEMU machine version, add and entry in this hash if it does +# not already exists and then bump the higherst version, or introduce a new one starting at 1, as +# the upstream version is counted as having a PVE revision of 0. +# Additionally you must describe in short what the basic changes done with such a new PVE machine +# revision in the respective subhash, use the full version including the pve one as key there. +# +# NOTE: Do not overuse this. one or two changes per upstream machine can be fine, if needed. But +# most of the time it's better to batch more together and if there is no time pressure then wait a +# few weeks/months until the next QEMU machine revision is ready. As it will get confusing otherwise +# and we lazily use some simple ascii sort when processing these, so more than 10 per entry require +# changes but should be avoided in the first place. +# TODO: add basic test to ensure the keys are correct and there's a change entry for each version. our $PVE_MACHINE_VERSION = { - '4.1' => 2, - '9.2' => 1, + '4.1' => { + highest => 2, + revisions => { + '+pve1' => 'Introduction of pveX versioning, no changes.', + '+pve2' => 'Increases the number of SCSI drives supported.', + }, + }, + '9.2' => { + highest => 1, + revisions => { + '+pve1' => 'Disables S3/S4 power states by default.', + }, + }, }; -# When bumping the pveX version, add a description why. -my $PVE_MACHINE_VERSION_DESCRIPTIONS = { - '4.1+pve1' => 'Introduction of pveX versioning, no changes.', - '4.1+pve2' => 'Increases supported SCSI drive count.', - '9.2+pve1' => 'Disables S3/S4 power states. These are often problematic in virtualized guests.', -}; - -# returns the description of a given machine version with pve version, e.g. 9.2+pve1 or undef if -# there is none -sub get_pve_version_description { - my ($version) = @_; - - return $PVE_MACHINE_VERSION_DESCRIPTIONS->{$version}; -} - my $machine_fmt = { type => { default_key => 1, @@ -169,14 +178,22 @@ sub is_machine_version_at_least { extract_version($machine_type), $major, $minor, $pve); } +sub get_machine_pve_revisions { + my ($machine_version_str) = @_; + + if ($machine_version_str =~ m/^(\d+\.\d+)/) { + return $PVE_MACHINE_VERSION->{$1}; + } + + die "internal error: cannot get pve version for invalid string '$machine_version_str'"; +} + sub get_pve_version { my ($verstr) = @_; - if ($verstr =~ m/^(\d+\.\d+)/) { - return $PVE_MACHINE_VERSION->{$1} // 0; - } + my $pve_machine = get_machine_pve_revisions($verstr); - die "internal error: cannot get pve version for invalid string '$verstr'"; + return $pve_machine->{highest} // 0; } sub can_run_pve_machine_version {