[pve-devel] [PATCH] add a forcemachine parameter to force a specific qemu machine version

[Dietmar Maurer]

I would prefer to split that patch into several pieces:

a.) add support for --machine configuration option
b.) add 'machine' option to vm_start
c.) use --machine for migrate
d.) use --machine for snapshot/rollback

 Further comments inline.


> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index
> 8ed9d5d..3cc66e8 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1327,7 +1327,7 @@ __PACKAGE__->register_method({
>  	    skiplock => get_standard_option('skiplock'),
>  	    stateuri => get_standard_option('pve-qm-stateuri'),
>  	    migratedfrom => get_standard_option('pve-node',{ optional => 1
> }),
> -
> +	    forcemachine => get_standard_option('forcemachine',{ optional
> => 1

can we simply name if 'machine' (instead of forcemachine)?

> +}),
>  	},
>      },
>      returns => {
> @@ -1356,6 +1356,8 @@ __PACKAGE__->register_method({
>  	raise_param_exc({ migratedfrom => "Only root may use this option."
> })
>  	    if $migratedfrom && $authuser ne 'root at pam';
> 
> +	my $forcemachine = extract_param($param, 'forcemachine');
> +
>  	my $storecfg = PVE::Storage::config();
> 
>  	if (&$vm_is_ha_managed($vmid) && !$stateuri && @@ -1384,7
> +1386,7 @@ __PACKAGE__->register_method({
> 
>  		syslog('info', "start VM $vmid: $upid\n");
> 
> -		PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri,
> $skiplock, $migratedfrom);
> +		PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri,
> $skiplock,
> +$migratedfrom, undef, $forcemachine);
> 
>  		return;
>  	    };
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm index
> aa960ae..15d6c06 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -166,6 +166,10 @@ sub prepare {
>      eval { $self->cmd_quiet($cmd); };
>      die "Can't connect to destination address using public key\n" if $@;
> 
> +    # save running qemu version
> +    my $current_machine =
> PVE::QemuServer::get_current_qemu_machine($vmid);
> +    $self->{current_machine} = $current_machine if ($current_machine);
> +

again, can we simply add a configuration option called 'machine'?

>      return $running;
>  }
> 
> @@ -317,7 +321,7 @@ sub phase2 {
> 
>      ## start on remote node
>      my $cmd = [@{$self->{rem_ssh}}, 'qm', 'start',
> -               $vmid, '--stateuri', 'tcp', '--skiplock', '--migratedfrom', $nodename];
> +               $vmid, '--stateuri', 'tcp', '--skiplock',
> + '--migratedfrom', $nodename, '--forcemachine',
> + $self->{current_machine} ];
> 
>      PVE::Tools::run_command($cmd, outfunc => sub {
>  	my $line = shift;
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index
> 440f4a1..7642f0f 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -49,6 +49,13 @@
> PVE::JSONSchema::register_standard_option('skiplock', {
>      optional => 1,
>  });
> 
> +PVE::JSONSchema::register_standard_option('forcemachine', {
> +    description => "Force a specific qemu machine type.",
> +    type => 'string',
> +    maxLength => 128,
> +    optional => 1,
> +});

Can we have a simple parser for that type?

> +
>  PVE::JSONSchema::register_standard_option('pve-qm-stateuri', {
>      description => "Some command save/restore state from this location.",
>      type => 'string',
> @@ -136,6 +143,26 @@ sub fairsched_cpulimit {
>      return fairsched_rate($id, $op, $cpulim1024);  }
> 
> +my $minimal_qemu_machine = "pc-i440fx-1.4";
> +
> +sub get_current_qemu_machine {
> +    my ($vmid) = @_;
> +
> +    my $machine = eval {
> +        my $cmd = { execute => 'query-machines', arguments => {} };
> +        my $res = PVE::QemuServer::vm_qmp_command($vmid, $cmd);
> +
> +        my $cur = eval { (grep { $_->{'is-current'} } @$res)[0]->{name} };
> +        my $def = eval { (grep { $_->{'is-default'} } @$res)[0]->{name}
> + };
> +
> +        # fallback to the default machine if current is not supported by qemu
> +        return $cur || $def;
> +    };
> +
> +    # could be undef if machine is not running
> +    return $machine;
> +}
> +
>  my $nodename = PVE::INotify::nodename();
> 
>  mkdir "/etc/pve/nodes/$nodename";
> @@ -416,6 +443,11 @@ EODESCR
>  	type => 'integer',
>  	minimum => 0,
>      },
> +    current_machine => {
> +	optional => 1,
> +	description => "Currently running qemu machine type.",
> +	type => 'string',
> +    },
>      vmstate => {
>  	optional => 1,
>  	type => 'string', format => 'pve-volume-id', @@ -1439,7 +1471,7 @@
> sub json_config_properties {
>      my $prop = shift;
> 
>      foreach my $opt (keys %$confdesc) {
> -	next if $opt eq 'parent' || $opt eq 'snaptime' || $opt eq 'vmstate';
> +	next if $opt eq 'parent' || $opt eq 'snaptime' || $opt eq 'vmstate' ||
> +$opt eq 'current_machine';
>  	$prop->{$opt} = $confdesc->{$opt};
>      }
> 
> @@ -2230,7 +2262,7 @@ sub foreach_volid {  }
> 
>  sub config_to_command {
> -    my ($storecfg, $vmid, $conf, $defaults) = @_;
> +    my ($storecfg, $vmid, $conf, $defaults, $forcemachine) = @_;
> 
>      my $cmd = [];
>      my $globalFlags = [];
> @@ -2250,6 +2282,10 @@ sub config_to_command {
> 
>      push @$cmd, '-id', $vmid;
> 
> +    # if we restore an old snapshot or migrate from an older qemu version
> +    # we need to force the original machine version
> +    push @$machineFlags, $forcemachine if $forcemachine;
> +
>      my $use_virtio = 0;
> 
>      my $qmpsocket = qmp_socket($vmid);
> @@ -2974,7 +3010,7 @@ sub qga_unfreezefs {  }
> 
>  sub vm_start {
> -    my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused) =
> @_;
> +    my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom,
> + $paused, $forcemachine) = @_;
> 
>      lock_config($vmid, sub {
>  	my $conf = load_config($vmid, $migratedfrom); @@ -2990,7 +3026,7
> @@ sub vm_start {
>  	# set environment variable useful inside network script
>  	$ENV{PVE_MIGRATED_FROM} = $migratedfrom if $migratedfrom;
> 
> -	my ($cmd, $vollist) = config_to_command($storecfg, $vmid, $conf,
> $defaults);
> +	my ($cmd, $vollist) = config_to_command($storecfg, $vmid, $conf,
> +$defaults, $forcemachine);
> 
>  	my $migrate_port = 0;
>  	my $migrate_uri;
> @@ -4133,6 +4169,7 @@ my $snapshot_copy_config = sub {
>  	next if $k eq 'snapshots';
>  	next if $k eq 'snapstate';
>  	next if $k eq 'snaptime';
> +	next if $k eq 'current_machine';
>  	next if $k eq 'vmstate';
>  	next if $k eq 'lock';
>  	next if $k eq 'digest';
> @@ -4259,6 +4296,8 @@ my $snapshot_prepare = sub {
>  	$snap->{snapstate} = "prepare";
>  	$snap->{snaptime} = time();
>  	$snap->{description} = $comment if $comment;
> +	my $current_machine = get_current_qemu_machine($vmid);
> +	$snap->{current_machine} = $current_machine if
> ($current_machine);
> 
>  	update_config_nolock($vmid, $conf, 1);
>      };
> @@ -4345,7 +4384,9 @@ sub snapshot_rollback {
> 
>  	if (!$prepare && $snap->{vmstate}) {
>  	    my $statefile = PVE::Storage::path($storecfg, $snap->{vmstate});
> -	    vm_start($storecfg, $vmid, $statefile);
> +
> +	    # machine was not saved at snapshot time so assume the minimal
> version
> +	    vm_start($storecfg, $vmid, $statefile, undef, undef, undef,
> +($snap->{current_machine} || $minimal_qemu_machine));
>  	}
>      };
> 
> --
> 1.7.10.4