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 <s.rei...@proxmox.com>
---
  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' || $opt eq 'runningcpu';
      $prop->{$opt} = $confdesc->{$opt};
      }
@@ -4802,8 +4810,9 @@ sub vm_start {
      PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-start', 1);
      if ($is_suspended) {
-        # enforce machine type on suspended vm to ensure HW compatibility +        # enforce machine type and CPU on suspended vm to ensure HW compatibility
          $forcemachine = $conf->{runningmachine};
+        $force_cpu = $conf->{runningcpu};
          print "Resuming suspended VM\n";
      }
@@ -5058,7 +5067,7 @@ sub vm_start {
          PVE::Storage::deactivate_volumes($storecfg, [$vmstate]);
          PVE::Storage::vdisk_free($storecfg, $vmstate);
          }
-        delete $conf->@{qw(lock vmstate runningmachine)};
+        delete $conf->@{qw(lock vmstate runningmachine runningcpu)};
          PVE::QemuConfig->write_config($vmid, $conf);
      }
@@ -5071,13 +5080,15 @@ sub vm_commandline {
      my $conf = PVE::QemuConfig->load_config($vmid);
      my $forcemachine;
+    my $forcecpu;
      if ($snapname) {
      my $snapshot = $conf->{snapshots}->{$snapname};
      die "snapshot '$snapname' does not exist\n" if !defined($snapshot);
-    # check for a 'runningmachine' in snapshot
-    $forcemachine = $snapshot->{runningmachine} if $snapshot->{runningmachine};
+    # check for machine or CPU overrides in snapshot
+    $forcemachine = $snapshot->{runningmachine};
+    $forcecpu = $snapshot->{runningcpu};
      $snapshot->{digest} = $conf->{digest}; # keep file digest for API
@@ -5086,7 +5097,8 @@ sub vm_commandline {
      my $defaults = load_defaults();
-    my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine);
+    my $cmd = config_to_command($storecfg, $vmid, $conf, $defaults,
+    $forcemachine, $forcecpu);
      return PVE::Tools::cmd2string($cmd);
  }
@@ -5360,7 +5372,7 @@ sub vm_suspend {
              mon_cmd($vmid, "savevm-end");
              PVE::Storage::deactivate_volumes($storecfg, [$vmstate]);
              PVE::Storage::vdisk_free($storecfg, $vmstate);
-            delete $conf->@{qw(vmstate runningmachine)};
+            delete $conf->@{qw(vmstate runningmachine runningcpu)};
              PVE::QemuConfig->write_config($vmid, $conf);
          };
          warn $@ if $@;


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to