[pve-devel] [PATCH qemu-server v2 2/3] improve snapshot machine logic

Dominik Csapak d.csapak at proxmox.com
Fri Sep 14 14:08:43 CEST 2018


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

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
changes from v1:
* refactored machine type regex into a standard-option
* includes the typo fix

 PVE/QemuConfig.pm                                  | 26 +++++++++++++---------
 PVE/QemuServer.pm                                  | 21 ++++++++++-------
 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, 35 insertions(+), 24 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index cf34ec5..de564b7 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..101e3ce 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -80,6 +80,14 @@ PVE::JSONSchema::register_standard_option('pve-qm-image-format', {
     optional => 1,
 });
 
+PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
+	description => "Specifies the Qemu machine type.",
+	type => 'string',
+	pattern => '(pc|pc(-i440fx)?-\d+\.\d+(\.pxe)?|q35|pc-q35-\d+\.\d+(\.pxe)?)',
+	maxLength => 40,
+	optional => 1,
+});
+
 #no warnings 'redefine';
 
 sub cgroups_write {
@@ -528,13 +536,10 @@ EODESCR
 	description => "Default storage for VM state volumes/files.",
 	optional => 1,
     }),
-    machine => {
-	description => "Specific the Qemu machine type.",
-	type => 'string',
-	pattern => '(pc|pc(-i440fx)?-\d+\.\d+(\.pxe)?|q35|pc-q35-\d+\.\d+(\.pxe)?)',
-	maxLength => 40,
-	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.",
+    }),
+    machine => get_standard_option('pve-qemu-machine'),
     smbios1 => {
 	description => "Specify SMBIOS type 1 fields.",
 	type => 'string', format => 'pve-qm-smbios1',
@@ -2255,7 +2260,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





More information about the pve-devel mailing list