[pve-devel] [PATCH-SERIES guest-common/qemu-server/container] activate storages for snapshot operations

[pve-devel] [PATCH-SERIES guest-common/qemu-server/container] activate storages for snapshot operations

[Fabian Ebner]

to make it work when the storage is just not active yet, and have
early errors when the storage cannot be activated. Also prohibits
snapshot operations when an involved storage is disabled.


Both qemu-server and pve-container build-depend upon pve-guest-common
for the added tests.


pve-guest-common:

Fabian Ebner (2):
  config: remove unused variable
  config: activate affected storages for snapshot operations

 src/PVE/AbstractConfig.pm | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)


qemu-server:

Fabian Ebner (1):
  snapshot: implement __snapshot_activate_storages

 PVE/QemuConfig.pm                             | 19 +++++++++++
 .../create/qemu-server/303.conf               | 13 +++++++
 .../delete/qemu-server/204.conf               | 33 ++++++++++++++++++
 .../rollback/qemu-server/303.conf             | 34 +++++++++++++++++++
 .../create/qemu-server/303.conf               | 13 +++++++
 .../delete/qemu-server/204.conf               | 33 ++++++++++++++++++
 .../rollback/qemu-server/303.conf             | 34 +++++++++++++++++++
 test/snapshot-test.pm                         | 32 +++++++++++++++++
 8 files changed, 211 insertions(+)
 create mode 100644 test/snapshot-expected/create/qemu-server/303.conf
 create mode 100644 test/snapshot-expected/delete/qemu-server/204.conf
 create mode 100644 test/snapshot-expected/rollback/qemu-server/303.conf
 create mode 100644 test/snapshot-input/create/qemu-server/303.conf
 create mode 100644 test/snapshot-input/delete/qemu-server/204.conf
 create mode 100644 test/snapshot-input/rollback/qemu-server/303.conf


pve-container:

Fabian Ebner (3):
  config: snapshot_delete_remove_drive: check for parsed value
  config: parse_volume: don't die when noerr is set
  snapshot: implement __snapshot_activate_storages

 src/PVE/LXC/Config.pm                         | 25 +++++++++++++--
 .../snapshot-expected/create/lxc/204.conf     | 10 ++++++
 .../snapshot-expected/delete/lxc/204.conf     | 25 +++++++++++++++
 .../snapshot-expected/rollback/lxc/209.conf   | 29 +++++++++++++++++
 src/test/snapshot-input/create/lxc/204.conf   | 10 ++++++
 src/test/snapshot-input/delete/lxc/204.conf   | 25 +++++++++++++++
 src/test/snapshot-input/rollback/lxc/209.conf | 29 +++++++++++++++++
 src/test/snapshot-test.pm                     | 32 +++++++++++++++++++
 8 files changed, 183 insertions(+), 2 deletions(-)
 create mode 100644 src/test/snapshot-expected/create/lxc/204.conf
 create mode 100644 src/test/snapshot-expected/delete/lxc/204.conf
 create mode 100644 src/test/snapshot-expected/rollback/lxc/209.conf
 create mode 100644 src/test/snapshot-input/create/lxc/204.conf
 create mode 100644 src/test/snapshot-input/delete/lxc/204.conf
 create mode 100644 src/test/snapshot-input/rollback/lxc/209.conf

-- 
2.30.2

