Re: [pve-devel] [PATCH qemu-server 1/4] vmstatus: make boolean value explicit
Am 03.03.21 um 17:53 schrieb Thomas Lamprecht: On 03.03.21 12:01, Fabian Ebner wrote: as otherwise the empty string is printed with 'qm status --verbose' when it's not a template. Signed-off-by: Fabian Ebner --- PVE/QemuServer.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index a498444..43d7c6b 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -2621,7 +2621,7 @@ sub vmstatus { $d->{diskread} = 0; $d->{diskwrite} = 0; -$d->{template} = PVE::QemuConfig->is_template($conf); +$d->{template} = PVE::QemuConfig->is_template($conf) ? 1 : 0; Any reason to not do: $d->{template} = 1 if PVE::QemuConfig->is_template($conf); (no hard feelings, but this is relatively common pattern for such things, especially if they can be normally more often omitted than not, templates are normally rather outnumbered by non-templates) Not really a reason, just that the key was always present before the change too. But it's not an API return key, so making this optional is no problem at all (and we do so for other ones like the 'serial' below, so it's also more consistent in a way). I'll send a v2. $d->{serial} = 1 if conf_has_serial($conf); $d->{lock} = $conf->{lock} if $conf->{lock}; ___ 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 1/4] vmstatus: make boolean value explicit
Am 04.03.21 um 09:04 schrieb Fabian Ebner: Am 03.03.21 um 17:53 schrieb Thomas Lamprecht: On 03.03.21 12:01, Fabian Ebner wrote: as otherwise the empty string is printed with 'qm status --verbose' when it's not a template. Signed-off-by: Fabian Ebner --- PVE/QemuServer.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index a498444..43d7c6b 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -2621,7 +2621,7 @@ sub vmstatus { $d->{diskread} = 0; $d->{diskwrite} = 0; - $d->{template} = PVE::QemuConfig->is_template($conf); + $d->{template} = PVE::QemuConfig->is_template($conf) ? 1 : 0; Any reason to not do: $d->{template} = 1 if PVE::QemuConfig->is_template($conf); (no hard feelings, but this is relatively common pattern for such things, especially if they can be normally more often omitted than not, templates are normally rather outnumbered by non-templates) Not really a reason, just that the key was always present before the change too. But it's not an API return key, so making this optional is no problem at all (and we do so for other ones like the 'serial' below, so it's also more consistent in a way). I'll send a v2. Well, turns out this is a reason. Namely the web UI currently relies on the property being there: rec = s.data.get('template'); template = rec.data.value || false; Should I still go for it? Also, we say lock => { description => "The current config lock, if any.", type => 'string', optional => 1, }, but for containers, we do $d->{lock} = $conf->{lock} || ''; so it doesn't fully match the description (it's de-facto not optional). The reason is that there's a 'lock' column for 'pct list', see pve-container commit d02262048cbbe91ca8b12f98e3dc7bbab28e4c64. Is it worth changing the description or should the behavior rather be changed (while adapting the printing for 'pct list' of course)? $d->{serial} = 1 if conf_has_serial($conf); $d->{lock} = $conf->{lock} if $conf->{lock}; ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ___ 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 1/3] machine: split out helper for handling query-machines qmp command result
LGTM, for both qemu-server patches: Reviewed-by: Stefan Reiter Not sure about the formatting in the GUI, but I'm also the wrong person to ask there. Maybe don't capitalize "Qemu", as we also don't do that for "running"/"stopped"/... either? On 01/03/2021 16:53, Fabian Ebner wrote: to be re-used in the vmstatus() call. Signed-off-by: Fabian Ebner --- PVE/QemuServer/Machine.pm | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index c168ade..2474951 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -18,11 +18,8 @@ sub machine_type_is_q35 { return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0; } -# this only works if VM is running -sub get_current_qemu_machine { -my ($vmid) = @_; - -my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-machines'); +sub current_from_query_machines { +my ($res) = @_; my ($current, $pve_version, $default); foreach my $e (@$res) { @@ -37,6 +34,15 @@ sub get_current_qemu_machine { return $current || $default || 'pc'; } +# this only works if VM is running +sub get_current_qemu_machine { +my ($vmid) = @_; + +my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-machines'); + +return current_from_query_machines($res); +} + # returns a string with major.minor+pve, patch version-part is ignored # as it's seldom ressembling a real QEMU machine type, so it would be '0' 99% of # the time anyway.. This explicitly separates pveversion from the machine. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager 7/7] ui: MachineEdit: add option for machine version pinning
Hidden behind "Advanced" options, as to not confuse inexperienced users. Signed-off-by: Stefan Reiter --- www/manager6/qemu/MachineEdit.js | 69 1 file changed, 69 insertions(+) diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js index 8a3a6f7a..4eab6b8f 100644 --- a/www/manager6/qemu/MachineEdit.js +++ b/www/manager6/qemu/MachineEdit.js @@ -2,8 +2,51 @@ Ext.define('PVE.qemu.MachineInputPanel', { extend: 'Proxmox.panel.InputPanel', xtype: 'pveMachineInputPanel', +controller: { + xclass: 'Ext.app.ViewController', + control: { + 'combobox[name=machine]': { + change: 'onMachineChange', + }, + }, + onMachineChange: function(field, value) { + let me = this; + let version = me.lookup('version'); + let store = version.getStore(); + let type = value === 'q35' ? 'q35' : 'i440fx'; + store.clearFilter(); + store.addFilter(val => + (val.data.name === 'latest' || val.data.name.indexOf(type) !== -1)); + version.setValue('latest'); + }, +}, + +onGetValues: function(values) { + if (values.version && values.version !== 'latest') { + values.machine = values.version; + delete values.delete; + } + delete values.version; + return values; +}, + +setValues: function(values) { + let me = this; + + if (values.machine !== '__default__' && values.machine !== 'q35') { + values.version = values.machine; + values.machine = values.version.match(/q35/) ? 'q35' : '__default__'; + + // avoid hiding a pinned version + Ext.ComponentQuery.query("#advancedcb")[0].setValue(true); + } + + this.callParent(arguments); +}, + items: [{ name: 'machine', + reference: 'machine', xtype: 'proxmoxKVComboBox', fieldLabel: gettext('Machine'), comboItems: [ @@ -11,6 +54,32 @@ Ext.define('PVE.qemu.MachineInputPanel', { ['q35', 'q35'], ], }], + +advancedItems: [{ + name: 'version', + reference: 'version', + xtype: 'combobox', + fieldLabel: gettext('Version'), + value: 'latest', + allowBlank: false, + editable: false, + valueField: 'name', + displayField: 'name', + queryParam: false, + store: { + autoLoad: true, + fields: ['name'], + proxy: { + type: 'proxmox', + url: "/api2/json/nodes/localhost/machine-types", + }, + listeners: { + load: function(records) { + this.insert(0, { name: 'latest' }); + }, + }, + }, +}], }); Ext.define('PVE.qemu.MachineEdit', { -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager 6/7] ui: create MachineEdit window
no functional change intended Signed-off-by: Stefan Reiter --- www/manager6/Makefile | 1 + www/manager6/qemu/HardwareView.js | 16 + www/manager6/qemu/MachineEdit.js | 38 +++ 3 files changed, 40 insertions(+), 15 deletions(-) create mode 100644 www/manager6/qemu/MachineEdit.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index 85f90ecd..a2f7be6d 100644 --- a/www/manager6/Makefile +++ b/www/manager6/Makefile @@ -204,6 +204,7 @@ JSSRC= \ qemu/HardwareView.js\ qemu/IPConfigEdit.js\ qemu/KeyboardEdit.js\ + qemu/MachineEdit.js \ qemu/MemoryEdit.js \ qemu/Monitor.js \ qemu/NetworkEdit.js \ diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js index 41d65b40..470baa46 100644 --- a/www/manager6/qemu/HardwareView.js +++ b/www/manager6/qemu/HardwareView.js @@ -154,21 +154,7 @@ Ext.define('PVE.qemu.HardwareView', { }, machine: { header: gettext('Machine'), - editor: caps.vms['VM.Config.HWType'] ? { - xtype: 'proxmoxWindowEdit', - subject: gettext('Machine'), - width: 350, - items: [{ - xtype: 'proxmoxKVComboBox', - name: 'machine', - value: '__default__', - fieldLabel: gettext('Machine'), - comboItems: [ - ['__default__', PVE.Utils.render_qemu_machine('')], - ['q35', 'q35'], - ], - }], -} : undefined, + editor: caps.vms['VM.Config.HWType'] ? 'PVE.qemu.MachineEdit' : undefined, iconCls: 'cogs', never_delete: true, group: 6, diff --git a/www/manager6/qemu/MachineEdit.js b/www/manager6/qemu/MachineEdit.js new file mode 100644 index ..8a3a6f7a --- /dev/null +++ b/www/manager6/qemu/MachineEdit.js @@ -0,0 +1,38 @@ +Ext.define('PVE.qemu.MachineInputPanel', { +extend: 'Proxmox.panel.InputPanel', +xtype: 'pveMachineInputPanel', + +items: [{ + name: 'machine', + xtype: 'proxmoxKVComboBox', + fieldLabel: gettext('Machine'), + comboItems: [ + ['__default__', PVE.Utils.render_qemu_machine('')], + ['q35', 'q35'], + ], +}], +}); + +Ext.define('PVE.qemu.MachineEdit', { +extend: 'Proxmox.window.Edit', + +subject: gettext('Machine'), + +items: [{ + xtype: 'pveMachineInputPanel', +}], + +initComponent: function() { + let me = this; + + me.callParent(); + + me.load({ + success: function(response) { + let vmconfig = response.result.data; + let machine = vmconfig.machine || '__default__'; + me.setValues({ machine: machine }); + }, + }); +}, +}); -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 3/7] api: add Machine module to query machine types
The file is provided by pve-qemu-kvm. Signed-off-by: Stefan Reiter --- PVE/API2/Qemu/Machine.pm | 49 PVE/API2/Qemu/Makefile | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 PVE/API2/Qemu/Machine.pm diff --git a/PVE/API2/Qemu/Machine.pm b/PVE/API2/Qemu/Machine.pm new file mode 100644 index 000..c0a8c57 --- /dev/null +++ b/PVE/API2/Qemu/Machine.pm @@ -0,0 +1,49 @@ +package PVE::API2::Qemu::Machine; + +use strict; +use warnings; + +use PVE::RESTHandler; +use PVE::JSONSchema qw(get_standard_option); +use PVE::Tools qw(file_get_contents); + +use base qw(PVE::RESTHandler); + +__PACKAGE__->register_method({ +name => 'types', +path => '', +method => 'GET', +proxyto => 'node', +description => "Get available QEMU/KVM machine types.", +permissions => { + user => 'all', +}, +parameters => { + additionalProperties => 0, + properties => { + node => get_standard_option('pve-node'), + }, +}, +returns => { + type => 'array', + items => { + type => 'object', + properties => { + name => { + type => 'string', + description => "Name of machine type.", + }, + }, + }, +}, +code => sub { + my $content = eval { + file_get_contents("/usr/share/kvm/machine-versions-x86_64"); + }; + die "could not get supported machine versions (try updating 'pve-qemu-kvm') - $@" if $@; + my @data = split(m/\n/, $content); + @data = map { { name => $_ } } @data; + return \@data; +}}); + +1; diff --git a/PVE/API2/Qemu/Makefile b/PVE/API2/Qemu/Makefile index f4b7be6..5d4abda 100644 --- a/PVE/API2/Qemu/Makefile +++ b/PVE/API2/Qemu/Makefile @@ -1,4 +1,4 @@ -SOURCES=Agent.pm CPU.pm +SOURCES=Agent.pm CPU.pm Machine.pm .PHONY: install install: -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager 5/7] api: register Qemu::Machine call
as 'machine-types', so it is clear this refers to QEMU machines, not the local machine (as one might think, this being a 'node' API call). Signed-off-by: Stefan Reiter --- Requires dependency bump on updated qemu-server. PVE/API2/Nodes.pm | 6 ++ 1 file changed, 6 insertions(+) diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm index 8172231e..7a39320a 100644 --- a/PVE/API2/Nodes.pm +++ b/PVE/API2/Nodes.pm @@ -35,6 +35,7 @@ use PVE::API2::Storage::Scan; use PVE::API2::Storage::Status; use PVE::API2::Qemu; use PVE::API2::Qemu::CPU; +use PVE::API2::Qemu::Machine; use PVE::API2::LXC; use PVE::API2::LXC::Status; use PVE::API2::VZDump; @@ -71,6 +72,11 @@ __PACKAGE__->register_method ({ path => 'cpu', }); +__PACKAGE__->register_method ({ +subclass => "PVE::API2::Qemu::Machine", +path => 'machine-types', +}); + __PACKAGE__->register_method ({ subclass => "PVE::API2::LXC", path => 'lxc', -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-qemu 1/7] add static supported machines file
Same rationale as the CPU flags file, avoids calling QEMU binary just to query machine types. Signed-off-by: Stefan Reiter --- debian/parse-machines.pl | 21 + debian/rules | 4 +++- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100755 debian/parse-machines.pl diff --git a/debian/parse-machines.pl b/debian/parse-machines.pl new file mode 100755 index 000..8ad335d --- /dev/null +++ b/debian/parse-machines.pl @@ -0,0 +1,21 @@ +#!/usr/bin/perl + +use warnings; +use strict; + +my @machines = (); + +while () { +if (/^\s*Supported machines are:/) { + next; +} + +s/^\s+//; +my @machine = split(/\s+/); +next if $machine[0] !~ m/^pc-(i440fx|q35)/; +push @machines, $machine[0]; +} + +die "no QEMU machine types detected from STDIN input" if scalar (@machines) <= 0; + +print join("\n", @machines) or die "$!\n"; diff --git a/debian/rules b/debian/rules index 4a6b472..386ac4b 100755 --- a/debian/rules +++ b/debian/rules @@ -22,6 +22,7 @@ PACKAGE=pve-qemu-kvm destdir := $(CURDIR)/debian/$(PACKAGE) flagfile := $(destdir)/usr/share/kvm/recognized-CPUID-flags-x86_64 +machinefile := $(destdir)/usr/share/kvm/machine-versions-x86_64 # default QEMU out-of-tree build directory is ./build BUILDDIR=build @@ -124,7 +125,7 @@ install: build rm $(destdir)/usr/share/kvm/qemu_vga.ndrv rm $(destdir)/usr/share/kvm/slof.bin rm $(destdir)/usr/share/kvm/u-boot.e500 - # remove Aplha files + # remove Alpha files rm $(destdir)/usr/share/kvm/palcode-clipper # remove RISC-V files rm $(destdir)/usr/share/kvm/opensbi-riscv32-generic-fw_dynamic.elf @@ -138,6 +139,7 @@ install: build # CPU flags are static for QEMU version, allows avoiding more costly checks $(destdir)/usr/bin/qemu-system-x86_64 -cpu help | ./debian/parse-cpu-flags.pl > $(flagfile) + $(destdir)/usr/bin/qemu-system-x86_64 -machine help | ./debian/parse-machines.pl > $(machinefile) # Build architecture-independent files here. binary-indep: build install -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 4/7] add postinst with Windows device incompatibility workaround
...for QEMU 5.2 Add a postinst with some inline perl (to avoid having to install a seperate file to run) that conservatively does its best to set the machine version for all VMs the bug might affect to 5.1. Signed-off-by: Stefan Reiter --- Requires Fabian's #3301 series: https://lists.proxmox.com/pipermail/pve-devel/2021-March/047175.html I did it as inline perl because I wasn't sure where to put a file containing this to be able to execute it from postinst without cluttering the machine after it is done. Feel free to extract it somewhere else if you see fit. Might also make sense to update the link from the forum for "more details" to the cover of this series, or something else... debian/postinst | 95 + 1 file changed, 95 insertions(+) create mode 100755 debian/postinst diff --git a/debian/postinst b/debian/postinst new file mode 100755 index 000..c93dd5d --- /dev/null +++ b/debian/postinst @@ -0,0 +1,95 @@ +#!/bin/sh + +# Abort if any command returns an error value +set -e + +# This script is called as the last step of the installation of the +# package. All the package's files are in place, dpkg has already +# done its automatic conffile handling, and all the packages we depend +# of are already fully installed and configured. + +case "$1" in + configure) +if test -n "$2"; then + if dpkg --compare-versions "$2" 'lt' '6.3-6'; then +echo +perl <<"EOPERL" + +use warnings; +use strict; + +use PVE::Cluster; +use PVE::QemuConfig; +use PVE::QemuServer; +use PVE::QemuServer::Helpers qw(min_version); + +print "Pinning Windows VM machine versions to 5.1\n"; +print "See https://forum.proxmox.com/threads/warning-latest-patch-just-broke-all-my-windows-vms-6-3-4-patch-inside.84915/ for details\n"; + +PVE::Cluster::check_cfs_is_mounted(); +PVE::Cluster::cfs_update(); + +my $vms = PVE::QemuServer::vmstatus(undef, 1); +foreach my $vmid (sort keys %$vms) { +my $running = defined($vms->{$vmid}->{pid}) ? 1 : 0; +eval { +PVE::QemuConfig->lock_config($vmid, sub { +my $conf = PVE::QemuConfig->load_config($vmid); +PVE::QemuConfig->check_lock($conf); + +return if !$conf->{ostype} || !PVE::QemuServer::windows_version($conf->{ostype}); + +if ($running) { +my $cur_machine = $vms->{$vmid}->{'running-machine'}; +$cur_machine =~ s/pc(-i440fx|-q35)?-//; +if (min_version($cur_machine, 5, 2)) { +print "$vmid: [skip] already running with machine version '$cur_machine'\n"; +return; +} +} + +if ($conf->{machine} && $conf->{machine} =~ m/^pc-/) { +print "$vmid: [skip] already pinned to '$conf->{machine}'\n"; +return; +} + +if ($conf->{pending}->{machine}) { +print "$vmid: [skip] machine changed in pending\n"; +return; +} + +my $cur = $conf->{machine} || "i440fx"; +$cur = "i440fx" if $cur eq "pc"; +my $new = "pc-$cur-5.1"; + +my $notice = ""; +if ($running) { +$conf->{pending}->{machine} = $new; +$notice = " (after restart)"; +} else { +$conf->{machine} = $new; +} + +PVE::QemuConfig->write_config($vmid, $conf); +print "$vmid: [ OK ] now pinned to '$new'$notice\n"; +}); +}; +warn "$vmid: [warn] failed to update: $@\n" if $@; +} + +print "If you want to upgrade to the newest version, set the machine back to 'latest' in the GUI, or 'pc'/'q35' on the CLI.\n"; + +EOPERL +echo + fi +fi +;; + + abort-upgrade|abort-remove|abort-deconfigure) +;; + + *) echo "$0: didn't understand being called with \`$1'" 1>&2 + exit 0;; +esac + +exit 0 -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-qemu 2/7] add ACPI compat patch for 5.1 and older machine types
Signed-off-by: Stefan Reiter --- This is required to make the whole deal work, otherwise even setting back the machine version to 5.1 or lower won't actually fix the issue. ...restore-device-paths-for-pre-5.1-vms.patch | 108 ++ debian/patches/series | 1 + 2 files changed, 109 insertions(+) create mode 100644 debian/patches/extra/0009-i386-acpi-restore-device-paths-for-pre-5.1-vms.patch diff --git a/debian/patches/extra/0009-i386-acpi-restore-device-paths-for-pre-5.1-vms.patch b/debian/patches/extra/0009-i386-acpi-restore-device-paths-for-pre-5.1-vms.patch new file mode 100644 index 000..0539608 --- /dev/null +++ b/debian/patches/extra/0009-i386-acpi-restore-device-paths-for-pre-5.1-vms.patch @@ -0,0 +1,108 @@ +From Mon Sep 17 00:00:00 2001 +From: Vitaly Cheptsov +Date: Tue, 2 Mar 2021 09:21:10 -0500 +Subject: [PATCH] i386/acpi: restore device paths for pre-5.1 vms + +After fixing the _UID value for the primary PCI root bridge in +af1b80ae it was discovered that this change updates Windows +configuration in an incompatible way causing network configuration +failure unless DHCP is used. More details provided on the list: + +https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html + +This change reverts the _UID update from 1 to 0 for q35 and i440fx +VMs before version 5.2 to maintain the original behaviour when +upgrading. + +Cc: qemu-sta...@nongnu.org +Cc: qemu-de...@nongnu.org +Reported-by: Thomas Lamprecht +Suggested-by: Michael S. Tsirkin +Signed-off-by: Vitaly Cheptsov +Message-Id: <20210301195919.9333-1-chept...@ispras.ru> +Tested-by: Thomas Lamprecht +Reviewed-by: Igor Mammedov +Reviewed-by: Michael S. Tsirkin +Signed-off-by: Michael S. Tsirkin +Fixes: af1b80ae56c9 ("i386/acpi: fix inconsistent QEMU/OVMF device paths") +--- + hw/i386/acpi-build.c | 4 ++-- + hw/i386/pc_piix.c| 2 ++ + hw/i386/pc_q35.c | 2 ++ + include/hw/i386/pc.h | 1 + + 4 files changed, 7 insertions(+), 2 deletions(-) + +diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c +index 1f5c211245..b5616582a5 100644 +--- a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c +@@ -1513,7 +1513,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, + dev = aml_device("PCI0"); + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); + aml_append(dev, aml_name_decl("_ADR", aml_int(0))); +-aml_append(dev, aml_name_decl("_UID", aml_int(0))); ++aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid))); + aml_append(sb_scope, dev); + aml_append(dsdt, sb_scope); + +@@ -1530,7 +1530,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08"))); + aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); + aml_append(dev, aml_name_decl("_ADR", aml_int(0))); +-aml_append(dev, aml_name_decl("_UID", aml_int(0))); ++aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid))); + aml_append(dev, build_q35_osc_method()); + aml_append(sb_scope, dev); + +diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c +index 13d1628f13..2524c96216 100644 +--- a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c +@@ -417,6 +417,7 @@ static void pc_i440fx_machine_options(MachineClass *m) + { + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); + pcmc->default_nic_model = "e1000"; ++pcmc->pci_root_uid = 0; + + m->family = "pc_piix"; + m->desc = "Standard PC (i440FX + PIIX, 1996)"; +@@ -448,6 +449,7 @@ static void pc_i440fx_5_1_machine_options(MachineClass *m) + compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); + compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); + pcmc->kvmclock_create_always = false; ++pcmc->pci_root_uid = 1; + } + + DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL, +diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c +index a3f4959c43..c58dad5ae3 100644 +--- a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c +@@ -329,6 +329,7 @@ static void pc_q35_machine_options(MachineClass *m) + { + PCMachineClass *pcmc = PC_MACHINE_CLASS(m); + pcmc->default_nic_model = "e1000e"; ++pcmc->pci_root_uid = 0; + + m->family = "pc_q35"; + m->desc = "Standard PC (Q35 + ICH9, 2009)"; +@@ -364,6 +365,7 @@ static void pc_q35_5_1_machine_options(MachineClass *m) + compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len); + compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len); + pcmc->kvmclock_create_always = false; ++pcmc->pci_root_uid = 1; + } + + DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL, +diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h +index 911e460097..7f8e1a791f 100644 +--- a/include/hw/i386/pc.h b/include/hw/i386/pc.h +@@ -99,6 +99,7 @@ struct PCMachineClass { + int legacy_acpi_table_size; + unsigned acpi_data_size;
[pve-devel] [PATCH 0/7] Work around QEMU 5.2 windows incompatibility
...and make machine version configurable in the GUI, to allow users to return to the default behaviour of always using the latest one. The workaround is based on a postinst script running when upgrading the qemu-server package, which sets the machine version of all effected Windows VMs to pc-(i440fx|q35)-5.1 to pin them to the last version still using the old ACPI table layout. Note: the postinst patch requires Fabian's #3301 series: https://lists.proxmox.com/pipermail/pve-devel/2021-March/047175.html pve-qemu: Stefan Reiter (2): add static supported machines file add ACPI compat patch for 5.1 and older machine types debian/parse-machines.pl | 21 ...restore-device-paths-for-pre-5.1-vms.patch | 108 ++ debian/patches/series | 1 + debian/rules | 4 +- 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100755 debian/parse-machines.pl create mode 100644 debian/patches/extra/0009-i386-acpi-restore-device-paths-for-pre-5.1-vms.patch qemu-server: Stefan Reiter (2): api: add Machine module to query machine types add postinst with Windows device incompatibility workaround PVE/API2/Qemu/Machine.pm | 49 + PVE/API2/Qemu/Makefile | 2 +- debian/postinst | 95 3 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 PVE/API2/Qemu/Machine.pm create mode 100755 debian/postinst manager: Stefan Reiter (3): api: register Qemu::Machine call ui: create MachineEdit window ui: MachineEdit: add option for machine version pinning PVE/API2/Nodes.pm | 6 ++ www/manager6/Makefile | 1 + www/manager6/qemu/HardwareView.js | 16 + www/manager6/qemu/MachineEdit.js | 107 ++ 4 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 www/manager6/qemu/MachineEdit.js -- 2.20.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 1/3] machine: split out helper for handling query-machines qmp command result
On 01.03.21 16:53, Fabian Ebner wrote: > to be re-used in the vmstatus() call. > > Signed-off-by: Fabian Ebner > --- > PVE/QemuServer/Machine.pm | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) > > applied, with Stefan's R-b, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 2/3] fix #3301: status: add currently running machine and QEMU version to full status
On 01.03.21 16:53, Fabian Ebner wrote: > Signed-off-by: Fabian Ebner > --- > PVE/QemuServer.pm | 32 > 1 file changed, 32 insertions(+) > > applied, with Stefan's R-b, thanks! ___ 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 4/7] add postinst with Windows device incompatibility workaround
I suppose we should also detect old QEMU versions, so we don't set 5.1 machine versions for QEMU < 5.1. Unlikely, but easy to detect: print "QEMU version too old for workaround" if !min_version(PVE::QemuServer::kvm_user_version(), 5, 1); or similar... I can send a v2 if required. On 04/03/2021 13:52, Stefan Reiter wrote: ...for QEMU 5.2 Add a postinst with some inline perl (to avoid having to install a seperate file to run) that conservatively does its best to set the machine version for all VMs the bug might affect to 5.1. Signed-off-by: Stefan Reiter --- Requires Fabian's #3301 series: https://lists.proxmox.com/pipermail/pve-devel/2021-March/047175.html I did it as inline perl because I wasn't sure where to put a file containing this to be able to execute it from postinst without cluttering the machine after it is done. Feel free to extract it somewhere else if you see fit. Might also make sense to update the link from the forum for "more details" to the cover of this series, or something else... debian/postinst | 95 + 1 file changed, 95 insertions(+) create mode 100755 debian/postinst diff --git a/debian/postinst b/debian/postinst new file mode 100755 index 000..c93dd5d --- /dev/null +++ b/debian/postinst @@ -0,0 +1,95 @@ +#!/bin/sh + +# Abort if any command returns an error value +set -e + +# This script is called as the last step of the installation of the +# package. All the package's files are in place, dpkg has already +# done its automatic conffile handling, and all the packages we depend +# of are already fully installed and configured. + +case "$1" in + configure) +if test -n "$2"; then + if dpkg --compare-versions "$2" 'lt' '6.3-6'; then +echo +perl <<"EOPERL" + +use warnings; +use strict; + +use PVE::Cluster; +use PVE::QemuConfig; +use PVE::QemuServer; +use PVE::QemuServer::Helpers qw(min_version); + +print "Pinning Windows VM machine versions to 5.1\n"; +print "See https://forum.proxmox.com/threads/warning-latest-patch-just-broke-all-my-windows-vms-6-3-4-patch-inside.84915/ for details\n"; + +PVE::Cluster::check_cfs_is_mounted(); +PVE::Cluster::cfs_update(); + +my $vms = PVE::QemuServer::vmstatus(undef, 1); +foreach my $vmid (sort keys %$vms) { +my $running = defined($vms->{$vmid}->{pid}) ? 1 : 0; +eval { +PVE::QemuConfig->lock_config($vmid, sub { +my $conf = PVE::QemuConfig->load_config($vmid); +PVE::QemuConfig->check_lock($conf); + +return if !$conf->{ostype} || !PVE::QemuServer::windows_version($conf->{ostype}); + +if ($running) { +my $cur_machine = $vms->{$vmid}->{'running-machine'}; +$cur_machine =~ s/pc(-i440fx|-q35)?-//; +if (min_version($cur_machine, 5, 2)) { +print "$vmid: [skip] already running with machine version '$cur_machine'\n"; +return; +} +} + +if ($conf->{machine} && $conf->{machine} =~ m/^pc-/) { +print "$vmid: [skip] already pinned to '$conf->{machine}'\n"; +return; +} + +if ($conf->{pending}->{machine}) { +print "$vmid: [skip] machine changed in pending\n"; +return; +} + +my $cur = $conf->{machine} || "i440fx"; +$cur = "i440fx" if $cur eq "pc"; +my $new = "pc-$cur-5.1"; + +my $notice = ""; +if ($running) { +$conf->{pending}->{machine} = $new; +$notice = " (after restart)"; +} else { +$conf->{machine} = $new; +} + +PVE::QemuConfig->write_config($vmid, $conf); +print "$vmid: [ OK ] now pinned to '$new'$notice\n"; +}); +}; +warn "$vmid: [warn] failed to update: $@\n" if $@; +} + +print "If you want to upgrade to the newest version, set the machine back to 'latest' in the GUI, or 'pc'/'q35' on the CLI.\n"; + +EOPERL +echo + fi +fi +;; + + abort-upgrade|abort-remove|abort-deconfigure) +;; + + *) echo "$0: didn't understand being called with \`$1'" 1>&2 + exit 0;; +esac + +exit 0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 0/7] Work around QEMU 5.2 windows incompatibility
Didn't follow this issue too closely, but what about restoring a pre-QEMU-ACPI-patch Windows VM backup in a post-QEMU-ACPI-patch world? Those would need to get pinned as well, right? Doing the pinning itself on restore shouldn't be difficult, but a good way to detect when it's needed is needed. Am 04.03.21 um 13:52 schrieb Stefan Reiter: ...and make machine version configurable in the GUI, to allow users to return to the default behaviour of always using the latest one. The workaround is based on a postinst script running when upgrading the qemu-server package, which sets the machine version of all effected Windows VMs to pc-(i440fx|q35)-5.1 to pin them to the last version still using the old ACPI table layout. Note: the postinst patch requires Fabian's #3301 series: https://lists.proxmox.com/pipermail/pve-devel/2021-March/047175.html pve-qemu: Stefan Reiter (2): add static supported machines file add ACPI compat patch for 5.1 and older machine types debian/parse-machines.pl | 21 ...restore-device-paths-for-pre-5.1-vms.patch | 108 ++ debian/patches/series | 1 + debian/rules | 4 +- 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100755 debian/parse-machines.pl create mode 100644 debian/patches/extra/0009-i386-acpi-restore-device-paths-for-pre-5.1-vms.patch qemu-server: Stefan Reiter (2): api: add Machine module to query machine types add postinst with Windows device incompatibility workaround PVE/API2/Qemu/Machine.pm | 49 + PVE/API2/Qemu/Makefile | 2 +- debian/postinst | 95 3 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 PVE/API2/Qemu/Machine.pm create mode 100755 debian/postinst manager: Stefan Reiter (3): api: register Qemu::Machine call ui: create MachineEdit window ui: MachineEdit: add option for machine version pinning PVE/API2/Nodes.pm | 6 ++ www/manager6/Makefile | 1 + www/manager6/qemu/HardwareView.js | 16 + www/manager6/qemu/MachineEdit.js | 107 ++ 4 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 www/manager6/qemu/MachineEdit.js ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel