From: Ciro Iriarte <cyruspy@gmail.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH storage] btrfs: fix clone_image cloning live data instead of the requested snapshot
Date: Mon, 07 Sep 2026 02:06:38 -0300 [thread overview]
Message-ID: <20260907.btrfsclonesnapfix@cyruspy.gmail.com> (raw)
clone_image() takes $snap but never passed it to filesystem_path(), so it always
resolved the live subvolume. Cloning from a snapshot therefore produced a clone of
the CURRENT state while the caller believed it had cloned the snapshot.
This is reachable from the GUI and the API, not a dormant path: volume_has_feature()
advertises 'clone' for the 'snap' key on raw volumes, and PVE::Storage::clone_image()
passes the snapshot straight through, so cloning a VM from one of its snapshots on a
btrfs storage silently gets current data. Nothing errors out and the clone is
perfectly readable, which is what makes it easy to miss -- the disk simply holds
different content than the snapshot it was named after.
Reproduced on a loop-backed btrfs by writing known content, snapshotting, then
overwriting the source: clone_image($snap) returned the post-overwrite md5. With
$snap passed through it returns the snapshot's md5.
Found while implementing copy-offload for this plugin, which needs the same
distinction and is where the question came up.
Generated-By: Claude (https://claude.ai)
Signed-off-by: Ciro Iriarte <ciro.iriarte+software@gmail.com>
Co-Authored-By: Claude <noreply@anthropic.com>
---
src/PVE/Storage/BTRFSPlugin.pm | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index fb47aa0..f58a1bb 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -298,7 +298,10 @@ sub clone_image {
$imagedir .= "/$vmid";
mkpath $imagedir;
- my $path = $class->filesystem_path($scfg, $volname);
+ # Clone the snapshot the caller asked for, not the live subvolume. Dropping $snap
+ # here silently produced a clone of the CURRENT state while the caller believed it
+ # had cloned the snapshot.
+ my $path = $class->filesystem_path($scfg, $volname, $snap);
my $newname = $class->find_free_diskname($storeid, $scfg, $vmid, $format, 1);
# For btrfs subvolumes we don't actually need the "link":
--
2.54.0
reply other threads:[~2026-09-07 5:09 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260907.btrfsclonesnapfix@cyruspy.gmail.com \
--to=cyruspy@gmail.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.