[pve-devel] [PATCH] add a forcemachine parameter to force a specific qemu machine version
Stefan Priebe - Profihost AG
s.priebe at profihost.ag
Mon Jun 3 11:10:21 CEST 2013
OK,
is there any recomand way to split a patch into subpatches with git?
Stefan
Am 03.06.2013 10:53, schrieb 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
>
More information about the pve-devel
mailing list