From a81ee831271f1f256bccf264ff42649346da6d1a Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Thu, 17 Jul 2025 19:46:13 +0200 Subject: [PATCH] config: rename external-snapshots to snapshot-as-volume-chain Not perfect but now it's still easy to rename and the new variant fits a bit better to the actual design and implementation. Add best-effort migration for storage.cfg, this has been never publicly released after all. Signed-off-by: Thomas Lamprecht --- debian/postinst | 11 +++++++++++ src/PVE/API2/Storage/Status.pm | 2 +- src/PVE/Storage/CIFSPlugin.pm | 2 +- src/PVE/Storage/DirPlugin.pm | 4 ++-- src/PVE/Storage/LVMPlugin.pm | 6 +++--- src/PVE/Storage/NFSPlugin.pm | 2 +- src/PVE/Storage/Plugin.pm | 31 +++++++++++++++++-------------- 7 files changed, 36 insertions(+), 22 deletions(-) diff --git a/debian/postinst b/debian/postinst index 9dbb3b9..c793d80 100644 --- a/debian/postinst +++ b/debian/postinst @@ -31,6 +31,17 @@ case "$1" in fi done fi + + # TODO: Can be dropped with some 9.x stable release, this was never in a publicly available + # package, so only for convenience for internal testing setups. + if dpkg --compare-versions "$2" 'lt' '9.0.5'; then + if grep -Pq '^\texternal-snapshots ' /etc/pve/storage.cfg; then + echo "Replacing old 'external-snapshots' with 'snapshot-as-volume-chain' in /etc/pve/storage.cfg" + sed -i 's/^\texternal-snapshots /\tsnapshot-as-volume-chain /' /etc/pve/storage.cfg || \ + echo "Failed to replace old 'external-snapshots' with 'snapshot-as-volume-chain' in /etc/pve/storage.cfg" + fi + fi + fi ;; diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm index 6ca5db6..25933f8 100644 --- a/src/PVE/API2/Storage/Status.pm +++ b/src/PVE/API2/Storage/Status.pm @@ -223,7 +223,7 @@ __PACKAGE__->register_method({ # TODO: add support to the storage plugin system to allow returing different supported # formats depending on the storage config instead, this is just a stop gap! if (lc($data->{type}) eq 'lvm') { - $data->{format}[0]->{qcow2} = 0 if !$scfg->{'external-snapshots'}; + $data->{format}->[0]->{qcow2} = 0 if !$scfg->{'snapshot-as-volume-chain'}; } $res->{$storeid} = $data; diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm index a79f68d..75d89c1 100644 --- a/src/PVE/Storage/CIFSPlugin.pm +++ b/src/PVE/Storage/CIFSPlugin.pm @@ -168,7 +168,7 @@ sub options { bwlimit => { optional => 1 }, preallocation => { optional => 1 }, options => { optional => 1 }, - 'external-snapshots' => { optional => 1, fixed => 1 }, + 'snapshot-as-volume-chain' => { optional => 1, fixed => 1 }, }; } diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm index d12b11e..9d15f33 100644 --- a/src/PVE/Storage/DirPlugin.pm +++ b/src/PVE/Storage/DirPlugin.pm @@ -95,7 +95,7 @@ sub options { is_mountpoint => { optional => 1 }, bwlimit => { optional => 1 }, preallocation => { optional => 1 }, - 'external-snapshots' => { optional => 1, fixed => 1 }, + 'snapshot-as-volume-chain' => { optional => 1, fixed => 1 }, }; } @@ -321,7 +321,7 @@ sub volume_qemu_snapshot_method { my $format = ($class->parse_volname($volname))[6]; return 'storage' if $format ne 'qcow2'; - return $scfg->{'external-snapshots'} ? 'mixed' : 'qemu'; + return $scfg->{'snapshot-as-volume-chain'} ? 'mixed' : 'qemu'; } 1; diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index ca8dded..7044c4f 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -399,7 +399,7 @@ sub options { base => { fixed => 1, optional => 1 }, tagged_only => { optional => 1 }, bwlimit => { optional => 1 }, - 'external-snapshots' => { optional => 1 }, + 'snapshot-as-volume-chain' => { optional => 1 }, }; } @@ -604,9 +604,9 @@ my sub alloc_lvm_image { die "unsupported format '$fmt'" if $fmt ne 'raw' && $fmt ne 'qcow2'; - die "external-snapshots option need to be enabled to use qcow2 format" + die "snapshot-as-volume-chain option need to be enabled to use qcow2 format" if $fmt eq 'qcow2' - && !$scfg->{'external-snapshots'}; + && !$scfg->{'snapshot-as-volume-chain'}; $class->parse_volname($name); diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm index 849b46d..a8339ef 100644 --- a/src/PVE/Storage/NFSPlugin.pm +++ b/src/PVE/Storage/NFSPlugin.pm @@ -104,7 +104,7 @@ sub options { 'create-subdirs' => { optional => 1 }, bwlimit => { optional => 1 }, preallocation => { optional => 1 }, - 'external-snapshots' => { optional => 1, fixed => 1 }, + 'snapshot-as-volume-chain' => { optional => 1, fixed => 1 }, }; } diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 800f201..ae97dfb 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -228,9 +228,11 @@ my $defaultData = { maximum => 65535, optional => 1, }, - 'external-snapshots' => { + 'snapshot-as-volume-chain' => { type => 'boolean', - description => 'Enable external snapshot.', + description => 'Enable support for creating storage-vendor agnostic snapshot' + . ' through volume backing-chains.', + default => 0, optional => 1, }, }, @@ -792,7 +794,7 @@ sub filesystem_path { my ($vtype, $name, $vmid, undef, undef, $isBase, $format) = $class->parse_volname($volname); $name = get_snap_name($class, $volname, $snapname) - if $scfg->{'external-snapshots'} && $snapname; + if $scfg->{'snapshot-as-volume-chain'} && $snapname; # Note: qcow2/qed has internal snapshot, so path is always # the same (with or without snapshot => same file). @@ -1055,13 +1057,13 @@ sub free_image { } my $snapshots = undef; - if ($scfg->{'external-snapshots'}) { + if ($scfg->{'snapshot-as-volume-chain'}) { $snapshots = $class->volume_snapshot_info($scfg, $storeid, $volname); } unlink($path) || die "unlink '$path' failed - $!\n"; - #delete external snapshots - if ($scfg->{'external-snapshots'}) { + # delete snapshots using a volume backing chaing layered by qcow2 + if ($scfg->{'snapshot-as-volume-chain'}) { for my $snapid ( sort { $snapshots->{$b}->{order} <=> $snapshots->{$a}->{order} } keys %$snapshots @@ -1274,7 +1276,7 @@ sub volume_resize { sub volume_snapshot { my ($class, $scfg, $storeid, $volname, $snap) = @_; - if ($scfg->{'external-snapshots'}) { + if ($scfg->{'snapshot-as-volume-chain'}) { die "can't snapshot this image format\n" if $volname !~ m/\.(qcow2)$/; @@ -1311,7 +1313,7 @@ sub volume_snapshot { sub volume_rollback_is_possible { my ($class, $scfg, $storeid, $volname, $snap, $blockers) = @_; - return 1 if !$scfg->{'external-snapshots'}; + return 1 if !$scfg->{'snapshot-as-volume-chain'}; #technically, we could manage multibranch, we it need lot more work for snapshot delete #we need to implemente block-stream from deleted snapshot to all others child branchs @@ -1348,7 +1350,7 @@ sub volume_snapshot_rollback { die "can't rollback snapshot this image format\n" if $volname !~ m/\.(qcow2|qed)$/; - if ($scfg->{'external-snapshots'}) { + if ($scfg->{'snapshot-as-volume-chain'}) { #simply delete the current snapshot and recreate it eval { free_snap_image($class, $storeid, $scfg, $volname, 'current') }; if ($@) { @@ -1375,7 +1377,7 @@ sub volume_snapshot_delete { my $cmd = ""; - if ($scfg->{'external-snapshots'}) { + if ($scfg->{'snapshot-as-volume-chain'}) { #qemu has already live commit|stream the snapshot, therefore we only have to drop the image itself if ($running) { @@ -2009,7 +2011,7 @@ sub volume_export { = @_; die "cannot export volumes together with their snapshots in $class\n" - if $with_snapshots && $scfg->{'external-snapshots'}; + if $with_snapshots && $scfg->{'snapshot-as-volume-chain'}; my $err_msg = "volume export format $format not available for $class\n"; if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) { @@ -2173,9 +2175,10 @@ sub rename_volume { die "not implemented in storage plugin '$class'\n" if $class->can('api') && $class->api() < 10; die "no path found\n" if !$scfg->{path}; - if ($scfg->{'external-snapshots'}) { + if ($scfg->{'snapshot-as-volume-chain'}) { my $snapshots = $class->volume_snapshot_info($scfg, $storeid, $source_volname); - die "we can't rename volume if external snapshot exists" if $snapshots->{current}->{parent}; + die "we can't rename volume if a snapshot backed by a volume-chain exists\n" + if $snapshots->{current}->{parent}; } my ( @@ -2331,7 +2334,7 @@ sub qemu_blockdev_options { my $format = ($class->parse_volname($volname))[6]; die "cannot attach only the snapshot of a '$format' image\n" if $options->{'snapshot-name'} - && ($format eq 'qcow2' && !$scfg->{'external-snapshots'} || $format eq 'qed'); + && ($format eq 'qcow2' && !$scfg->{'snapshot-as-volume-chain'} || $format eq 'qed'); # The 'file' driver only works for regular files. The check below is taken from # block/file-posix.c:hdev_probe_device() in QEMU. Do not bother with detecting 'host_cdrom'