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