[pve-devel] [PATCH v8 qemu-server 3/8] Include "-cpu" parameter with live-migration

[Thomas Lamprecht]

On 2/12/20 4:11 PM, Stefan Reiter wrote:
> This is required to support custom CPU models, since the
> "cpu-models.conf" file is not versioned, and can be changed while a VM
> using a custom model is running. Changing the file in such a state can
> lead to a different "-cpu" argument on the receiving side.
> 
> This patch fixes this by passing the entire "-cpu" option (extracted
> from /proc/.../cmdline) as a "qm start" parameter. Note that this is
> only done if the VM to migrate is using a custom model (which we can
> check just fine, since the <vmid>.conf *is* versioned with pending
> changes), thus not breaking any live-migration directionality.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> New in v6.

I get a merge conflict with this one. Could you please rebase the remaining
patches on current master?	

Fabi, once Stefan got to sent the rebase, it would be great if you could take
a look at this. Thanks!

> 
>  PVE/API2/Qemu.pm            |  9 ++++++++-
>  PVE/QemuMigrate.pm          | 22 ++++++++++++++++++++++
>  PVE/QemuServer.pm           | 14 ++++++++++----
>  PVE/QemuServer/CPUConfig.pm |  2 ++
>  4 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index caca430..5ea6cf2 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1986,6 +1986,11 @@ __PACKAGE__->register_method({
>  		optional => 1,
>  	    },
>  	    machine => get_standard_option('pve-qemu-machine'),
> +	    'force-cpu' => {
> +		description => "Override QEMU's -cpu argument with the given string.",
> +		type => 'string',
> +		optional => 1,
> +	    },
>  	    targetstorage => {
>  		description => "Target storage for the migration. (Can be '1' to use the same storage id as on the source node.)",
>  		type => 'string',
> @@ -2014,6 +2019,7 @@ __PACKAGE__->register_method({
>  	my $timeout = extract_param($param, 'timeout');
>  
>  	my $machine = extract_param($param, 'machine');
> +	my $force_cpu = extract_param($param, 'force-cpu');
>  
>  	my $get_root_param = sub {
>  	    my $value = extract_param($param, $_[0]);
> @@ -2066,7 +2072,8 @@ __PACKAGE__->register_method({
>  		syslog('info', "start VM $vmid: $upid\n");
>  
>  		PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri, $skiplock, $migratedfrom, undef, $machine,
> -					  $spice_ticket, $migration_network, $migration_type, $targetstorage, $timeout);
> +					  $spice_ticket, $migration_network, $migration_type, $targetstorage, $timeout,
> +					  $force_cpu);
>  		return;
>  	    };
>  
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 6db5e62..de27328 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -11,6 +11,8 @@ use PVE::Tools;
>  use PVE::Cluster;
>  use PVE::Storage;
>  use PVE::QemuServer;
> +use PVE::QemuServer::CPUConfig;
> +use PVE::QemuServer::Helpers;
>  use PVE::QemuServer::Machine;
>  use PVE::QemuServer::Monitor qw(mon_cmd);
>  use Time::HiRes qw( usleep );
> @@ -219,7 +221,23 @@ sub prepare {
>  
>  	$self->{forcemachine} = PVE::QemuServer::Machine::qemu_machine_pxe($vmid, $conf);
>  
> +	# To support custom CPU types, we keep QEMU's "-cpu" parameter intact.
> +	# Since the parameter itself contains no reference to a custom model,
> +	# this makes migration independent of changes to "cpu-models.conf".
> +	if ($conf->{cpu}) {
> +	    my $cpuconf = PVE::QemuServer::CPUConfig::parse_cpu_conf_basic($conf->{cpu});
> +	    if ($cpuconf && PVE::QemuServer::CPUConfig::is_custom_model($cpuconf->{cputype})) {
> +		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;
> +		$self->{forcecpu} = $1;
> +	    }
> +	}
>      }
> +
>      my $loc_res = PVE::QemuServer::check_local_resources($conf, 1);
>      if (scalar @$loc_res) {
>  	if ($self->{running} || !$self->{opts}->{force}) {
> @@ -582,6 +600,10 @@ sub phase2 {
>  	push @$cmd, '--machine', $self->{forcemachine};
>      }
>  
> +    if ($self->{forcecpu}) {
> +	push @$cmd, '--force-cpu', $self->{forcecpu};
> +    }
> +
>      if ($self->{online_local_volumes}) {
>  	push @$cmd, '--targetstorage', ($self->{opts}->{targetstorage} // '1');
>      }
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index f0c8abd..4252b68 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3406,7 +3406,7 @@ sub query_understood_cpu_flags {
>  }
>  
>  sub config_to_command {
> -    my ($storecfg, $vmid, $conf, $defaults, $forcemachine) = @_;
> +    my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $force_cpu) = @_;
>  
>      my $cmd = [];
>      my $globalFlags = [];
> @@ -3804,7 +3804,12 @@ sub config_to_command {
>  	push @$rtcFlags, 'base=localtime';
>      }
>  
> -    push @$cmd, get_cpu_options($conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough);
> +    if ($force_cpu) {
> +	push @$cmd, '-cpu', $force_cpu;
> +    } else {
> +	push @$cmd, get_cpu_options($conf, $arch, $kvm, $kvm_off,
> +	    $machine_version, $winversion, $gpu_passthrough);
> +    }
>  
>      PVE::QemuServer::Memory::config($conf, $vmid, $sockets, $cores, $defaults, $hotplug_features, $cmd);
>  
> @@ -5190,7 +5195,8 @@ sub vmconfig_update_disk {
>  
>  sub vm_start {
>      my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused,
> -	$forcemachine, $spice_ticket, $migration_network, $migration_type, $targetstorage, $timeout) = @_;
> +	$forcemachine, $spice_ticket, $migration_network, $migration_type,
> +	$targetstorage, $timeout, $force_cpu) = @_;
>  
>      PVE::QemuConfig->lock_config($vmid, sub {
>  	my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
> @@ -5279,7 +5285,7 @@ sub vm_start {
>  	    print "Resuming suspended VM\n";
>  	}
>  
> -	my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine);
> +	my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine, $force_cpu);
>  
>  	my $migration_ip;
>  	my $get_migration_ip = sub {
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 65f0d76..315ef75 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -99,6 +99,8 @@ my @supported_cpu_flags = (
>  my $cpu_flag_supported_re = qr/([+-])(@{[join('|', @supported_cpu_flags)]})/;
>  my $cpu_flag_any_re = qr/([+-])([a-zA-Z0-9\-_\.]+)/;
>  
> +our $qemu_cmdline_cpu_re = qr/^((?>[+-]?[\w\-_=]+,?)+)$/;
> +
>  my $cpu_fmt = {
>      cputype => {
>  	description => "Emulated CPU type. Can be default or custom name (custom model names must be prefixed with 'custom-').",
>