From 41c6e4bf7a28420c3b3b4d517d394c0ae647ee36 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Wed, 16 Jul 2025 15:16:24 +0200 Subject: [PATCH] replace volume_support_qemu_snapshot with volume_qemu_snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This also changes the return values, since their meanings are rather weird from the storage point of view. For instance, "internal" meant it is *not* the storage which does the snapshot, while "external" meant a mixture of storage and qemu-server side actions. `undef` meant the storage does it all... ┌────────────┬───────────┐ │ previous │ new │ ├────────────┼───────────┤ │ "internal" │ "qemu" │ │ "external" │ "mixed" │ │ undef │ "storage" │ └────────────┴───────────┘ Signed-off-by: Wolfgang Bumiller --- ApiChangeLog | 28 +++++++++++++++++++++------- src/PVE/Storage.pm | 4 ++-- src/PVE/Storage/DirPlugin.pm | 7 +++---- src/PVE/Storage/LVMPlugin.pm | 5 +++-- src/PVE/Storage/Plugin.pm | 9 +++++---- src/PVE/Storage/RBDPlugin.pm | 5 +++-- 6 files changed, 37 insertions(+), 21 deletions(-) diff --git a/ApiChangeLog b/ApiChangeLog index 58996a9..1b06fa7 100644 --- a/ApiChangeLog +++ b/ApiChangeLog @@ -25,13 +25,27 @@ Future changes should be documented in here. * Introduce rename_snapshot() plugin method This method allow to rename a vm disk snapshot name to a different snapshot name. -* Introduce volume_support_qemu_snapshot() plugin method - This method is used to known if the a snapshot need to be done by qemu - or by the storage api. - returned values are : - 'internal' : support snapshot with qemu internal snapshot - 'external' : support snapshot with qemu external snapshot - undef : don't support qemu snapshot +* Introduce volume_qemu_snapshot_method() plugin method + This method declares how snapshots should be handled for *running* VMs. + This should return one of the following: + 'qemu': + Qemu must perform the snapshot. The storage plugin does nothing. + 'storage': + The storage plugin *transparently* performs the snapshot and the running VM does not need to + do anything. + 'mixed': + For taking a snapshot: The storage performs an offline snapshot and qemu then has to reopen + the volume. + For removing a snapshot: One of 2 things will happen (both must be supported): + a) Qemu will "unhook" the snapshot by moving its data into the child snapshot, and then call + `volume_snapshot_delete` with `running` set, in which case the storage should delete only + the snapshot without touching the surrounding snapshots. + b) Qemu will "commit" the child snapshot to the one which is being removed, then call + `volume_snapshot_delete()` on the child snapshot, then call `rename_snapshot()` to move the + merged snapshot into place. + NOTE: Storages must support using "current" as a special name in `rename_snapshot()` to + cheaply convert a snapshot into the current disk state and back. + ## Version 11: diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 893b611..6ca9f88 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -2370,7 +2370,7 @@ sub rename_snapshot { ); } -sub volume_support_qemu_snapshot { +sub volume_qemu_snapshot_method { my ($cfg, $volid) = @_; my ($storeid, $volname) = parse_volume_id($volid, 1); @@ -2380,7 +2380,7 @@ sub volume_support_qemu_snapshot { my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); - return $plugin->volume_support_qemu_snapshot($storeid, $scfg, $volname); + return $plugin->volume_qemu_snapshot_method($storeid, $scfg, $volname); } return undef; } diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm index 543aacb..d12b11e 100644 --- a/src/PVE/Storage/DirPlugin.pm +++ b/src/PVE/Storage/DirPlugin.pm @@ -315,14 +315,13 @@ sub get_import_metadata { }; } -sub volume_support_qemu_snapshot { +sub volume_qemu_snapshot_method { my ($class, $storeid, $scfg, $volname) = @_; my $format = ($class->parse_volname($volname))[6]; - return if $format ne 'qcow2'; + return 'storage' if $format ne 'qcow2'; - my $type = $scfg->{'external-snapshots'} ? 'external' : 'internal'; - return $type; + return $scfg->{'external-snapshots'} ? 'mixed' : 'qemu'; } 1; diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index 1241e93..ca8dded 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -1301,11 +1301,12 @@ sub rename_snapshot { lvrename($scfg, $source_snap_volname, $target_snap_volname); } -sub volume_support_qemu_snapshot { +sub volume_qemu_snapshot_method { my ($class, $storeid, $scfg, $volname) = @_; my $format = ($class->parse_volname($volname))[6]; - return 'external' if $format eq 'qcow2'; + return 'mixed' if $format eq 'qcow2'; + return 'storage'; } 1; diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 2e16c59..800f201 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -2428,9 +2428,9 @@ sub new_backup_provider { =pod -=head3 volume_support_qemu_snapshot +=head3 volume_qemu_snapshot_method - $blockdev = $plugin->volume_support_qemu_snapshot($storeid, $scfg, $volname) + $blockdev = $plugin->volume_qemu_snapshot_method($storeid, $scfg, $volname) Returns a string with the type of snapshot that qemu can do for a specific volume @@ -2439,11 +2439,12 @@ Returns a string with the type of snapshot that qemu can do for a specific volum undef : don't support qemu snapshot =cut -sub volume_support_qemu_snapshot { +sub volume_qemu_snapshot_method { my ($class, $storeid, $scfg, $volname) = @_; my $format = ($class->parse_volname($volname))[6]; - return 'internal' if $format eq 'qcow2'; + return 'qemu' if $format eq 'qcow2'; + return 'storage'; } sub config_aware_base_mkdir { diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm index 2201900..cf371c7 100644 --- a/src/PVE/Storage/RBDPlugin.pm +++ b/src/PVE/Storage/RBDPlugin.pm @@ -1063,10 +1063,11 @@ sub rename_snapshot { die "rename_snapshot is not implemented for $class"; } -sub volume_support_qemu_snapshot { +sub volume_qemu_snapshot_method { my ($class, $storeid, $scfg, $volname) = @_; - return 'internal' if !$scfg->{krbd}; + return 'qemu' if !$scfg->{krbd}; + return 'storage'; } 1;