[pve-devel] [PATCH qemu-server 18/18] fix #7066: api: allow live snapshot (remove) of qcow2 TPM drive with snapshot-as-volume-chain

[Fiona Ebner <f.ebner@proxmox.com>]

Commit "snapshot: support live snapshot (remove) of qcow2 TPM drive on
storage with snapshot-as-volume-chain" prepared for this. It's not a
revert of commit c7d839df ("snapshot: prohibit live snapshot (remove)
of qcow2 TPM drive on storage with snapshot-as-volume-chain"), because
there is a single limitation remaining. That limitation is removing
the top-most snapshot live when the current image is exported via
FUSE, because exporting unshares the 'resize' permission, which would
be required by both 'block-commit' and 'block-stream', for example:

> QEMU storage daemon 100 qsd command 'block-commit' failed - Permission
> conflict on node '#block017': permissions 'resize' are both required
> by node 'drive-tpmstate0' (uses node '#block017' as 'file' child) and
> unshared by commit job 'commit-drive-tpmstate0' (uses node '#block017'
> as 'main node' child).

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 5a627936..859afce9 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -726,11 +726,11 @@ my $check_cpu_model_access = sub {
     }
 };
 
-# TODO switch to doing internal snapshots only for TPM? Need a way to tell the storage. Also needs
-# handling for pre-existing as-volume-chain snapshots then. Or is there a way to make QSD+swtpm
-# compatible with using volume-chain live?
-my sub assert_tpm_snapshot_compat {
-    my ($vmid, $conf, $op, $snap_conf) = @_;
+# The top-most snapshot for a FUSE-exported TPM state cannot be removed live, because exporting
+# unshares the 'resize' permission, which would be required by both 'block-commit' and
+# 'block-stream'.
+my sub assert_tpm_snapshot_delete_possible {
+    my ($vmid, $conf, $snap_conf, $snap_name) = @_;
 
     return if !$conf->{tpmstate0};
     return if !PVE::QemuServer::Helpers::vm_running_locally($vmid);
@@ -739,19 +739,19 @@ my sub assert_tpm_snapshot_compat {
     my $volid = $drive->{file};
     my $storecfg = PVE::Storage::config();
 
-    if ($snap_conf) {
-        return if !$snap_conf->{tpmstate0};
-        my $snap_drive = PVE::QemuServer::Drive::parse_drive('tpmstate0', $snap_conf->{tpmstate0});
-        return if $volid ne $snap_drive->{file};
-    }
+    return if $conf->{parent} ne $snap_name; # allowed if not top-most snapshot
+
+    return if !$snap_conf->{tpmstate0};
+    my $snap_drive = PVE::QemuServer::Drive::parse_drive('tpmstate0', $snap_conf->{tpmstate0});
+    return if $volid ne $snap_drive->{file};
 
     my $format = PVE::QemuServer::Drive::checked_volume_format($storecfg, $volid);
     my ($storeid) = PVE::Storage::parse_volume_id($volid, 1);
     if ($storeid && $format eq 'qcow2') {
         my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
         if ($scfg && $scfg->{'snapshot-as-volume-chain'}) {
-            die "snapshot $op of TPM state '$volid' on storage with 'snapshot-as-volume-chain' is"
-                . " not yet supported while the VM is running.\n";
+            die "top-most snapshot of TPM state '$volid' on storage with 'snapshot-as-volume-chain'"
+                . " cannot be removed while the VM is running.\n";
         }
     }
 }
@@ -6074,14 +6074,6 @@ __PACKAGE__->register_method({
             0);
 
         my $realcmd = sub {
-            PVE::QemuConfig->lock_config(
-                $vmid,
-                sub {
-                    my $conf = PVE::QemuConfig->load_config($vmid);
-                    assert_tpm_snapshot_compat($vmid, $conf, 'create');
-                },
-            );
-
             PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname");
             PVE::QemuConfig->snapshot_create(
                 $vmid, $snapname, $param->{vmstate}, $param->{description},
@@ -6338,11 +6330,8 @@ __PACKAGE__->register_method({
                 $vmid,
                 sub {
                     my $conf = PVE::QemuConfig->load_config($vmid);
-                    assert_tpm_snapshot_compat(
-                        $vmid,
-                        $conf,
-                        'delete',
-                        $conf->{snapshots}->{$snapname},
+                    assert_tpm_snapshot_delete_possible(
+                        $vmid, $conf, $conf->{snapshots}->{$snapname}, $snapname,
                     );
                 },
             );
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel