[pve-devel] [PATCH qemu-server 2/4] improve snapshot machine logic
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Sep 13 12:54:11 CEST 2018
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.csapak at 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 at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
More information about the pve-devel
mailing list