From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id C1FB01FF0AF for ; Thu, 10 Sep 2026 17:04:25 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 460042156A; Thu, 10 Sep 2026 17:04:21 +0200 (CEST) From: =?UTF-8?q?Michael=20K=C3=B6ppl?= To: pve-devel@lists.proxmox.com Subject: [PATCH e2e-tests 1/1] tests: storage: report a non-qcow2 volume-chain storage as a setup gap Date: Thu, 10 Sep 2026 17:04:07 +0200 Message-ID: <20260910150407.1430309-1-m.koeppl@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789052646165 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.725 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: HYDYMOTUQMFSSDPQDSJBVN4SPK2YALG3 X-Message-ID-Hash: HYDYMOTUQMFSSDPQDSJBVN4SPK2YALG3 X-MailFrom: m.koeppl@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: For storage that can only snapshot through volume chains, the snapshot feature is only enabled for qcow2 volumes. LVM switches its default format to qcow2 once `snapshot-as-volume-chain` is enabled. On other storages, disks keep being allocated as raw, so every snapshot attempt fails with "snapshot feature not available" and the suite reported the snapshot capabilities as unsupported, even though the plugin implements them. The plugin is not strictly missing the capability. However, it is a setup gap, so report is accordingly. The volume-chain snapshot mechanism itself stays covered by the volume-chain snapshot test, which allocates qcow2 because that is what it verifies. The messages now also name the format that was tried, which makes it easier to tell the two "unsupported" cases apart. Reported-by: Hannes Dürr Signed-off-by: Michael Köppl --- The initial idea here was to automatically detect whether qcow2 should be used based on whether the storage config had `snapshot-as-volume-chain 1`. However, making that assumption seems wrong. For LVM, PVE automatically changes the disk format to qcow2 if snapshot-as-volume-chain is enabled. If a custom storage requires the features to be enabled in order to support snapshots, the configuration should also be adapted accordingly instead of the test suite picking the disk format for the storage plugin automatically. Should this be applied, the section from the README should also be added to the wiki page. Proxmox/Test/Util.pm | 43 ++++++++++++++++++- storage-plugin-tests/README | 18 ++++++++ storage-plugin-tests/tests/disk_snapshot.pl | 12 +++--- .../tests/snapshot_rollback_data.pl | 6 ++- .../tests/vm_destroy_with_snapshots.pl | 12 ++++-- 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/Proxmox/Test/Util.pm b/Proxmox/Test/Util.pm index 9b38924..0701747 100644 --- a/Proxmox/Test/Util.pm +++ b/Proxmox/Test/Util.pm @@ -3,8 +3,17 @@ use strict; use warnings FATAL => 'all'; use parent 'Exporter'; -our @EXPORT_OK = - qw(poll_until retry skip_unsupported skip_prerequisite guest_name node_from_upid backup_target); +our @EXPORT_OK = qw( + poll_until + retry + skip_unsupported + skip_prerequisite + skip_failed_snapshot + guest_name + node_from_upid + backup_target + guest_disk_format +); use Time::HiRes qw(time sleep); use Test::More (); @@ -99,6 +108,22 @@ sub skip_prerequisite { Test::More::plan(skip_all => "PREREQUISITE: $reason"); } +# Skip test if snapshot could not be taken. A storage that snapshots through volume chains offers +# the snapshot feature for qcow2 volumes only, so a volume in another format means the storage is +# configured such that its own snapshot mechanism cannot be used at all. Suggest a re-run with an +# updated configuration instead of assuming a plugin gap. +sub skip_failed_snapshot { + my ($storage_cfg, $storage_id, $vol_format, $error) = @_; + + skip_prerequisite("storage '$storage_id' requires qcow2 volumes for snapshots through volume" + . " chains, but the disk was allocated as '$vol_format'. Change the storage's default" + . " format or set 'format qcow2' on the storage and re-run") + if $storage_cfg->{'snapshot-as-volume-chain'} && $vol_format ne 'qcow2'; + + skip_unsupported("the plugin does not support snapshots of '$vol_format' volumes" + . (length($error // '') ? ": $error" : '')); +} + # Where a backup test should dump to: the storage under test if it can hold backups, else the # validated --backup-fallback-storage. $content is the storage's content-type hashref. No # auto-picking: that could grab a disabled storage and fail the run for an unrelated reason. Skips @@ -126,4 +151,18 @@ sub backup_target { return $fallback; } +# The format the storage reports for a guest's disk. Returns 'unknown' when the volume cannot be +# queried. +sub guest_disk_format { + my ($pve, $storage_id, $vmid, $drive) = @_; + + my $node = $pve->get_nodename(); + my $cfg = eval { $pve->client()->get("/nodes/$node/qemu/$vmid/config", {}) } // {}; + my ($volid) = ($cfg->{$drive} // '') =~ m/^([^,]+)/; + return 'unknown' if !$volid; + + my $vol = eval { $pve->client()->get("/nodes/$node/storage/$storage_id/content/$volid", {}) }; + return ($vol // {})->{format} // 'unknown'; +} + 1; diff --git a/storage-plugin-tests/README b/storage-plugin-tests/README index 5f486a4..165888d 100644 --- a/storage-plugin-tests/README +++ b/storage-plugin-tests/README @@ -50,6 +50,24 @@ They need either internet access plus `libguestfs-tools` (so a Debian cloud image can be downloaded and prepared once), or `--qga-image` pointing at your own agent-equipped image. +Snapshots and the disk format +----------------------------- + +PVE offers the snapshot feature for storages that snapshot through volume chains +(`snapshot-as-volume-chain`) only for qcow2 volumes. An LVM storage switches its +default format to qcow2 as soon as that option is enabled. Other storages keep +allocating `raw`, so no disk created on them the usual way can be snapshotted at +all. + +The suite tests the storage as it is configured and does not pick a format of +its own, so on such a storage the snapshot capabilities come out as not tested +until the storage allocates qcow2 by default: + + pvesm set mystorage --format qcow2 + +The one exception is `volume-chain snapshots`, which allocates a qcow2 disk +itself, because a qcow2 backing chain is the mechanism it verifies. + Reading the result ------------------ diff --git a/storage-plugin-tests/tests/disk_snapshot.pl b/storage-plugin-tests/tests/disk_snapshot.pl index 8191069..2a20a51 100755 --- a/storage-plugin-tests/tests/disk_snapshot.pl +++ b/storage-plugin-tests/tests/disk_snapshot.pl @@ -6,7 +6,7 @@ use Test::More; use lib '..'; use Proxmox::Test::PVEInstance; -use Proxmox::Test::Util qw(skip_unsupported skip_prerequisite guest_name); +use Proxmox::Test::Util qw(skip_failed_snapshot skip_prerequisite guest_name guest_disk_format); my $storage_id = $ENV{PLUGIN_STORAGE_ID}; if (!$storage_id) { @@ -93,9 +93,7 @@ $pve->task_ok($upid, "VM started successfully") or do { }; # A direct write at offset 0 of a non-raw volume (qcow2) would corrupt the image header. -my $vol_format = - eval { $pve->client()->get("/nodes/$node/storage/$storage_id/content/$volid", {})->{format}; } - // 'unknown'; +my $vol_format = guest_disk_format($pve, $storage_id, $vmid, 'scsi0'); my $can_verify_data = 0; if ($vol_format eq 'raw') { $can_verify_data = $write_marker->('A'); @@ -103,8 +101,8 @@ if ($vol_format eq 'raw') { diag("volume format '$vol_format' cannot be verified by direct writes, skipping data checks"); } -# A plugin without snapshot support errors on the API call or fails the task; report that as -# unsupported instead of failing. +# A plugin without snapshot support for this volume format errors here. Report that as unsupported +# or as a storage setup gap instead of failing. my $snapshot = sub { my ($name) = @_; return $pve->client()->post("/nodes/$node/qemu/$vmid/snapshot", { snapname => $name }); @@ -121,7 +119,7 @@ if (!$pve->task_successful($snap1_result)) { . ($snap1_result // 'n/a') . ", err=$@). Plugin likely doesn't support snapshots."); $pve->destroy_guest(qemu => $vmid); - skip_unsupported("the plugin does not support snapshots"); + skip_failed_snapshot($storage_cfg, $storage_id, $vol_format); } pass("snap1 created"); diff --git a/storage-plugin-tests/tests/snapshot_rollback_data.pl b/storage-plugin-tests/tests/snapshot_rollback_data.pl index 2ab5c5d..2dd8773 100755 --- a/storage-plugin-tests/tests/snapshot_rollback_data.pl +++ b/storage-plugin-tests/tests/snapshot_rollback_data.pl @@ -6,7 +6,7 @@ use Test::More; use lib '..'; use Proxmox::Test::PVEInstance; -use Proxmox::Test::Util qw(skip_unsupported skip_prerequisite guest_name); +use Proxmox::Test::Util qw(skip_failed_snapshot skip_prerequisite guest_name guest_disk_format); use Proxmox::Test::StorageGuest qw(boot_storage_guest guest_write_marker guest_read_marker); # Verifies snapshot rollback restores on-disk data, for any format: a guest writes a marker, the @@ -47,8 +47,10 @@ eval { ); }; if ($@ || !$pve->task_successful($snap)) { + my $err = $@ || $snap; + my $vol_format = guest_disk_format($pve, $storage_id, $vmid, 'scsi0'); $pve->destroy_guest(qemu => $vmid); - skip_unsupported("the plugin does not support snapshots: " . ($@ || $snap)); + skip_failed_snapshot($scfg, $storage_id, $vol_format, $err); } pass('took a disk snapshot'); diff --git a/storage-plugin-tests/tests/vm_destroy_with_snapshots.pl b/storage-plugin-tests/tests/vm_destroy_with_snapshots.pl index 0e18bb7..35639b6 100755 --- a/storage-plugin-tests/tests/vm_destroy_with_snapshots.pl +++ b/storage-plugin-tests/tests/vm_destroy_with_snapshots.pl @@ -6,7 +6,8 @@ use Test::More; use lib '..'; use Proxmox::Test::PVEInstance; -use Proxmox::Test::Util qw(skip_unsupported skip_prerequisite guest_name poll_until); +use Proxmox::Test::Util + qw(skip_failed_snapshot skip_prerequisite guest_name poll_until guest_disk_format); # Verifies that a VM can be destroyed while snapshots still exist, without deleting them one by one # first. @@ -73,8 +74,8 @@ my $snap1_upid = eval { $snapshot->('snap1') }; my $snap1_result; $snap1_result = eval { $pve->wait_for_task($snap1_upid, 1) } if $snap1_upid; -# A plugin without snapshot support errors on the API call or fails the task. Report that as -# unsupported instead of failing. +# A plugin without snapshot support for this volume format errors here. Report that as unsupported +# or as a storage setup gap instead of failing. if (!$pve->task_successful($snap1_result)) { diag("First snapshot did not succeed (UPID=" . ($snap1_upid // 'undef') @@ -82,9 +83,12 @@ if (!$pve->task_successful($snap1_result)) { . ($snap1_result // 'n/a') . ", err=$@). Plugin likely doesn't support snapshots."); + # Query the format while the volume is still around for printing in the skip message. + my $vol_format = guest_disk_format($pve, $storage_id, $vmid, 'scsi0'); + $pve->destroy_guest(qemu => $vmid); - skip_unsupported("the plugin does not support snapshots"); + skip_failed_snapshot($storage_cfg, $storage_id, $vol_format); } pass("snap1 created"); -- 2.47.3