Re: [pve-devel] [PATCH v9 qemu-server 2/6] Include "-cpu" parameter with snapshots/suspend
On 07/04/2020 09:04, Fabian Ebner wrote: On 06.04.20 15:01, Stefan Reiter wrote: On 06/04/2020 14:31, Fabian Ebner wrote: On 26.03.20 16:13, Stefan Reiter wrote: Just like with live-migration, custom CPU models might change after a snapshot has been taken (or a VM suspended), which would lead to a different QEMU invocation on rollback/resume. Save the "-cpu" argument as a new "runningcpu" option into the VM conf akin to "runningmachine" and use as override during rollback/resume. No functional change with non-custom CPU types intended. The changes apply to all CPU types, but that's not a bad thing. If one takes a snapshot, restarts the VM with a different CPU type and does a rollback, it'll use the CPU at the time of the snapshot even if no custom models are involved. Was that a thing before? It should have worked that way anyhow, since the 'cpu' setting was stored in the snapshot's section in the config. Ah, you're right. As long as the defaults for the standard models don't change, vm_start will re-generate the same -cpu option when no custom model is involved. The default models can also be versioned with 'runningmachine', so for non-custom cpus this was always safe. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v9 qemu-server 2/6] Include "-cpu" parameter with snapshots/suspend
On 06.04.20 15:01, Stefan Reiter wrote:
On 06/04/2020 14:31, Fabian Ebner wrote:
On 26.03.20 16:13, Stefan Reiter wrote:
Just like with live-migration, custom CPU models might change after a
snapshot has been taken (or a VM suspended), which would lead to a
different QEMU invocation on rollback/resume.
Save the "-cpu" argument as a new "runningcpu" option into the VM conf
akin to "runningmachine" and use as override during rollback/resume.
No functional change with non-custom CPU types intended.
The changes apply to all CPU types, but that's not a bad thing. If one
takes a snapshot, restarts the VM with a different CPU type and does a
rollback, it'll use the CPU at the time of the snapshot even if no
custom models are involved.
Was that a thing before? It should have worked that way anyhow, since
the 'cpu' setting was stored in the snapshot's section in the config.
Ah, you're right. As long as the defaults for the standard models don't
change, vm_start will re-generate the same -cpu option when no custom
model is involved.
Signed-off-by: Stefan Reiter
---
PVE/API2/Qemu.pm | 1 +
PVE/QemuConfig.pm | 34 ++
PVE/QemuServer.pm | 28
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 80fd7ea..2c7e998 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1129,6 +1129,7 @@ my $update_vm_api = sub {
push @delete, 'lock'; # this is the real deal to write it out
}
push @delete, 'runningmachine' if $conf->{runningmachine};
+ push @delete, 'runningcpu' if $conf->{runningcpu};
}
PVE::QemuConfig->check_lock($conf) if !$skiplock;
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 8d03774..386223d 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -5,6 +5,7 @@ use warnings;
use PVE::AbstractConfig;
use PVE::INotify;
+use PVE::QemuServer::CPUConfig;
use PVE::QemuServer::Drive;
use PVE::QemuServer::Helpers;
use PVE::QemuServer::Monitor qw(mon_cmd);
@@ -155,15 +156,26 @@ sub __snapshot_save_vmstate {
my $statefile = PVE::Storage::vdisk_alloc($storecfg, $target,
$vmid, 'raw', $name, $size*1024);
my $runningmachine =
PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
- if ($suspend) {
- $conf->{vmstate} = $statefile;
- $conf->{runningmachine} = $runningmachine;
- } else {
- my $snap = $conf->{snapshots}->{$snapname};
- $snap->{vmstate} = $statefile;
- $snap->{runningmachine} = $runningmachine;
+ # get current QEMU -cpu argument
+ my $runningcpu;
+ if (my $pid = PVE::QemuServer::check_running($vmid)) {
+ my $cmdline = PVE::QemuServer::Helpers::parse_cmdline($pid);
+ die "could not read commandline of running machine\n"
+ if !$cmdline->{cpu}->{value};
+
+ # sanitize and untaint value
+ $cmdline->{cpu}->{value} =~
$PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re;
+ $runningcpu = $1;
+ }
+
+ if (!$suspend) {
+ $conf = $conf->{snapshots}->{$snapname};
}
+ $conf->{vmstate} = $statefile;
+ $conf->{runningmachine} = $runningmachine;
+ $conf->{runningcpu} = $runningcpu;
+
return $statefile;
}
@@ -309,6 +321,11 @@ sub __snapshot_rollback_hook {
if (defined($conf->{runningmachine})) {
$data->{forcemachine} = $conf->{runningmachine};
delete $conf->{runningmachine};
+
+ # runningcpu is newer than runningmachine, so assume it only
exists
+ # here, if at all
+ $data->{forcecpu} = delete $conf->{runningcpu}
+ if defined($conf->{runningcpu});
} else {
# Note: old code did not store 'machine', so we try to be
smart
# and guess the snapshot was generated with kvm 1.4
(pc-i440fx-1.4).
@@ -361,7 +378,8 @@ sub __snapshot_rollback_vm_start {
my ($class, $vmid, $vmstate, $data) = @_;
my $storecfg = PVE::Storage::config();
- PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef,
undef, undef, $data->{forcemachine});
+ PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef,
undef, undef,
+ $data->{forcemachine}, undef, undef, undef, undef, undef, undef,
$data->{forcecpu});
}
sub __snapshot_rollback_get_unused {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 98c2a9a..70a5234 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -567,8 +567,15 @@ EODESCR
optional => 1,
}),
runningmachine => get_standard_option('pve-qemu-machine', {
- description => "Specifies the Qemu machine type of the running
vm. This is used internally for snapshots.",
+ description => "Specifies the QEMU machine type of the running
vm. This is used internally for snapshots.",
}),
+ runningcpu => {
+ description => "Specifies the QEMU '-cpu' parameter of the
running vm. This is used internally for snapshots.",
+ optional => 1,
+ type => 'string',
+ pattern =>
Re: [pve-devel] [PATCH v9 qemu-server 2/6] Include "-cpu" parameter with snapshots/suspend
On 06/04/2020 14:31, Fabian Ebner wrote:
On 26.03.20 16:13, Stefan Reiter wrote:
Just like with live-migration, custom CPU models might change after a
snapshot has been taken (or a VM suspended), which would lead to a
different QEMU invocation on rollback/resume.
Save the "-cpu" argument as a new "runningcpu" option into the VM conf
akin to "runningmachine" and use as override during rollback/resume.
No functional change with non-custom CPU types intended.
The changes apply to all CPU types, but that's not a bad thing. If one
takes a snapshot, restarts the VM with a different CPU type and does a
rollback, it'll use the CPU at the time of the snapshot even if no
custom models are involved.
Was that a thing before? It should have worked that way anyhow, since
the 'cpu' setting was stored in the snapshot's section in the config.
Signed-off-by: Stefan Reiter
---
PVE/API2/Qemu.pm | 1 +
PVE/QemuConfig.pm | 34 ++
PVE/QemuServer.pm | 28
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 80fd7ea..2c7e998 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1129,6 +1129,7 @@ my $update_vm_api = sub {
push @delete, 'lock'; # this is the real deal to write it out
}
push @delete, 'runningmachine' if $conf->{runningmachine};
+ push @delete, 'runningcpu' if $conf->{runningcpu};
}
PVE::QemuConfig->check_lock($conf) if !$skiplock;
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 8d03774..386223d 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -5,6 +5,7 @@ use warnings;
use PVE::AbstractConfig;
use PVE::INotify;
+use PVE::QemuServer::CPUConfig;
use PVE::QemuServer::Drive;
use PVE::QemuServer::Helpers;
use PVE::QemuServer::Monitor qw(mon_cmd);
@@ -155,15 +156,26 @@ sub __snapshot_save_vmstate {
my $statefile = PVE::Storage::vdisk_alloc($storecfg, $target,
$vmid, 'raw', $name, $size*1024);
my $runningmachine =
PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
- if ($suspend) {
- $conf->{vmstate} = $statefile;
- $conf->{runningmachine} = $runningmachine;
- } else {
- my $snap = $conf->{snapshots}->{$snapname};
- $snap->{vmstate} = $statefile;
- $snap->{runningmachine} = $runningmachine;
+ # get current QEMU -cpu argument
+ my $runningcpu;
+ if (my $pid = PVE::QemuServer::check_running($vmid)) {
+ my $cmdline = PVE::QemuServer::Helpers::parse_cmdline($pid);
+ die "could not read commandline of running machine\n"
+ if !$cmdline->{cpu}->{value};
+
+ # sanitize and untaint value
+ $cmdline->{cpu}->{value} =~
$PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re;
+ $runningcpu = $1;
+ }
+
+ if (!$suspend) {
+ $conf = $conf->{snapshots}->{$snapname};
}
+ $conf->{vmstate} = $statefile;
+ $conf->{runningmachine} = $runningmachine;
+ $conf->{runningcpu} = $runningcpu;
+
return $statefile;
}
@@ -309,6 +321,11 @@ sub __snapshot_rollback_hook {
if (defined($conf->{runningmachine})) {
$data->{forcemachine} = $conf->{runningmachine};
delete $conf->{runningmachine};
+
+ # runningcpu is newer than runningmachine, so assume it only
exists
+ # here, if at all
+ $data->{forcecpu} = delete $conf->{runningcpu}
+ if defined($conf->{runningcpu});
} else {
# Note: old code did not store 'machine', so we try to be smart
# and guess the snapshot was generated with kvm 1.4
(pc-i440fx-1.4).
@@ -361,7 +378,8 @@ sub __snapshot_rollback_vm_start {
my ($class, $vmid, $vmstate, $data) = @_;
my $storecfg = PVE::Storage::config();
- PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef,
undef, undef, $data->{forcemachine});
+ PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef,
undef, undef,
+ $data->{forcemachine}, undef, undef, undef, undef, undef, undef,
$data->{forcecpu});
}
sub __snapshot_rollback_get_unused {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 98c2a9a..70a5234 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -567,8 +567,15 @@ EODESCR
optional => 1,
}),
runningmachine => get_standard_option('pve-qemu-machine', {
- description => "Specifies the Qemu machine type of the running
vm. This is used internally for snapshots.",
+ description => "Specifies the QEMU machine type of the running
vm. This is used internally for snapshots.",
}),
+ runningcpu => {
+ description => "Specifies the QEMU '-cpu' parameter of the
running vm. This is used internally for snapshots.",
+ optional => 1,
+ type => 'string',
+ pattern => $PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re,
+ format_description => 'QEMU -cpu parameter'
+ },
machine => get_standard_option('pve-qemu-machine'),
arch => {
description => "Virtual
Re: [pve-devel] [PATCH v9 qemu-server 2/6] Include "-cpu" parameter with snapshots/suspend
On 26.03.20 16:13, Stefan Reiter wrote:
Just like with live-migration, custom CPU models might change after a
snapshot has been taken (or a VM suspended), which would lead to a
different QEMU invocation on rollback/resume.
Save the "-cpu" argument as a new "runningcpu" option into the VM conf
akin to "runningmachine" and use as override during rollback/resume.
No functional change with non-custom CPU types intended.
The changes apply to all CPU types, but that's not a bad thing. If one
takes a snapshot, restarts the VM with a different CPU type and does a
rollback, it'll use the CPU at the time of the snapshot even if no
custom models are involved.
Signed-off-by: Stefan Reiter
---
PVE/API2/Qemu.pm | 1 +
PVE/QemuConfig.pm | 34 ++
PVE/QemuServer.pm | 28
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 80fd7ea..2c7e998 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1129,6 +1129,7 @@ my $update_vm_api = sub {
push @delete, 'lock'; # this is the real deal to write it out
}
push @delete, 'runningmachine' if $conf->{runningmachine};
+ push @delete, 'runningcpu' if $conf->{runningcpu};
}
PVE::QemuConfig->check_lock($conf) if !$skiplock;
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 8d03774..386223d 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -5,6 +5,7 @@ use warnings;
use PVE::AbstractConfig;
use PVE::INotify;
+use PVE::QemuServer::CPUConfig;
use PVE::QemuServer::Drive;
use PVE::QemuServer::Helpers;
use PVE::QemuServer::Monitor qw(mon_cmd);
@@ -155,15 +156,26 @@ sub __snapshot_save_vmstate {
my $statefile = PVE::Storage::vdisk_alloc($storecfg, $target, $vmid,
'raw', $name, $size*1024);
my $runningmachine =
PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
-if ($suspend) {
- $conf->{vmstate} = $statefile;
- $conf->{runningmachine} = $runningmachine;
-} else {
- my $snap = $conf->{snapshots}->{$snapname};
- $snap->{vmstate} = $statefile;
- $snap->{runningmachine} = $runningmachine;
+# get current QEMU -cpu argument
+my $runningcpu;
+if (my $pid = PVE::QemuServer::check_running($vmid)) {
+ my $cmdline = PVE::QemuServer::Helpers::parse_cmdline($pid);
+ die "could not read commandline of running machine\n"
+ if !$cmdline->{cpu}->{value};
+
+ # sanitize and untaint value
+ $cmdline->{cpu}->{value} =~
$PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re;
+ $runningcpu = $1;
+}
+
+if (!$suspend) {
+ $conf = $conf->{snapshots}->{$snapname};
}
+$conf->{vmstate} = $statefile;
+$conf->{runningmachine} = $runningmachine;
+$conf->{runningcpu} = $runningcpu;
+
return $statefile;
}
@@ -309,6 +321,11 @@ sub __snapshot_rollback_hook {
if (defined($conf->{runningmachine})) {
$data->{forcemachine} = $conf->{runningmachine};
delete $conf->{runningmachine};
+
+ # runningcpu is newer than runningmachine, so assume it only exists
+ # here, if at all
+ $data->{forcecpu} = delete $conf->{runningcpu}
+ if defined($conf->{runningcpu});
} else {
# Note: old code did not store 'machine', so we try to be smart
# and guess the snapshot was generated with kvm 1.4 (pc-i440fx-1.4).
@@ -361,7 +378,8 @@ sub __snapshot_rollback_vm_start {
my ($class, $vmid, $vmstate, $data) = @_;
my $storecfg = PVE::Storage::config();
-PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef, undef, undef,
$data->{forcemachine});
+PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef, undef, undef,
+ $data->{forcemachine}, undef, undef, undef, undef, undef, undef,
$data->{forcecpu});
}
sub __snapshot_rollback_get_unused {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 98c2a9a..70a5234 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -567,8 +567,15 @@ EODESCR
optional => 1,
}),
runningmachine => get_standard_option('pve-qemu-machine', {
- description => "Specifies the Qemu machine type of the running vm. This is
used internally for snapshots.",
+ description => "Specifies the QEMU machine type of the running vm. This is
used internally for snapshots.",
}),
+runningcpu => {
+ description => "Specifies the QEMU '-cpu' parameter of the running vm. This
is used internally for snapshots.",
+ optional => 1,
+ type => 'string',
+ pattern => $PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re,
+ format_description => 'QEMU -cpu parameter'
+},
machine => get_standard_option('pve-qemu-machine'),
arch => {
description => "Virtual processor architecture. Defaults to the host.",
@@ -1948,7 +1955,8 @@
[pve-devel] [PATCH v9 qemu-server 2/6] Include "-cpu" parameter with snapshots/suspend
Just like with live-migration, custom CPU models might change after a
snapshot has been taken (or a VM suspended), which would lead to a
different QEMU invocation on rollback/resume.
Save the "-cpu" argument as a new "runningcpu" option into the VM conf
akin to "runningmachine" and use as override during rollback/resume.
No functional change with non-custom CPU types intended.
Signed-off-by: Stefan Reiter
---
PVE/API2/Qemu.pm | 1 +
PVE/QemuConfig.pm | 34 ++
PVE/QemuServer.pm | 28
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 80fd7ea..2c7e998 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1129,6 +1129,7 @@ my $update_vm_api = sub {
push @delete, 'lock'; # this is the real deal to write it out
}
push @delete, 'runningmachine' if $conf->{runningmachine};
+ push @delete, 'runningcpu' if $conf->{runningcpu};
}
PVE::QemuConfig->check_lock($conf) if !$skiplock;
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 8d03774..386223d 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -5,6 +5,7 @@ use warnings;
use PVE::AbstractConfig;
use PVE::INotify;
+use PVE::QemuServer::CPUConfig;
use PVE::QemuServer::Drive;
use PVE::QemuServer::Helpers;
use PVE::QemuServer::Monitor qw(mon_cmd);
@@ -155,15 +156,26 @@ sub __snapshot_save_vmstate {
my $statefile = PVE::Storage::vdisk_alloc($storecfg, $target, $vmid,
'raw', $name, $size*1024);
my $runningmachine =
PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
-if ($suspend) {
- $conf->{vmstate} = $statefile;
- $conf->{runningmachine} = $runningmachine;
-} else {
- my $snap = $conf->{snapshots}->{$snapname};
- $snap->{vmstate} = $statefile;
- $snap->{runningmachine} = $runningmachine;
+# get current QEMU -cpu argument
+my $runningcpu;
+if (my $pid = PVE::QemuServer::check_running($vmid)) {
+ my $cmdline = PVE::QemuServer::Helpers::parse_cmdline($pid);
+ die "could not read commandline of running machine\n"
+ if !$cmdline->{cpu}->{value};
+
+ # sanitize and untaint value
+ $cmdline->{cpu}->{value} =~
$PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re;
+ $runningcpu = $1;
+}
+
+if (!$suspend) {
+ $conf = $conf->{snapshots}->{$snapname};
}
+$conf->{vmstate} = $statefile;
+$conf->{runningmachine} = $runningmachine;
+$conf->{runningcpu} = $runningcpu;
+
return $statefile;
}
@@ -309,6 +321,11 @@ sub __snapshot_rollback_hook {
if (defined($conf->{runningmachine})) {
$data->{forcemachine} = $conf->{runningmachine};
delete $conf->{runningmachine};
+
+ # runningcpu is newer than runningmachine, so assume it only exists
+ # here, if at all
+ $data->{forcecpu} = delete $conf->{runningcpu}
+ if defined($conf->{runningcpu});
} else {
# Note: old code did not store 'machine', so we try to be smart
# and guess the snapshot was generated with kvm 1.4 (pc-i440fx-1.4).
@@ -361,7 +378,8 @@ sub __snapshot_rollback_vm_start {
my ($class, $vmid, $vmstate, $data) = @_;
my $storecfg = PVE::Storage::config();
-PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef, undef, undef,
$data->{forcemachine});
+PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef, undef, undef,
+ $data->{forcemachine}, undef, undef, undef, undef, undef, undef,
$data->{forcecpu});
}
sub __snapshot_rollback_get_unused {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 98c2a9a..70a5234 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -567,8 +567,15 @@ EODESCR
optional => 1,
}),
runningmachine => get_standard_option('pve-qemu-machine', {
- description => "Specifies the Qemu machine type of the running vm. This
is used internally for snapshots.",
+ description => "Specifies the QEMU machine type of the running vm. This
is used internally for snapshots.",
}),
+runningcpu => {
+ description => "Specifies the QEMU '-cpu' parameter of the running vm.
This is used internally for snapshots.",
+ optional => 1,
+ type => 'string',
+ pattern => $PVE::QemuServer::CPUConfig::qemu_cmdline_cpu_re,
+ format_description => 'QEMU -cpu parameter'
+},
machine => get_standard_option('pve-qemu-machine'),
arch => {
description => "Virtual processor architecture. Defaults to the host.",
@@ -1948,7 +1955,8 @@ sub json_config_properties {
my $prop = shift;
foreach my $opt (keys %$confdesc) {
- next if $opt eq 'parent' || $opt eq 'snaptime' || $opt eq 'vmstate' ||
$opt eq 'runningmachine';
+ next if $opt eq 'parent' || $opt eq 'snaptime' || $opt eq 'vmstate' ||
+ $opt eq 'runningmachine' ||