[pve-devel] [PATCH guest-common 1/2] config: remove unused variable

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/AbstractConfig.pm | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index 3348d8a..b3f70f2 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -829,8 +829,6 @@ sub snapshot_create {
 sub snapshot_delete {
     my ($class, $vmid, $snapname, $force, $drivehash) = @_;
 
-    my $prepare = 1;
-
     my $unused = [];
 
     my $conf = $class->load_config($vmid);
-- 
2.30.2

[pve-devel] [PATCH guest-common 2/2] config: activate affected storages for snapshot operations

[Fabian Ebner]

For snapshot creation, the storage for the vmstate file is activated
via vdisk_alloc when the state file is created.

Do not activate the volumes themselves, as that has unnecessary side
effects (e.g. waiting for zvol device link for ZFS, mapping the volume
for RBD). If a storage can only do snapshot operations on a volume
that has been activated, it needs to activate the volume itself.

The actual implementation will be in the plugins, to be able to skip
CD ROM drives and bind-mounts, etc. when iterating over the volumes.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/AbstractConfig.pm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index b3f70f2..d4f3d44 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -776,6 +776,13 @@ sub __snapshot_commit {
     $class->lock_config($vmid, $updatefn);
 }
 
+# Activates the storages affected by the snapshot operations.
+sub __snapshot_activate_storages {
+    my ($class, $conf, $include_vmstate) = @_;
+
+    return; # FIXME PVE 8.x change to die 'implement me' and bump Breaks for older plugins
+}
+
 # Creates a snapshot for the VM/CT.
 sub snapshot_create {
     my ($class, $vmid, $snapname, $save_vmstate, $comment) = @_;
@@ -791,6 +798,8 @@ sub snapshot_create {
     my $drivehash = {};
 
     eval {
+	$class->__snapshot_activate_storages($conf, 0);
+
 	if ($freezefs) {
 	    $class->__snapshot_freeze($vmid, 0);
 	}
@@ -836,6 +845,8 @@ sub snapshot_delete {
 
     die "snapshot '$snapname' does not exist\n" if !defined($snap);
 
+    $class->__snapshot_activate_storages($snap, 1) if !$drivehash;
+
     $class->set_lock($vmid, 'snapshot-delete')
 	if (!$drivehash); # doesn't already have a 'snapshot' lock
 
@@ -970,6 +981,8 @@ sub snapshot_rollback {
 	$snap = $get_snapshot_config->($conf);
 
 	if ($prepare) {
+	    $class->__snapshot_activate_storages($snap, 1);
+
 	    my $repl_conf = PVE::ReplicationConfig->new();
 	    if ($repl_conf->check_for_existing_jobs($vmid, 1)) {
 		# remove all replication snapshots
-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/1] snapshot: implement __snapshot_activate_storages

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/QemuConfig.pm                             | 19 +++++++++++
 .../create/qemu-server/303.conf               | 13 +++++++
 .../delete/qemu-server/204.conf               | 33 ++++++++++++++++++
 .../rollback/qemu-server/303.conf             | 34 +++++++++++++++++++
 .../create/qemu-server/303.conf               | 13 +++++++
 .../delete/qemu-server/204.conf               | 33 ++++++++++++++++++
 .../rollback/qemu-server/303.conf             | 34 +++++++++++++++++++
 test/snapshot-test.pm                         | 32 +++++++++++++++++
 8 files changed, 211 insertions(+)
 create mode 100644 test/snapshot-expected/create/qemu-server/303.conf
 create mode 100644 test/snapshot-expected/delete/qemu-server/204.conf
 create mode 100644 test/snapshot-expected/rollback/qemu-server/303.conf
 create mode 100644 test/snapshot-input/create/qemu-server/303.conf
 create mode 100644 test/snapshot-input/delete/qemu-server/204.conf
 create mode 100644 test/snapshot-input/rollback/qemu-server/303.conf

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 7ee8876..4e660c5 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -241,6 +241,25 @@ sub __snapshot_save_vmstate {
     return $statefile;
 }
 
+sub __snapshot_activate_storages {
+    my ($class, $conf, $include_vmstate) = @_;
+
+    my $storecfg = PVE::Storage::config();
+    my $opts = $include_vmstate ? { 'extra_keys' => ['vmstate'] } : {};
+    my $storage_hash = {};
+
+    $class->foreach_volume_full($conf, $opts, sub {
+	my ($key, $drive) = @_;
+
+	return if PVE::QemuServer::drive_is_cdrom($drive);
+
+	my ($storeid) = PVE::Storage::parse_volume_id($drive->{file});
+	$storage_hash->{$storeid} = 1;
+    });
+
+    PVE::Storage::activate_storage_list($storecfg, [ sort keys $storage_hash->%* ]);
+}
+
 sub __snapshot_check_running {
     my ($class, $vmid) = @_;
     return PVE::QemuServer::Helpers::vm_running_locally($vmid);
diff --git a/test/snapshot-expected/create/qemu-server/303.conf b/test/snapshot-expected/create/qemu-server/303.conf
new file mode 100644
index 0000000..2731bd1
--- /dev/null
+++ b/test/snapshot-expected/create/qemu-server/303.conf
@@ -0,0 +1,13 @@
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+machine: q35
+memory: 8192
+name: win
+net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1
+numa: 0
+ostype: win7
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+sockets: 1
+vga: qxl
diff --git a/test/snapshot-expected/delete/qemu-server/204.conf b/test/snapshot-expected/delete/qemu-server/204.conf
new file mode 100644
index 0000000..c521154
--- /dev/null
+++ b/test/snapshot-expected/delete/qemu-server/204.conf
@@ -0,0 +1,33 @@
+agent: 1
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+memory: 8192
+name: win
+net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1
+numa: 0
+ostype: win7
+parent: test
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+sockets: 1
+vga: qxl
+
+[test]
+#test comment
+agent: 1
+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
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+snaptime: 1234567890
+sockets: 1
+vga: qxl
+vmstate: somestorage:state-volume
diff --git a/test/snapshot-expected/rollback/qemu-server/303.conf b/test/snapshot-expected/rollback/qemu-server/303.conf
new file mode 100644
index 0000000..518c954
--- /dev/null
+++ b/test/snapshot-expected/rollback/qemu-server/303.conf
@@ -0,0 +1,34 @@
+agent: 1
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+memory: 8192
+name: win
+net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1
+numa: 0
+ostype: win7
+parent: test
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+sockets: 1
+vga: qxl
+
+[test]
+#test comment
+agent: 1
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+machine: q35
+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
+vga: qxl
+vmstate: somestorage:state-volume
diff --git a/test/snapshot-input/create/qemu-server/303.conf b/test/snapshot-input/create/qemu-server/303.conf
new file mode 100644
index 0000000..2731bd1
--- /dev/null
+++ b/test/snapshot-input/create/qemu-server/303.conf
@@ -0,0 +1,13 @@
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+machine: q35
+memory: 8192
+name: win
+net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1
+numa: 0
+ostype: win7
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+sockets: 1
+vga: qxl
diff --git a/test/snapshot-input/delete/qemu-server/204.conf b/test/snapshot-input/delete/qemu-server/204.conf
new file mode 100644
index 0000000..c521154
--- /dev/null
+++ b/test/snapshot-input/delete/qemu-server/204.conf
@@ -0,0 +1,33 @@
+agent: 1
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+memory: 8192
+name: win
+net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1
+numa: 0
+ostype: win7
+parent: test
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+sockets: 1
+vga: qxl
+
+[test]
+#test comment
+agent: 1
+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
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+snaptime: 1234567890
+sockets: 1
+vga: qxl
+vmstate: somestorage:state-volume
diff --git a/test/snapshot-input/rollback/qemu-server/303.conf b/test/snapshot-input/rollback/qemu-server/303.conf
new file mode 100644
index 0000000..518c954
--- /dev/null
+++ b/test/snapshot-input/rollback/qemu-server/303.conf
@@ -0,0 +1,34 @@
+agent: 1
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+memory: 8192
+name: win
+net0: e1000=12:34:56:78:90:12,bridge=somebr0,firewall=1
+numa: 0
+ostype: win7
+parent: test
+smbios1: uuid=01234567-890a-bcde-f012-34567890abcd
+sockets: 1
+vga: qxl
+
+[test]
+#test comment
+agent: 1
+bootdisk: ide0
+cores: 4
+ide0: local:snapshotable-disk-1,discard=on,size=32G
+ide2: none,media=cdrom
+machine: q35
+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
+vga: qxl
+vmstate: somestorage:state-volume
diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm
index cc04f01..3f1ac7c 100644
--- a/test/snapshot-test.pm
+++ b/test/snapshot-test.pm
@@ -15,6 +15,7 @@ use PVE::ReplicationConfig;
 use Test::MockModule;
 use Test::More;
 
+my $activate_storage_possible = 1;
 my $nodename;
 my $snapshot_possible;
 my $vol_snapshot_possible = {};
@@ -105,6 +106,15 @@ sub mocked_volume_rollback_is_possible {
     die "volume_rollback_is_possible failed\n";
 }
 
+sub mocked_activate_storage {
+    my ($storecfg, $storeid) = @_;
+    die "Storage config not mocked! aborting\n"
+	if defined($storecfg);
+    die "storage activation failed\n"
+	if !$activate_storage_possible;
+    return;
+}
+
 sub mocked_activate_volumes {
     my ($storecfg, $volumes) = @_;
     die "Storage config not mocked! aborting\n"
@@ -410,6 +420,7 @@ $repl_config_module->mock('check_for_existing_jobs' => sub { return });
 my $storage_module = Test::MockModule->new('PVE::Storage');
 $storage_module->mock('config', sub { return; });
 $storage_module->mock('path', sub { return "/some/store/statefile/path"; });
+$storage_module->mock('activate_storage', \&mocked_activate_storage);
 $storage_module->mock('activate_volumes', \&mocked_activate_volumes);
 $storage_module->mock('deactivate_volumes', \&mocked_deactivate_volumes);
 $storage_module->mock('vdisk_free', \&mocked_vdisk_free);
@@ -555,6 +566,13 @@ $vm_mon->{savevm_start} = 1;
 printf("Successful snapshot_create with no existing snapshots but set machine type\n");
 testcase_create("301", "test", 1, "test comment", "", { "local:snapshotable-disk-1" => "test" });
 
+$activate_storage_possible = 0;
+
+printf("Expected error for snapshot_create when storage activation is not possible\n");
+testcase_create("303", "test", 1, "test comment", "storage activation failed\n\n");
+
+$activate_storage_possible = 1;
+
 $nodename = "delete";
 printf("\n");
 printf("Running delete tests\n");
@@ -587,6 +605,13 @@ testcase_delete("202", "test", 0, "volume snapshot delete disabled\n", { "local:
 printf("Expected error for snapshot_delete with locked config\n");
 testcase_delete("203", "test", 0, "VM is locked (backup)\n");
 
+$activate_storage_possible = 0;
+
+printf("Expected error for snapshot_delete when storage activation is not possible\n");
+testcase_delete("204", "test", 0, "storage activation failed\n");
+
+$activate_storage_possible = 1;
+
 $nodename = "rollback";
 printf("\n");
 printf("Running rollback tests\n");
@@ -643,6 +668,13 @@ testcase_rollback("301", "test", "", { "local:snapshotable-disk-1" => "test" });
 printf("Successful snapshot_rollback with saved vmstate and machine config and runningmachine \n");
 testcase_rollback("302", "test", "", { "local:snapshotable-disk-1" => "test" });
 
+$activate_storage_possible = 0;
+
+printf("Expected error for snapshot_rollback when storage activation is not possible\n");
+testcase_rollback("303", "test", "storage activation failed\n");
+
+$activate_storage_possible = 1;
+
 done_testing();
 
 1;
-- 
2.30.2

[pve-devel] [PATCH container 1/3] config: snapshot_delete_remove_drive: check for parsed value

[Fabian Ebner]

to avoid a potential warning. parse_volume is called with noerr=1, so this might
be undef instead of a hash.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC/Config.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 8557e4c..f832f90 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -192,7 +192,7 @@ sub __snapshot_delete_remove_drive {
 	delete $snap->{$remove_drive};
 
 	$class->add_unused_volume($snap, $mountpoint->{volume})
-	    if ($mountpoint->{type} eq 'volume');
+	    if $mountpoint && ($mountpoint->{type} eq 'volume');
     }
 }
 
-- 
2.30.2

[pve-devel] [PATCH container 2/3] config: parse_volume: don't die when noerr is set

[Fabian Ebner]

AFAICT, the only existing callers using noerr=1 are in
__snapshot_delete_remove_drive, and in AbstractConfig's
foreach_volume_full, and they both should be fine with the change.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Required for the next patch.

 src/PVE/LXC/Config.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index f832f90..7a1f72d 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1186,7 +1186,9 @@ sub parse_volume {
 	return $parse_ct_mountpoint_full->($class, $unused_desc, $volume_string, $noerr);
     }
 
-    die "parse_volume - unknown type: $key\n";
+    die "parse_volume - unknown type: $key\n" if !$noerr;
+
+    return;
 }
 
 sub print_volume {
-- 
2.30.2

[pve-devel] [PATCH container 3/3] snapshot: implement __snapshot_activate_storages

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Requires the previous patch, as otherwise, foreach_volume_full will
die when calling parse_volume with 'vmstate'.

 src/PVE/LXC/Config.pm                         | 19 +++++++++++
 .../snapshot-expected/create/lxc/204.conf     | 10 ++++++
 .../snapshot-expected/delete/lxc/204.conf     | 25 +++++++++++++++
 .../snapshot-expected/rollback/lxc/209.conf   | 29 +++++++++++++++++
 src/test/snapshot-input/create/lxc/204.conf   | 10 ++++++
 src/test/snapshot-input/delete/lxc/204.conf   | 25 +++++++++++++++
 src/test/snapshot-input/rollback/lxc/209.conf | 29 +++++++++++++++++
 src/test/snapshot-test.pm                     | 32 +++++++++++++++++++
 8 files changed, 179 insertions(+)
 create mode 100644 src/test/snapshot-expected/create/lxc/204.conf
 create mode 100644 src/test/snapshot-expected/delete/lxc/204.conf
 create mode 100644 src/test/snapshot-expected/rollback/lxc/209.conf
 create mode 100644 src/test/snapshot-input/create/lxc/204.conf
 create mode 100644 src/test/snapshot-input/delete/lxc/204.conf
 create mode 100644 src/test/snapshot-input/rollback/lxc/209.conf

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 7a1f72d..fd29c38 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -101,6 +101,25 @@ sub __snapshot_save_vmstate {
     die "implement me - snapshot_save_vmstate\n";
 }
 
+sub __snapshot_activate_storages {
+    my ($class, $conf, $include_vmstate) = @_;
+
+    my $storecfg = PVE::Storage::config();
+    my $opts = $include_vmstate ? { 'extra_keys' => ['vmstate'] } : {};
+    my $storage_hash = {};
+
+    $class->foreach_volume_full($conf, $opts, sub {
+	my ($vs, $mountpoint) = @_;
+
+	return if $mountpoint->{type} ne 'volume';
+
+	my ($storeid) = PVE::Storage::parse_volume_id($mountpoint->{volume});
+	$storage_hash->{$storeid} = 1;
+    });
+
+    PVE::Storage::activate_storage_list($storecfg, [ sort keys $storage_hash->%* ]);
+}
+
 sub __snapshot_check_running {
     my ($class, $vmid) = @_;
     return PVE::LXC::check_running($vmid);
diff --git a/src/test/snapshot-expected/create/lxc/204.conf b/src/test/snapshot-expected/create/lxc/204.conf
new file mode 100644
index 0000000..4546668
--- /dev/null
+++ b/src/test/snapshot-expected/create/lxc/204.conf
@@ -0,0 +1,10 @@
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:unsnapshotable-disk-1,mp=/invalid/mountpoint
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+rootfs: local:snapshotable-disk-1
+swap: 512
diff --git a/src/test/snapshot-expected/delete/lxc/204.conf b/src/test/snapshot-expected/delete/lxc/204.conf
new file mode 100644
index 0000000..a21c535
--- /dev/null
+++ b/src/test/snapshot-expected/delete/lxc/204.conf
@@ -0,0 +1,25 @@
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:unsnapshotable-disk-1,mp=/invalid/mountpoint
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+parent: test
+rootfs: local:snapshotable-disk-1
+swap: 512
+
+[test]
+#test comment
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:unsnapshotable-disk-1,mp=/invalid/mountpoint
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+rootfs: local:snapshotable-disk-1
+snaptime: 1234567890
+swap: 512
diff --git a/src/test/snapshot-expected/rollback/lxc/209.conf b/src/test/snapshot-expected/rollback/lxc/209.conf
new file mode 100644
index 0000000..c9a23c9
--- /dev/null
+++ b/src/test/snapshot-expected/rollback/lxc/209.conf
@@ -0,0 +1,29 @@
+# should be preserved
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:snapshotable-disk-2,mp=/invalid/mp0
+mp1: local:unsnapshotable-disk-1,mp=/invalid/mp1
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+parent: test
+rootfs: local:snapshotable-disk-1
+swap: 512
+unused0: preserved:some-disk-1
+
+[test]
+# should be thrown away
+arch: amd64
+cpulimit: 2
+cpuunits: 2048
+hostname: test2
+memory: 4096
+mp0: local:snapshotable-disk-2,mp=/invalid/mp0
+mp1: local:snapshotable-disk-4,mp=/invalid/mp1
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+rootfs: local:snapshotable-disk-1
+snaptime: 1234567890
+swap: 1024
diff --git a/src/test/snapshot-input/create/lxc/204.conf b/src/test/snapshot-input/create/lxc/204.conf
new file mode 100644
index 0000000..4546668
--- /dev/null
+++ b/src/test/snapshot-input/create/lxc/204.conf
@@ -0,0 +1,10 @@
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:unsnapshotable-disk-1,mp=/invalid/mountpoint
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+rootfs: local:snapshotable-disk-1
+swap: 512
diff --git a/src/test/snapshot-input/delete/lxc/204.conf b/src/test/snapshot-input/delete/lxc/204.conf
new file mode 100644
index 0000000..a21c535
--- /dev/null
+++ b/src/test/snapshot-input/delete/lxc/204.conf
@@ -0,0 +1,25 @@
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:unsnapshotable-disk-1,mp=/invalid/mountpoint
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+parent: test
+rootfs: local:snapshotable-disk-1
+swap: 512
+
+[test]
+#test comment
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:unsnapshotable-disk-1,mp=/invalid/mountpoint
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+rootfs: local:snapshotable-disk-1
+snaptime: 1234567890
+swap: 512
diff --git a/src/test/snapshot-input/rollback/lxc/209.conf b/src/test/snapshot-input/rollback/lxc/209.conf
new file mode 100644
index 0000000..c9a23c9
--- /dev/null
+++ b/src/test/snapshot-input/rollback/lxc/209.conf
@@ -0,0 +1,29 @@
+# should be preserved
+arch: amd64
+cpulimit: 1
+cpuunits: 1024
+hostname: test
+memory: 2048
+mp0: local:snapshotable-disk-2,mp=/invalid/mp0
+mp1: local:unsnapshotable-disk-1,mp=/invalid/mp1
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+parent: test
+rootfs: local:snapshotable-disk-1
+swap: 512
+unused0: preserved:some-disk-1
+
+[test]
+# should be thrown away
+arch: amd64
+cpulimit: 2
+cpuunits: 2048
+hostname: test2
+memory: 4096
+mp0: local:snapshotable-disk-2,mp=/invalid/mp0
+mp1: local:snapshotable-disk-4,mp=/invalid/mp1
+net0: bridge=vmbr0,hwaddr=12:34:56:78:90:12,ip=dhcp,ip6=dhcp,name=eth0,type=veth
+ostype: redhat
+rootfs: local:snapshotable-disk-1
+snaptime: 1234567890
+swap: 1024
diff --git a/src/test/snapshot-test.pm b/src/test/snapshot-test.pm
index 91a2af9..4fc735b 100644
--- a/src/test/snapshot-test.pm
+++ b/src/test/snapshot-test.pm
@@ -16,6 +16,7 @@ use PVE::ReplicationConfig;
 use Test::MockModule;
 use Test::More;
 
+my $activate_storage_possible = 1;
 my $nodename;
 my $snapshot_possible;
 my $vol_snapshot_possible = {};
@@ -122,6 +123,15 @@ sub mocked_volume_snapshot_needs_fsfreeze {
     return 0;
 }
 
+sub mocked_activate_storage {
+    my ($storecfg, $storeid) = @_;
+    die "Storage config not mocked! aborting\n"
+	if defined($storecfg);
+    die "storage activation failed\n"
+	if !$activate_storage_possible;
+    return;
+}
+
 sub mocked_vm_stop {
     if ($kill_possible) {
 	$running = 0;
@@ -390,6 +400,7 @@ $vol_snapshot_rollback_possible->{"local:snapshotable-disk-4"} = 1;
 printf("\n");
 printf("Setting up Mocking for PVE::Storage\n");
 my $storage_module = new Test::MockModule('PVE::Storage');
+$storage_module->mock('activate_storage', \&mocked_activate_storage);
 $storage_module->mock('config', sub { return undef; });
 $storage_module->mock('volume_snapshot', \&mocked_volume_snapshot);
 $storage_module->mock('volume_snapshot_delete', \&mocked_volume_snapshot_delete);
@@ -429,6 +440,13 @@ $freeze_possible = 1;
 printf("Expected error for snapshot_create when mp volume snapshot is not possible\n");
 testcase_create("203", "test", 0, "test comment", "volume snapshot disabled\n\n", { "local:snapshotable-disk-1" => "test" }, { "local:snapshotable-disk-1" => "test" });
 
+$activate_storage_possible = 0;
+
+printf("Expected error for snapshot_create when storage activation is not possible\n");
+testcase_create("204", "test", 0, "test comment", "storage activation failed\n\n");
+
+$activate_storage_possible = 1;
+
 $nodename = "delete";
 printf("\n");
 printf("Running delete tests\n");
@@ -461,6 +479,13 @@ testcase_delete("203", "test", 0, "volume snapshot delete disabled\n", { "local:
 printf("Expected error for snapshot_delete with locked config\n");
 testcase_delete("202", "test", 0, "CT is locked (backup)\n");
 
+$activate_storage_possible = 0;
+
+printf("Expected error for snapshot_delete when storage activation is not possible\n");
+testcase_delete("204", "test", 0, "storage activation failed\n");
+
+$activate_storage_possible = 1;
+
 $nodename = "rollback";
 printf("\n");
 printf("Running rollback tests\n");
@@ -511,4 +536,11 @@ testcase_rollback("207", "test", "volume_rollback_is_possible failed\n");
 printf("Expected error for snapshot_rollback with mp rollback failure (results in inconsistent state)\n");
 testcase_rollback("208", "test", "volume snapshot rollback disabled\n", { "local:snapshotable-disk-1" => "test", "local:snapshotable-disk-2" => "test" });
 
+$activate_storage_possible = 0;
+
+printf("Expected error for snapshot_rollback when storage activation is not possible\n");
+testcase_rollback("209", "test", "storage activation failed\n");
+
+$activate_storage_possible = 1;
+
 done_testing();
-- 
2.30.2