[pve-devel] [PATCH qemu-server 2/3] Rework snapshot code, has_feature

Fabian Grünbichler f.gruenbichler at proxmox.com
Mon Mar 7 12:41:13 CET 2016


Drop snapshot_create, snapshot_delete and snapshot_rollback
in favour of PVE::AbstractConfig. Qemu-specific parts are
implemented in __snapshot_XX methods in PVE::QemuConfig.

has_feature is made an implementation of the abstract
has_feature, and thus moves to PVE::QemuConfig.

Note: a new hook method needed to be introduced to be called
before creating a volume snapshot, after creating a volume
snapshot, and after unfreezing the guestfs after creating a
volume snapshot. The base method in PVE::AbstractConfig is a
noop, the implemention in PVE::QemuConfig runs the necessary
Qemu monitor commands.
---
 PVE/API2/Qemu.pm                                   |   8 +-
 PVE/QemuConfig.pm                                  | 228 ++++++++++
 PVE/QemuServer.pm                                  | 501 ---------------------
 test/snapshot-expected/delete/qemu-server/202.conf |   1 +
 test/snapshot-test.pm                              |  38 +-
 5 files changed, 254 insertions(+), 522 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index e4956cb..40ea353 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2042,7 +2042,7 @@ __PACKAGE__->register_method({
 	my $storecfg = PVE::Storage::config();
 
 	my $nodelist = PVE::QemuServer::shared_nodes($conf, $storecfg);
-	my $hasFeature = PVE::QemuServer::has_feature($feature, $conf, $storecfg, $snapname, $running);
+	my $hasFeature = PVE::QemuConfig->has_feature($feature, $conf, $storecfg, $snapname, $running);
 
 	return {
 	    hasFeature => $hasFeature,
@@ -2862,7 +2862,7 @@ __PACKAGE__->register_method({
 
 	my $realcmd = sub {
 	    PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname");
-	    PVE::QemuServer::snapshot_create($vmid, $snapname, $param->{vmstate}, 
+	    PVE::QemuConfig->snapshot_create($vmid, $snapname, $param->{vmstate}, 
 					     $param->{description});
 	};
 
@@ -3036,7 +3036,7 @@ __PACKAGE__->register_method({
 
 	my $realcmd = sub {
 	    PVE::Cluster::log_msg('info', $authuser, "rollback snapshot VM $vmid: $snapname");
-	    PVE::QemuServer::snapshot_rollback($vmid, $snapname);
+	    PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
 	};
 
 	return $rpcenv->fork_worker('qmrollback', $vmid, $authuser, $realcmd);
@@ -3084,7 +3084,7 @@ __PACKAGE__->register_method({
 
 	my $realcmd = sub {
 	    PVE::Cluster::log_msg('info', $authuser, "delete snapshot VM $vmid: $snapname");
-	    PVE::QemuServer::snapshot_delete($vmid, $snapname, $param->{force});
+	    PVE::QemuConfig->snapshot_delete($vmid, $snapname, $param->{force});
 	};
 
 	return $rpcenv->fork_worker('qmdelsnapshot', $vmid, $authuser, $realcmd);
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 219f3c3..c9c5e60 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -3,6 +3,12 @@ package PVE::QemuConfig;
 use strict;
 use warnings;
 
+use PVE::AbstractConfig;
+use PVE::INotify;
+use PVE::QemuServer;
+use PVE::Storage;
+use PVE::Tools;
+
 use base qw(PVE::AbstractConfig);
 
 my $nodename = PVE::INotify::nodename();
@@ -41,6 +47,228 @@ sub cfs_config_path {
     return "nodes/$node/qemu-server/$vmid.conf";
 }
 
+sub has_feature {
+    my ($class, $feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
+
+    my $err;
+    PVE::QemuServer::foreach_drive($conf, sub {
+	my ($ds, $drive) = @_;
+
+	return if PVE::QemuServer::drive_is_cdrom($drive);
+	return if $backup_only && !$drive->{backup};
+	my $volid = $drive->{file};
+	$err = 1 if !PVE::Storage::volume_has_feature($storecfg, $feature, $volid, $snapname, $running);
+   });
+
+    return $err ? 0 : 1;
+}
+
+sub __snapshot_save_vmstate {
+    my ($class, $vmid, $conf, $snapname, $storecfg) = @_;
+
+    my $snap = $conf->{snapshots}->{$snapname};
+
+    my $target;
+
+    # search shared storage first
+    PVE::QemuServer::foreach_writable_storage($conf, sub {
+	my ($sid) = @_;
+	my $scfg = PVE::Storage::storage_config($storecfg, $sid);
+	return if !$scfg->{shared};
+
+	$target = $sid if !$target || $scfg->{path}; # prefer file based storage
+    });
+
+    if (!$target) {
+	# now search local storage
+	PVE::QemuServer::foreach_writable_storage($conf, sub {
+	    my ($sid) = @_;
+	    my $scfg = PVE::Storage::storage_config($storecfg, $sid);
+	    return if $scfg->{shared};
+
+	    $target = $sid if !$target || $scfg->{path}; # prefer file based storage;
+	});
+    }
+
+    $target = 'local' if !$target;
+
+    my $driver_state_size = 500; # assume 32MB is enough to safe all driver state;
+    # we abort live save after $conf->{memory}, so we need at max twice that space
+    my $size = $conf->{memory}*2 + $driver_state_size;
+
+    my $name = "vm-$vmid-state-$snapname";
+    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);
+}
+
+sub __snapshot_check_running {
+    my ($class, $vmid) = @_;
+    return PVE::QemuServer::check_running($vmid);
+}
+
+sub __snapshot_check_freeze_needed {
+    my ($class, $vmid, $config, $save_vmstate) = @_;
+
+    my $running = $class->__snapshot_check_running($vmid);
+    if ($save_vmstate) {
+	return ($running, $running && $config->{agent} && PVE::QemuServer::qga_check_running($vmid));
+    } else {
+	return ($running, 0);
+    }
+}
+
+sub __snapshot_freeze {
+    my ($class, $vmid, $unfreeze) = @_;
+
+    if ($unfreeze) {
+	eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
+	warn "guest-fsfreeze-thaw problems - $@" if $@;
+    } else {
+	eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
+	warn "guest-fsfreeze-freeze problems - $@" if $@;
+    }
+}
+
+sub __snapshot_create_vol_snapshots_hook {
+    my ($class, $vmid, $snap, $running, $hook) = @_;
+
+    if ($running) {
+	if ($hook eq "before") {
+	    if ($snap->{vmstate}) {
+		my $storecfg = PVE::Storage::config();
+		my $path = PVE::Storage::path($storecfg, $snap->{vmstate});
+		PVE::QemuServer::vm_mon_cmd($vmid, "savevm-start", statefile => $path);
+		for(;;) {
+		    my $stat = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "query-savevm");
+		    if (!$stat->{status}) {
+			die "savevm not active\n";
+		    } elsif ($stat->{status} eq 'active') {
+			sleep(1);
+			next;
+		    } elsif ($stat->{status} eq 'completed') {
+			last;
+		    } else {
+			die "query-savevm returned status '$stat->{status}'\n";
+		    }
+		}
+	    } else {
+		PVE::QemuServer::vm_mon_cmd($vmid, "savevm-start");
+	    }
+	} elsif ($hook eq "after") {
+	    eval { PVE::QemuServer::vm_mon_cmd($vmid, "savevm-end")  };
+	    warn $@ if $@;
+	} elsif ($hook eq "after-freeze") {
+	    # savevm-end is async, we need to wait
+	    for (;;) {
+		my $stat = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "query-savevm");
+		if (!$stat->{bytes}) {
+		    last;
+		} else {
+		    print "savevm not yet finished\n";
+		    sleep(1);
+		    next;
+		}
+	    }
+	}
+    }
+}
+
+sub __snapshot_create_vol_snapshot {
+    my ($class, $vmid, $ds, $drive, $snapname) = @_;
+
+    return if PVE::QemuServer::drive_is_cdrom($drive);
+
+    my $volid = $drive->{file};
+    my $device = "drive-$ds";
+    my $storecfg = PVE::Storage::config();
+
+    PVE::QemuServer::qemu_volume_snapshot($vmid, $device, $storecfg, $volid, $snapname);
+}
+
+sub __snapshot_delete_remove_drive {
+    my ($class, $snap, $remove_drive) = @_;
+
+    if ($remove_drive eq 'vmstate') {
+	delete $snap->{$remove_drive};
+    } else {
+	my $drive = PVE::QemuServer::parse_drive($remove_drive, $snap->{$remove_drive});
+	return if PVE::QemuServer::drive_is_cdrom($drive);
+
+	my $volid = $drive->{file};
+	delete $snap->{$remove_drive};
+	$class->add_unused_volume($snap, $volid);
+    }
+}
+
+sub __snapshot_delete_vmstate_file {
+    my ($class, $snap, $force) = @_;
+
+    my $storecfg = PVE::Storage::config();
+
+    eval {  PVE::Storage::vdisk_free($storecfg, $snap->{vmstate}); };
+    if (my $err = $@) {
+	die $err if !$force;
+	warn $err;
+    }
+}
+
+sub __snapshot_delete_vol_snapshot {
+    my ($class, $vmid, $ds, $drive, $snapname, $unused) = @_;
+
+    return if PVE::QemuServer::drive_is_cdrom($drive);
+    my $storecfg = PVE::Storage::config();
+    my $volid = $drive->{file};
+    my $device = "drive-$ds";
+
+    PVE::QemuServer::qemu_volume_snapshot_delete($vmid, $device, $storecfg, $volid, $snapname);
+
+    push @$unused, $volid;
+}
+
+sub __snapshot_rollback_vol_possible {
+    my ($class, $drive, $snapname) = @_;
+
+    return if PVE::QemuServer::drive_is_cdrom($drive);
+
+    my $storecfg = PVE::Storage::config();
+    my $volid = $drive->{file};
+
+    PVE::Storage::volume_rollback_is_possible($storecfg, $volid, $snapname);
+}
+
+sub __snapshot_rollback_vol_rollback {
+    my ($class, $drive, $snapname) = @_;
+
+    return if PVE::QemuServer::drive_is_cdrom($drive);
+
+    my $storecfg = PVE::Storage::config();
+    PVE::Storage::volume_snapshot_rollback($storecfg, $drive->{file}, $snapname);
+}
+
+sub __snapshot_rollback_vm_stop {
+    my ($class, $vmid) = @_;
+
+    my $storecfg = PVE::Storage::config();
+    PVE::QemuServer::vm_stop($storecfg, $vmid, undef, undef, 5, undef, undef);
+}
+
+sub __snapshot_rollback_vm_start {
+    my ($class, $vmid, $vmstate, $forcemachine) = @_;
+
+    my $storecfg = PVE::Storage::config();
+    my $statefile = PVE::Storage::path($storecfg, $vmstate);
+    PVE::QemuServer::vm_start($storecfg, $vmid, $statefile, undef, undef, undef, $forcemachine);
+}
+
+sub __snapshot_foreach_volume {
+    my ($class, $conf, $func) = @_;
+
+    PVE::QemuServer::foreach_drive($conf, $func);
+}
 # END implemented abstract methods from PVE::AbstractConfig
 
 1;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index d68872b..04ffc05 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5725,50 +5725,6 @@ sub restore_tar_archive {
     warn $@ if $@;
 };
 
-
-# Internal snapshots
-
-# NOTE: Snapshot create/delete involves several non-atomic
-# action, and can take a long time.
-# So we try to avoid locking the file and use 'lock' variable
-# inside the config file instead.
-
-my $snapshot_copy_config = sub {
-    my ($source, $dest) = @_;
-
-    foreach my $k (keys %$source) {
-	next if $k eq 'snapshots';
-	next if $k eq 'snapstate';
-	next if $k eq 'snaptime';
-	next if $k eq 'vmstate';
-	next if $k eq 'lock';
-	next if $k eq 'digest';
-	next if $k eq 'description';
-	next if $k =~ m/^unused\d+$/;
-
-	$dest->{$k} = $source->{$k};
-    }
-};
-
-my $snapshot_apply_config = sub {
-    my ($conf, $snap) = @_;
-
-    # copy snapshot list
-    my $newconf = {
-	snapshots => $conf->{snapshots},
-    };
-
-    # keep description and list of unused disks
-    foreach my $k (keys %$conf) {
-	next if !($k =~ m/^unused\d+$/ || $k eq 'description');
-	$newconf->{$k} = $conf->{$k};
-    }
-
-    &$snapshot_copy_config($snap, $newconf);
-
-    return $newconf;
-};
-
 sub foreach_writable_storage {
     my ($conf, $func) = @_;
 
@@ -5792,245 +5748,6 @@ sub foreach_writable_storage {
     }
 }
 
-my $alloc_vmstate_volid = sub {
-    my ($storecfg, $vmid, $conf, $snapname) = @_;
-
-    # Note: we try to be smart when selecting a $target storage
-
-    my $target;
-
-    # search shared storage first
-    foreach_writable_storage($conf, sub {
-	my ($sid) = @_;
-	my $scfg = PVE::Storage::storage_config($storecfg, $sid);
-	return if !$scfg->{shared};
-
-	$target = $sid if !$target || $scfg->{path}; # prefer file based storage
-    });
-
-    if (!$target) {
-	# now search local storage
-	foreach_writable_storage($conf, sub {
-	    my ($sid) = @_;
-	    my $scfg = PVE::Storage::storage_config($storecfg, $sid);
-	    return if $scfg->{shared};
-
-	    $target = $sid if !$target || $scfg->{path}; # prefer file based storage;
-	});
-    }
-
-    $target = 'local' if !$target;
-
-    my $driver_state_size = 500; # assume 32MB is enough to safe all driver state;
-    # we abort live save after $conf->{memory}, so we need at max twice that space
-    my $size = $conf->{memory}*2 + $driver_state_size;
-
-    my $name = "vm-$vmid-state-$snapname";
-    my $scfg = PVE::Storage::storage_config($storecfg, $target);
-    $name .= ".raw" if $scfg->{path}; # add filename extension for file base storage
-    my $volid = PVE::Storage::vdisk_alloc($storecfg, $target, $vmid, 'raw', $name, $size*1024);
-
-    return $volid;
-};
-
-sub snapshot_save_vmstate {
-    my ($vmid, $conf, $snapname, $storecfg) = @_;
-
-    my $snap = $conf->{snapshots}->{$snapname};
-
-    $snap->{vmstate} = &$alloc_vmstate_volid($storecfg, $vmid, $conf, $snapname);
-    # always overwrite machine if we save vmstate. This makes sure we
-    # can restore it later using correct machine type
-    $snap->{machine} = get_current_qemu_machine($vmid);
-}
-
-sub snapshot_prepare {
-    my ($vmid, $snapname, $save_vmstate, $comment) = @_;
-
-    my $snap;
-
-    my $updatefn =  sub {
-
-	my $conf = PVE::QemuConfig->load_config($vmid);
-
-	die "you can't take a snapshot if it's a template\n"
-	    if PVE::QemuConfig->is_template($conf);
-
-	PVE::QemuConfig->check_lock($conf);
-
-	$conf->{lock} = 'snapshot';
-
-	die "snapshot name '$snapname' already used\n"
-	    if defined($conf->{snapshots}->{$snapname});
-
-	my $storecfg = PVE::Storage::config();
-	die "snapshot feature is not available\n"
-	    if !has_feature('snapshot', $conf, $storecfg, undef, undef, $snapname eq 'vzdump');
-
-	$snap = $conf->{snapshots}->{$snapname} = {};
-
-	if ($save_vmstate && check_running($vmid)) {
-	    snapshot_save_vmstate($vmid, $conf, $snapname, $storecfg);
-	}
-
-	&$snapshot_copy_config($conf, $snap);
-
-	$snap->{snapstate} = "prepare";
-	$snap->{snaptime} = time();
-	$snap->{description} = $comment if $comment;
-
-	PVE::QemuConfig->write_config($vmid, $conf);
-    };
-
-    PVE::QemuConfig->lock_config($vmid, $updatefn);
-
-    return $snap;
-}
-
-sub snapshot_commit {
-    my ($vmid, $snapname) = @_;
-
-    my $updatefn = sub {
-
-	my $conf = PVE::QemuConfig->load_config($vmid);
-
-	die "missing snapshot lock\n"
-	    if !($conf->{lock} && $conf->{lock} eq 'snapshot');
-
-	my $snap = $conf->{snapshots}->{$snapname};
-
-	die "snapshot '$snapname' does not exist\n" if !defined($snap);
-
-	die "wrong snapshot state\n"
-	    if !($snap->{snapstate} && $snap->{snapstate} eq "prepare");
-
-	delete $snap->{snapstate};
-	delete $conf->{lock};
-
-	$conf->{parent} = $snapname;
-
-	PVE::QemuConfig->write_config($vmid, $conf);
-    };
-
-    PVE::QemuConfig->lock_config($vmid, $updatefn);
-}
-
-sub snapshot_rollback {
-    my ($vmid, $snapname) = @_;
-
-    my $prepare = 1;
-
-    my $storecfg = PVE::Storage::config();
-
-    my $conf = PVE::QemuConfig->load_config($vmid);
-
-    my $get_snapshot_config = sub {
-
-	die "you can't rollback if vm is a template\n" if PVE::QemuConfig->is_template($conf);
-
-	my $res = $conf->{snapshots}->{$snapname};
-
-	die "snapshot '$snapname' does not exist\n" if !defined($res);
-
-	return $res;
-    };
-
-    my $snap = &$get_snapshot_config();
-
-    foreach_drive($snap, sub {
-	my ($ds, $drive) = @_;
-
-	return if drive_is_cdrom($drive);
-
-	my $volid = $drive->{file};
-
-	PVE::Storage::volume_rollback_is_possible($storecfg, $volid, $snapname);
-    });
-
-    my $updatefn = sub {
-
-	$conf = PVE::QemuConfig->load_config($vmid);
-
-	$snap = &$get_snapshot_config();
-
-	die "unable to rollback to incomplete snapshot (snapstate = $snap->{snapstate})\n"
-	    if $snap->{snapstate};
-
-	if ($prepare) {
-	    PVE::QemuConfig->check_lock($conf);
-	    vm_stop($storecfg, $vmid, undef, undef, 5, undef, undef);
-	}
-
-	die "unable to rollback vm $vmid: vm is running\n"
-	    if check_running($vmid);
-
-	if ($prepare) {
-	    $conf->{lock} = 'rollback';
-	} else {
-	    die "got wrong lock\n" if !($conf->{lock} && $conf->{lock} eq 'rollback');
-	    delete $conf->{lock};
-	}
-
-	my $forcemachine;
-
-	if (!$prepare) {
-	    my $has_machine_config = defined($conf->{machine});
-
-	    # copy snapshot config to current config
-	    $conf = &$snapshot_apply_config($conf, $snap);
-	    $conf->{parent} = $snapname;
-
-	    # 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).
-	    $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} && !$has_machine_config;
-	}
-
-	PVE::QemuConfig->write_config($vmid, $conf);
-
-	if (!$prepare && $snap->{vmstate}) {
-	    my $statefile = PVE::Storage::path($storecfg, $snap->{vmstate});
-	    vm_start($storecfg, $vmid, $statefile, undef, undef, undef, $forcemachine);
-	}
-    };
-
-    PVE::QemuConfig->lock_config($vmid, $updatefn);
-
-    foreach_drive($snap, sub {
-	my ($ds, $drive) = @_;
-
-	return if drive_is_cdrom($drive);
-
-	my $volid = $drive->{file};
-	my $device = "drive-$ds";
-
-	PVE::Storage::volume_snapshot_rollback($storecfg, $volid, $snapname);
-    });
-
-    $prepare = 0;
-    PVE::QemuConfig->lock_config($vmid, $updatefn);
-}
-
-my $savevm_wait = sub {
-    my ($vmid) = @_;
-
-    for(;;) {
-	my $stat = vm_mon_cmd_nocheck($vmid, "query-savevm");
-	if (!$stat->{status}) {
-	    die "savevm not active\n";
-	} elsif ($stat->{status} eq 'active') {
-	    sleep(1);
-	    next;
-	} elsif ($stat->{status} eq 'completed') {
-	    last;
-	} else {
-	    die "query-savevm returned status '$stat->{status}'\n";
-	}
-    }
-};
-
 sub do_snapshots_with_qemu {
     my ($storecfg, $volid) = @_;
 
@@ -6059,224 +5776,6 @@ sub qga_check_running {
     return 1;
 }
 
-sub check_freeze_needed {
-    my ($vmid, $config, $save_vmstate) = @_;
-
-    my $running = check_running($vmid);
-    if ($save_vmstate) {
-	return ($running, $running && $config->{agent} && qga_check_running($vmid));
-    } else {
-	return ($running, 0);
-    }
-}
-
-sub snapshot_create {
-    my ($vmid, $snapname, $save_vmstate, $comment) = @_;
-
-    my $snap = snapshot_prepare($vmid, $snapname, $save_vmstate, $comment);
-
-    $save_vmstate = 0 if !$snap->{vmstate}; # vm is not running
-
-    my $config = PVE::QemuConfig->load_config($vmid);
-
-    my ($running, $freezefs) = check_freeze_needed($vmid, $config, $snap->{vmstate});
-
-    my $drivehash = {};
-
-    eval {
-	if ($freezefs) {
-	    eval { vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
-	    warn "guest-fsfreeze-freeze problems - $@" if $@;
-	}
-
-	# create internal snapshots of all drives
-
-	my $storecfg = PVE::Storage::config();
-
-	if ($running) {
-	    if ($snap->{vmstate}) {
-		my $path = PVE::Storage::path($storecfg, $snap->{vmstate});
-		vm_mon_cmd($vmid, "savevm-start", statefile => $path);
-		&$savevm_wait($vmid);
-	    } else {
-		vm_mon_cmd($vmid, "savevm-start");
- 	    }
-	};
-
-	foreach_drive($snap, sub {
-	    my ($ds, $drive) = @_;
-
-	    return if drive_is_cdrom($drive);
-
-	    my $volid = $drive->{file};
-	    my $device = "drive-$ds";
-
-	    qemu_volume_snapshot($vmid, $device, $storecfg, $volid, $snapname);
-	    $drivehash->{$ds} = 1;
-       });
-    };
-    my $err = $@;
-
-    if ($running) {
-	eval { vm_mon_cmd($vmid, "savevm-end")  };
-	warn $@ if $@;
-
-	if ($freezefs) {
-	    eval { vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
-	    warn "guest-fsfreeze-thaw problems - $@" if $@;
-	}
-
-	# savevm-end is async, we need to wait
-	for (;;) {
-	    my $stat = vm_mon_cmd_nocheck($vmid, "query-savevm");
-	    if (!$stat->{bytes}) {
-		last;
-	    } else {
-		print "savevm not yet finished\n";
-		sleep(1);
-		next;
-	    }
-	}
-    }
-
-    if ($err) {
-	warn "snapshot create failed: starting cleanup\n";
-	eval { snapshot_delete($vmid, $snapname, 1, $drivehash); };
-	warn $@ if $@;
-	die $err;
-    }
-
-    snapshot_commit($vmid, $snapname);
-}
-
-# Note: $drivehash is only set when called from snapshot_create.
-sub snapshot_delete {
-    my ($vmid, $snapname, $force, $drivehash) = @_;
-
-    my $prepare = 1;
-
-    my $snap;
-    my $unused = [];
-
-    my $unlink_parent = sub {
-	my ($confref, $new_parent) = @_;
-
-	if ($confref->{parent} && $confref->{parent} eq $snapname) {
-	    if ($new_parent) {
-		$confref->{parent} = $new_parent;
-	    } else {
-		delete $confref->{parent};
-	    }
-	}
-    };
-
-    my $updatefn =  sub {
-	my ($remove_drive) = @_;
-
-	my $conf = PVE::QemuConfig->load_config($vmid);
-
-	if (!$drivehash) {
-	    PVE::QemuConfig->check_lock($conf);
-	    die "you can't delete a snapshot if vm is a template\n"
-		if PVE::QemuConfig->is_template($conf);
-	}
-
-	$snap = $conf->{snapshots}->{$snapname};
-
-	die "snapshot '$snapname' does not exist\n" if !defined($snap);
-
-	# remove parent refs
-	if (!$prepare) {
-	    &$unlink_parent($conf, $snap->{parent});
-	    foreach my $sn (keys %{$conf->{snapshots}}) {
-		next if $sn eq $snapname;
-		&$unlink_parent($conf->{snapshots}->{$sn}, $snap->{parent});
-	    }
-	}
-
-	if ($remove_drive) {
-	    if ($remove_drive eq 'vmstate') {
-		delete $snap->{$remove_drive};
-	    } else {
-		my $drive = parse_drive($remove_drive, $snap->{$remove_drive});
-		my $volid = $drive->{file};
-		delete $snap->{$remove_drive};
-		add_unused_volume($conf, $volid);
-	    }
-	}
-
-	if ($prepare) {
-	    $snap->{snapstate} = 'delete';
-	} else {
-	    delete $conf->{snapshots}->{$snapname};
-	    delete $conf->{lock} if $drivehash;
-	    foreach my $volid (@$unused) {
-		add_unused_volume($conf, $volid);
-	    }
-	}
-
-	PVE::QemuConfig->write_config($vmid, $conf);
-    };
-
-    PVE::QemuConfig->lock_config($vmid, $updatefn);
-
-    # now remove vmstate file
-
-    my $storecfg = PVE::Storage::config();
-
-    if ($snap->{vmstate}) {
-	eval {  PVE::Storage::vdisk_free($storecfg, $snap->{vmstate}); };
-	if (my $err = $@) {
-	    die $err if !$force;
-	    warn $err;
-	}
-	# save changes (remove vmstate from snapshot)
-	PVE::QemuConfig->lock_config($vmid, $updatefn, 'vmstate') if !$force;
-    };
-
-    # now remove all internal snapshots
-    foreach_drive($snap, sub {
-	my ($ds, $drive) = @_;
-
-	return if drive_is_cdrom($drive);
-
-	my $volid = $drive->{file};
-	my $device = "drive-$ds";
-
-	if (!$drivehash || $drivehash->{$ds}) {
-	    eval { qemu_volume_snapshot_delete($vmid, $device, $storecfg, $volid, $snapname); };
-	    if (my $err = $@) {
-		die $err if !$force;
-		warn $err;
-	    }
-	}
-
-	# save changes (remove drive fron snapshot)
-	PVE::QemuConfig->lock_config($vmid, $updatefn, $ds) if !$force;
-	push @$unused, $volid;
-    });
-
-    # now cleanup config
-    $prepare = 0;
-    PVE::QemuConfig->lock_config($vmid, $updatefn);
-}
-
-sub has_feature {
-    my ($feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
-
-    my $err;
-    foreach_drive($conf, sub {
-	my ($ds, $drive) = @_;
-
-	return if drive_is_cdrom($drive);
-	return if $backup_only && !$drive->{backup};
-	my $volid = $drive->{file};
-	$err = 1 if !PVE::Storage::volume_has_feature($storecfg, $feature, $volid, $snapname, $running);
-    });
-
-    return $err ? 0 : 1;
-}
-
 sub template_create {
     my ($vmid, $conf, $disk) = @_;
 
diff --git a/test/snapshot-expected/delete/qemu-server/202.conf b/test/snapshot-expected/delete/qemu-server/202.conf
index 810c4f4..1ceb910 100644
--- a/test/snapshot-expected/delete/qemu-server/202.conf
+++ b/test/snapshot-expected/delete/qemu-server/202.conf
@@ -29,5 +29,6 @@ smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
 snapstate: delete
 snaptime: 1234567890
 sockets: 1
+unused0: local:snapshotable-disk-1
 vga: qxl
 virtio0: local:unsnapshotable-disk-2,discard=on,size=32G
diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm
index 70355fd..43b1515 100644
--- a/test/snapshot-test.pm
+++ b/test/snapshot-test.pm
@@ -152,7 +152,7 @@ sub testcase_prepare {
 	plan tests => 2;
 	$@ = undef;
 	eval {
-	    PVE::QemuServer::snapshot_prepare($vmid, $snapname, $save_vmstate, $comment);
+	    PVE::QemuConfig->__snapshot_prepare($vmid, $snapname, $save_vmstate, $comment);
 	};
 	is($@, $exp_err, "\$@ correct");
 	ok(test_file("snapshot-expected/prepare/qemu-server/$vmid.conf", "snapshot-working/prepare/qemu-server/$vmid.conf"), "config file correct");
@@ -165,7 +165,7 @@ sub testcase_commit {
 	plan tests => 2;
 	$@ = undef;
 	eval {
-	    PVE::QemuServer::snapshot_commit($vmid, $snapname);
+	    PVE::QemuConfig->__snapshot_commit($vmid, $snapname);
 	};
 	is($@, $exp_err, "\$@ correct");
 	ok(test_file("snapshot-expected/commit/qemu-server/$vmid.conf", "snapshot-working/commit/qemu-server/$vmid.conf"), "config file correct");
@@ -182,7 +182,7 @@ sub testcase_create {
 	$exp_vol_snap_delete = {} if !defined($exp_vol_snap_delete);
 	$@ = undef;
 	eval {
-	    PVE::QemuServer::snapshot_create($vmid, $snapname, $save_vmstate, $comment);
+	    PVE::QemuConfig->snapshot_create($vmid, $snapname, $save_vmstate, $comment);
 	};
 	is($@, $exp_err, "\$@ correct");
 	is_deeply($vol_snapshot, $exp_vol_snap, "created correct volume snapshots");
@@ -199,7 +199,7 @@ sub testcase_delete {
 	$exp_vol_snap_delete = {} if !defined($exp_vol_snap_delete);
 	$@ = undef;
 	eval {
-	    PVE::QemuServer::snapshot_delete($vmid, $snapname, $force);
+	    PVE::QemuConfig->snapshot_delete($vmid, $snapname, $force);
 	};
 	is($@, $exp_err, "\$@ correct");
 	is_deeply($vol_snapshot_delete, $exp_vol_snap_delete, "deleted correct volume snapshots");
@@ -216,7 +216,7 @@ sub testcase_rollback {
 	$exp_vol_snap_rollback = {} if !defined($exp_vol_snap_rollback);
 	$@ = undef;
 	eval {
-	    PVE::QemuServer::snapshot_rollback($vmid, $snapname);
+	    PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
 	};
 	is($@, $exp_err, "\$@ correct");
 	is_deeply($vol_snapshot_rollback, $exp_vol_snap_rollback, "rolled back to correct volume snapshots");
@@ -263,20 +263,14 @@ sub write_config {
 
     PVE::Tools::file_set_contents($filename, $raw);
 }
-# END mocked PVE::QemuConfig methods
 
-# BEGIN redefine PVE::QemuServer methods
 sub has_feature {
-    my ($feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
+    my ($class, $feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
     return $snapshot_possible;
 }
 
-sub check_running {
-    return $running;
-}
-
-sub snapshot_save_vmstate {
-    my ($vmid, $conf, $snapname, $storecfg) = @_;
+sub __snapshot_save_vmstate {
+    my ($class, $vmid, $conf, $snapname, $storecfg) = @_;
     die "save_vmstate failed\n"
 	if !$save_vmstate_works;
 
@@ -284,6 +278,14 @@ sub snapshot_save_vmstate {
     $snap->{vmstate} = "somestorage:state-volume";
     $snap->{machine} = "somemachine";
 }
+# END mocked PVE::QemuConfig methods
+
+# BEGIN redefine PVE::QemuServer methods
+
+sub check_running {
+    return $running;
+}
+
 
 sub do_snapshots_with_qemu {
     return 0;
@@ -353,6 +355,8 @@ $qemu_config_module->mock('config_file_lock', \&config_file_lock);
 $qemu_config_module->mock('cfs_config_path', \&cfs_config_path);
 $qemu_config_module->mock('load_config', \&load_config);
 $qemu_config_module->mock('write_config', \&write_config);
+$qemu_config_module->mock('has_feature', \&has_feature);
+$qemu_config_module->mock('__snapshot_save_vmstate', \&__snapshot_save_vmstate);
 
 $running = 1;
 $freeze_possible = 1;
@@ -489,14 +493,14 @@ testcase_create("106", "test", 1, "test comment", "", { "local:snapshotable-disk
 $freeze_possible = 1;
 
 printf("Expected error for snapshot_create when volume snapshot is not possible\n");
-testcase_create("201", "test", 0, "test comment", "volume snapshot disabled\n");
+testcase_create("201", "test", 0, "test comment", "volume snapshot disabled\n\n");
 
 printf("Expected error for snapshot_create when volume snapshot is not possible for one drive\n");
-testcase_create("202", "test", 0, "test comment", "volume snapshot disabled\n", { "local:snapshotable-disk-1" => "test" }, { "local:snapshotable-disk-1" => "test" });
+testcase_create("202", "test", 0, "test comment", "volume snapshot disabled\n\n", { "local:snapshotable-disk-1" => "test" }, { "local:snapshotable-disk-1" => "test" });
 
 $vm_mon->{savevm_start} = 0;
 printf("Expected error for snapshot_create when Qemu mon command 'savevm-start' fails\n");
-testcase_create("203", "test", 0, "test comment", "savevm-start disabled\n");
+testcase_create("203", "test", 0, "test comment", "savevm-start disabled\n\n");
 $vm_mon->{savevm_start} = 1;
 
 
-- 
2.1.4





More information about the pve-devel mailing list