On 1/16/20 4:40 PM, 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.
few general things:
1. wouldn't this belong before or in patch 03/10 to avoid temporary
breakage possibility? I mean a user has to actively enable it so
it could be OK, but still.
2. Doesn't this need a bump of the perversion for the machine? Else
this fails on migration with due to the new option, which could
be confusing/seem like a bug. Maybe only allow/add the whole custom
thing with a bumped machine version?
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
>
> New in v7.
>
> PVE/API2/Qemu.pm | 1 +
> PVE/QemuConfig.pm | 36 +++++++++++++++++++++++++++---------
> PVE/QemuServer.pm | 28 ++++++++++++++++++++--------
> 3 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index f0b9e58..c2f6513 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1098,6 +1098,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 1ba728a..852c309 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -5,9 +5,10 @@ use warnings;
>
> use PVE::AbstractConfig;
> use PVE::INotify;
> +use PVE::QemuServer;
> +use PVE::QemuServer::CPUConfig;
> use PVE::QemuServer::Helpers;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> -use PVE::QemuServer;
> use PVE::QemuServer::Machine;
> use PVE::Storage;
> use PVE::Tools;
> @@ -156,15 +157,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;
> }
>
> @@ -310,6 +322,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).
> @@ -362,7 +379,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,
> $data->{forcecpu});
> }
>
> sub __snapshot_rollback_get_unused {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 4a5dfac..d7b9c09 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -532,8 +532,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.",
> @@ -2364,7 +2371,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};
> }
>
> @@ -5231,8 +5239,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";
> }
>
> @@ -5478,7 +5487,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);
> }
>
> @@ -5491,13 +5500,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
>
> @@ -5506,7 +5517,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);
> }
> @@ -5780,7 +5792,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
