[pve-devel] applied: [PATCH qemu-server 1/2] Use 'QEMU version' -> '+pve-version' mapping for machine types

[Thomas Lamprecht]

On 2/10/20 4:05 PM, Stefan Reiter wrote:
> The previously introduced approach can fail for pinned versions when a
> new QEMU release is introduced. The saner approach is to use a mapping
> that gives one pve-version for each QEMU release.
> 
> Fortunately, the old system has not been bumped yet, so we can still
> change it without too much effort.
> 
> QEMU versions without a mapping are assumed to be pve0, 4.1 is mapped to
> pve1 since thats what we had as our default previously.
> 
> Pinned machine versions (i.e. pc-i440fx-4.1) are always assumed to be
> pve0, for specific pve-versions they'd have to be pinned as well (i.e.
> pc-i440fx-4.1+pve1).
> 
> The new logic also makes the pve-version dynamic, and starts VMs with
> the lowest possible 'feature-level', i.e. if a feature is only available
> with 4.1+pve2, but the VM isn't using it, we still start it with
> 4.1+pve0.
> 
> We die if we don't support a version that is requested from us. This
> allows us to use the pve-version as live-migration blocks (i.e. bumping
> the version and then live-migrating a VM which uses the new feature (so
> is running with the bumped version) to an outdated node will present the
> user with a helpful error message and fail instead of silently modifying
> the config and only failing *after* the migration).
> 
> $version_guard is introduced in config_to_command to use for features
> that need to check pve-version, it automatically handles selecting the
> newest necessary pve-version for the VM.
> 
> Tests have to be adjusted, since all of them now resolve to pve0 instead
> of pve1. EXPECT_ERROR matching is changed to use 'eq' instead of regex
> to allow special characters in error messages.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> For some reason, thinking this through is astonishingly confusing. My head is
> spinning with QEMU versions, and I'll probably have nightmares of '+pve0'
> tonight, but I *think* this should now cover all of our bases?
> 
> Based on several off-list email and in-person discussions with Thomas and
> Dominik.
> 
> Speaking of @Dominik: You'd have to rebase your recent patch [0] to use the new
> format.
> 
> Anyway, thourough review appreciated, let's avoid having to change this again.
> 
> [0] https://pve.proxmox.com/pipermail/pve-devel/2020-February/041588.html
> 
> 
>  PVE/QemuServer.pm                             | 53 +++++++++++++++----
>  PVE/QemuServer/Machine.pm                     | 38 ++++++++++++-
>  test/cfg2cmd/i440fx-win10-hostpci.conf.cmd    |  2 +-
>  .../minimal-defaults-to-new-machine.conf      |  2 +-
>  ...imal-defaults-unsupported-pve-version.conf |  5 ++
>  test/cfg2cmd/minimal-defaults.conf.cmd        |  2 +-
>  test/cfg2cmd/pinned-version.conf.cmd          |  2 +-
>  .../q35-linux-hostpci-multifunction.conf.cmd  |  2 +-
>  test/cfg2cmd/q35-linux-hostpci.conf.cmd       |  2 +-
>  test/cfg2cmd/q35-win10-hostpci.conf.cmd       |  2 +-
>  test/cfg2cmd/spice-linux-4.1.conf.cmd         |  2 +-
>  test/run_config2command_tests.pl              |  2 +-
>  12 files changed, 92 insertions(+), 22 deletions(-)
>  create mode 100644 test/cfg2cmd/minimal-defaults-unsupported-pve-version.conf
> 

applied, head now also spinning, and it's probably a good idea to let this update
traverse a bit slower through our repository cascade than normal..

Thanks!