Re: [pve-devel] [PATCH qemu-server 1/4] vmstatus: make boolean value explicit

2021-03-04 Thread 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.


  
  	$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

2021-03-04 Thread Fabian Ebner

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

2021-03-04 Thread Stefan Reiter

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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread Stefan Reiter
...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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread 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

-- 
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

2021-03-04 Thread Thomas Lamprecht
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

2021-03-04 Thread Thomas Lamprecht
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

2021-03-04 Thread Stefan Reiter
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

2021-03-04 Thread Fabian Ebner
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