On Thu, Sep 13, 2018 at 12:15:05PM +0200, Dominik Csapak wrote: > instead of overwriting the 'machine' config in the snapshot, > use its own 'runningmachine' config only for the snapshot > > this way, we do not lose the machine type if it was > explicitely set during the snapshot, but deleted afterwards > > we also have to adapt the tests for this
LGTM, small nit inline. > > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> > --- > PVE/QemuConfig.pm | 26 > +++++++++++++--------- > PVE/QemuServer.pm | 9 +++++++- > test/snapshot-expected/create/qemu-server/102.conf | 2 +- > test/snapshot-expected/create/qemu-server/104.conf | 2 +- > test/snapshot-expected/create/qemu-server/106.conf | 2 +- > .../snapshot-expected/prepare/qemu-server/102.conf | 2 +- > .../snapshot-expected/prepare/qemu-server/104.conf | 2 +- > test/snapshot-test.pm | 2 +- > 8 files changed, 30 insertions(+), 17 deletions(-) > > diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm > index 5cc2d48..35ed9b6 100644 > --- a/PVE/QemuConfig.pm > +++ b/PVE/QemuConfig.pm > @@ -146,9 +146,7 @@ sub __snapshot_save_vmstate { > my $scfg = PVE::Storage::storage_config($storecfg, $target); > $name .= ".raw" if $scfg->{path}; # add filename extension for file base > storage > $snap->{vmstate} = PVE::Storage::vdisk_alloc($storecfg, $target, $vmid, > 'raw', $name, $size*1024); > - # always overwrite machine if we save vmstate. This makes sure we > - # can restore it later using correct machine type > - $snap->{machine} = PVE::QemuServer::get_current_qemu_machine($vmid); > + $snap->{runningmachine} = > PVE::QemuServer::get_current_qemu_machine($vmid); > } > > sub __snapshot_check_running { > @@ -288,13 +286,21 @@ sub __snapshot_rollback_hook { > # we save the machine of the current config > $data->{oldmachine} = $conf->{machine}; > } 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). > - $data->{forcemachine} = $conf->{machine} || 'pc-i440fx-1.4'; > - > - # we remove the 'machine' configuration if not explicitly specified > - # in the original config. > - delete $conf->{machine} if $snap->{vmstate} && > !defined($data->{oldmachine}); > + # if we have a 'runningmachine' entry in the snapshot > + # we use that for the forcemachine parameter, > + # else we use the old logic > + if (defined($conf->{runningmachine})) { > + $data->{forcemachine} = $conf->{runningmachine}; > + delete $conf->{runningmachine}; > + } 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). > + $data->{forcemachine} = $conf->{machine} || 'pc-i440fx-1.4'; > + > + # we remove the 'machine' configuration if not explicitly specified > + # in the original config. > + delete $conf->{machine} if $snap->{vmstate} && > !defined($data->{oldmachine}); > + } > } > > return; > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 015f8f7..a3e602d 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -528,6 +528,13 @@ EODESCR > description => "Default storage for VM state volumes/files.", > optional => 1, > }), > + runningmachine => { > + description => "Specifies the Qemu machine type of the running vm. This > is used internally for snapshots.", > + type => 'string', > + pattern => > '(pc|pc(-i440fx)?-\d+\.\d+(\.pxe)?|q35|pc-q35-\d+\.\d+(\.pxe)?)', > + maxLength => 40, > + optional => 1, IMHO this should become a standard option and used for all occurences. > + }, > machine => { > description => "Specific the Qemu machine type.", > type => 'string', > @@ -2255,7 +2262,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 'runningmachine'; > $prop->{$opt} = $confdesc->{$opt}; > } > > diff --git a/test/snapshot-expected/create/qemu-server/102.conf > b/test/snapshot-expected/create/qemu-server/102.conf > index c521154..9b57004 100644 > --- a/test/snapshot-expected/create/qemu-server/102.conf > +++ b/test/snapshot-expected/create/qemu-server/102.conf > @@ -20,12 +20,12 @@ bootdisk: ide0 > cores: 4 > ide0: local:snapshotable-disk-1,discard=on,size=32G > ide2: none,media=cdrom > -machine: somemachine > memory: 8192 > name: win > net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1 > numa: 0 > ostype: win7 > +runningmachine: somemachine > smbios1: uuid=01234567-890a-bcde-f012-34567890abcd > snaptime: 1234567890 > sockets: 1 > diff --git a/test/snapshot-expected/create/qemu-server/104.conf > b/test/snapshot-expected/create/qemu-server/104.conf > index ef7285e..54f1c21 100644 > --- a/test/snapshot-expected/create/qemu-server/104.conf > +++ b/test/snapshot-expected/create/qemu-server/104.conf > @@ -39,13 +39,13 @@ bootdisk: ide0 > cores: 4 > ide0: local:snapshotable-disk-1,discard=on,size=32G > ide2: none,media=cdrom > -machine: somemachine > memory: 8192 > name: win > net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1 > numa: 0 > ostype: win7 > parent: test > +runningmachine: somemachine > smbios1: uuid=01234567-890a-bcde-f012-34567890abcd > snaptime: 1234567890 > sockets: 1 > diff --git a/test/snapshot-expected/create/qemu-server/106.conf > b/test/snapshot-expected/create/qemu-server/106.conf > index c521154..9b57004 100644 > --- a/test/snapshot-expected/create/qemu-server/106.conf > +++ b/test/snapshot-expected/create/qemu-server/106.conf > @@ -20,12 +20,12 @@ bootdisk: ide0 > cores: 4 > ide0: local:snapshotable-disk-1,discard=on,size=32G > ide2: none,media=cdrom > -machine: somemachine > memory: 8192 > name: win > net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1 > numa: 0 > ostype: win7 > +runningmachine: somemachine > smbios1: uuid=01234567-890a-bcde-f012-34567890abcd > snaptime: 1234567890 > sockets: 1 > diff --git a/test/snapshot-expected/prepare/qemu-server/102.conf > b/test/snapshot-expected/prepare/qemu-server/102.conf > index 4ab1787..92db74a 100644 > --- a/test/snapshot-expected/prepare/qemu-server/102.conf > +++ b/test/snapshot-expected/prepare/qemu-server/102.conf > @@ -18,12 +18,12 @@ bootdisk: ide0 > cores: 4 > ide0: somestore:somedisk,discard=on,size=32G > ide2: none,media=cdrom > -machine: somemachine > memory: 8192 > name: win > net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1 > numa: 0 > ostype: win7 > +runningmachine: somemachine > smbios1: uuid=01234567-890a-bcde-f012-34567890abcd > snapstate: prepare > snaptime: 1234567890 > diff --git a/test/snapshot-expected/prepare/qemu-server/104.conf > b/test/snapshot-expected/prepare/qemu-server/104.conf > index b62f2c6..02e2d3c 100644 > --- a/test/snapshot-expected/prepare/qemu-server/104.conf > +++ b/test/snapshot-expected/prepare/qemu-server/104.conf > @@ -35,13 +35,13 @@ bootdisk: ide0 > cores: 4 > ide0: somestore:somedisk,discard=on,size=32G > ide2: none,media=cdrom > -machine: somemachine > memory: 8192 > name: win > net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1 > numa: 0 > ostype: win7 > parent: test > +runningmachine: somemachine > smbios1: uuid=01234567-890a-bcde-f012-34567890abcd > snapstate: prepare > snaptime: 1234567890 > diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm > index d95d77f..8988942 100644 > --- a/test/snapshot-test.pm > +++ b/test/snapshot-test.pm > @@ -295,7 +295,7 @@ sub __snapshot_save_vmstate { > > my $snap = $conf->{snapshots}->{$snapname}; > $snap->{vmstate} = "somestorage:state-volume"; > - $snap->{machine} = "somemachine"; > + $snap->{runningmachine} = "somemachine" > } > # END mocked PVE::QemuConfig methods > > -- > 2.11.0 > > > _______________________________________________ > pve-devel mailing list > pve-devel@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel