[RFC PATCH storage 2/5] rbd: implement copy-offload via snapshot + clone + flatten

[Ciro Iriarte <cyruspy@gmail.com>]

A reference implementation on an in-tree backend, so the hook can be exercised
without vendor hardware.

Ceph produces a full copy without moving data through the host: snapshot the source,
clone the snapshot (copy-on-write, instant), then flatten the clone. The flatten is
the actual copy and runs on the Ceph cluster.

  prepare  create the target image, reserving the name
  start    snap (only when the caller supplied none) + clone. Both are O(1) metadata
           operations, so this is cheap enough to run inside a guest freeze, and it
           fixes the copy's point in time -- hence 'copy-offload-atomic'.
  status   pending while the clone still has a parent; complete once flattened

RBD is the case the "complete means INDEPENDENT, not merely readable" rule was
written for. A clone is fully readable the instant it exists, but deleting the source
out from under an unflattened clone destroys it.

prepare CREATES the target rather than only choosing a name. The caller releases the
storage lock between prepare and start, so a merely-chosen name could be taken by a
concurrent allocation, and the caller's rollback would then free a volume belonging to
that other operation. The placeholder is the smallest image rbd accepts: it only has
to hold the name until start renames the real clone over it, and start must delete it
while the caller may be holding a guest frozen. 'rbd rm' walks every object, so a
full-size placeholder would stall the guest in proportion to the disk -- anything up
to the 4 MiB object size is a single object and as cheap to delete as an image can be.

The flatten is handed to the Ceph manager with 'ceph rbd task add flatten': it runs
cluster-side, survives this worker dying, and is cancellable. That also makes failure
diagnosable -- status distinguishes complete (no parent), pending (a manager task for
the image, with progress) and failed (still a clone, but no task), where the last case
would otherwise be indistinguishable from a slow copy and poll forever.

Copying from a caller-supplied snapshot uses clone format 2, which does not require
protecting the parent -- the snapshot belongs to the caller and must not be
protected/unprotected behind their back.

Once the target is independent, status removes the snapshot the copy was taken from on
the source: that is the last moment it can, because a flattened image no longer records
its origin, and leaving it behind pins space and blocks deleting the source later. Only
snapshots this plugin created can match the derived name, never a caller's. free_image()
covers the same cleanup for a copy that failed rather than completed.

The copy-offload options are listed in options(), without which they cannot be set on a
storage at all and the feature is unreachable.

LIVE VALIDATION on a real Ceph cluster (Tentacle 20.2.1, erasure-coded pool). Standalone
copy: finished in 3s, target had NO parent, md5 identical to the source, and it still
read correctly after DELETING THE SOURCE -- the independence guarantee demonstrated
rather than asserted. Via qemu-server, full-cloning a RUNNING two-disk VM: the guest
freeze held only two snapshot creations and two single-object deletions, the guest was
resumed, both clones were independent and byte-identical, and no snapshot was left on
either source disk.

copy_images_start() is deliberately not overridden. Ceph group snapshots were tested on
the same cluster and do capture a common instant, but the snapshot is reachable only by
snap-id, cannot be protected, and above all an image may belong to only ONE group -- so
grouping would mean mutating the source images' group membership, colliding with any
grouping an administrator set up and leaving stray membership behind if a clone dies.
Since the caller already brackets every start in one guest freeze, that is a bad trade.

The clone is built under a prefixed staging name and the placeholder is MOVED aside
rather than deleted, because 'rbd rm' followed by 'rbd rename' leaves the disk number
unreserved in between. A concurrent allocation could take it there, and this copy's
rollback would then free that other operation's volume -- silent third-party data loss.

Both transient names are PREFIXES, and that is load-bearing in two opposite directions:

- rbd_ls(), which feeds list_images(), selects on the ANCHORED m/^(?:vm|base)-(\d+)-/,
  so a prefixed name never shows up as a phantom disk of that VM. The obvious
  '$volname.copytmp' SUFFIX does match it, so the staging clone was visible in the GUI
  for the whole duration of the copy -- which is the long part.
- find_free_diskname() passes the RAW 'rbd ls' output to get_next_vm_diskname(), whose
  disk-number regex is UNANCHORED, so a prefixed name still reserves the disk NUMBER.

Verified through the real allocator rather than by reading the regexes: with only the
parked placeholder present, get_next_vm_diskname() returns vm-101-disk-1, not disk-0.
copy_offload_naming_test.pm pins both halves, since nothing about the naming looks
load-bearing at a glance and a rename back to a suffix would reintroduce both bugs at
once without failing anything else.

Removing the placeholder is left to copy_image_status(): 'rbd rm' walks every object of
an image, and copy_image_start() can run while the caller holds a guest frozen. It is
best effort there -- the copy is already complete and correct by then, so dying would
make the caller free it. prepare() and free_image() also reap leftovers from a copy that
died mid-flight; they are invisible to list_images(), so nothing else ever would, and a
stale placeholder makes the rename fail for every future copy to that name.

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/RBDPlugin.pm         | 318 +++++++++++++++++++++++++++
 src/test/copy_offload_naming_test.pm |  82 +++++++
 src/test/run_plugin_tests.pl         |   1 +
 3 files changed, 401 insertions(+)
 create mode 100644 src/test/copy_offload_naming_test.pm

diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index b537425..31e9414 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -468,6 +468,8 @@ sub options {
         krbd => { optional => 1 },
         keyring => { optional => 1 },
         bwlimit => { optional => 1 },
+        'copy-offload' => { optional => 1 },
+        'copy-offload-timeout' => { optional => 1 },
     };
 }
 
@@ -701,6 +703,279 @@ sub clone_image {
     return $newvol;
 }
 
+# ---- storage-offloaded full copy -------------------------------------------------
+#
+# Ceph can produce a full copy without moving the data through the host: snapshot the
+# source, clone the snapshot (copy-on-write, instant), then flatten the clone. The
+# flatten is what actually copies, and it runs on the Ceph cluster.
+#
+# That maps onto the hook's three phases as follows:
+#
+#   prepare  pick the target name. Nothing exists on the cluster yet.
+#   start    snap + protect + clone. Both are O(1) metadata operations, so this is
+#            safe to run inside a guest freeze, and it is what FIXES the copy's point
+#            in time -- writes to the source afterwards cannot affect the clone.
+#   status   pending until the flatten has removed the clone's parent, i.e. until the
+#            target is genuinely INDEPENDENT and the source may be deleted.
+#
+# The last point is why 'complete' is defined as independent rather than readable: a
+# clone is fully readable the moment it exists, but deleting the source out from under
+# an unflattened clone would destroy it.
+
+my $rbd_copy_snap = sub {
+    my ($target_volname) = @_;
+    # Derived from the target name so status/cleanup can find it again without state.
+    return "__copy_$target_volname";
+};
+
+# Names for the two transient images a copy needs: the clone as it is being built, and
+# the placeholder once it has been moved out of the way.
+#
+# Both use a PREFIX. rbd_ls(), which feeds list_images(), selects on the ANCHORED
+# m/^(?:vm|base)-(\d+)-/, so a prefixed name is invisible there and never appears as a
+# phantom disk of that VM. find_free_diskname() meanwhile passes the RAW 'rbd ls' output
+# to get_next_vm_diskname(), whose disk-number regex is UNANCHORED, so a prefixed name
+# still reserves the disk NUMBER. Invisible and still reserving is exactly what these
+# need; a '.copytmp' SUFFIX would have been the opposite on both counts.
+my sub copy_staging_name { return "copynew-$_[0]" }
+my sub copy_parked_name { return "copytmp-$_[0]" }
+
+# Remove transient images a previous copy to this name left behind. They are invisible
+# to list_images(), so nothing else would ever reap them, and a leftover placeholder
+# would make the rename in copy_image_start() fail for every future copy to this name.
+my sub copy_reap_leftovers {
+    my ($scfg, $storeid, $volname) = @_;
+
+    for my $stale (copy_staging_name($volname), copy_parked_name($volname)) {
+        next if !rbd_volume_exists($scfg, $storeid, $stale);
+        warn "removing stale copy image '$stale'\n";
+        eval {
+            my $c = $rbd_cmd->($scfg, $storeid, 'rm', $stale);
+            run_rbd_command($c, errmsg => "rbd rm stale '$stale' error");
+        };
+        warn $@ if $@;
+    }
+}
+
+sub copy_image_prepare {
+    my (
+        $class, $scfg, $storeid, $volname,
+        $target_scfg, $target_storeid, $target_vmid, $snap, $opts,
+    ) = @_;
+
+    die "storage '$target_storeid' is on a different Ceph cluster - cannot clone across\n"
+        if ($scfg->{monhost} // '') ne ($target_scfg->{monhost} // '');
+
+    my $format = $opts->{format} // 'raw';
+    die "rbd copy offload cannot produce format '$format'\n" if $format ne 'raw';
+
+    my $name = $class->find_free_diskname($target_storeid, $target_scfg, $target_vmid);
+
+    # Safe here: we hold the target storage lock, and the name is ours because
+    # find_free_diskname() just handed it out.
+    copy_reap_leftovers($target_scfg, $target_storeid, $name);
+
+    # Actually CREATE the target, do not just pick a name. The caller runs this under
+    # the target storage lock and releases it before copy_image_start(), so a name that
+    # was merely chosen could be taken by a concurrent allocation in between -- and the
+    # caller's rollback would then free a volume belonging to that other operation.
+    # Creating it here makes the reservation real and the "prepared targets are
+    # freeable" contract true.
+    #
+    # Deliberately the SMALLEST image rbd accepts, not the source's size: this
+    # placeholder exists only to hold the name until copy_image_start() renames the real
+    # clone over it, and start has to delete it while the caller may be holding a guest
+    # frozen. 'rbd rm' walks every object of an image, so a full-size placeholder stalls
+    # the guest in proportion to the disk -- seconds for a GiB, far worse for a TiB.
+    # Anything up to the 4 MiB object size is a single object, so this is as cheap to
+    # delete as an image can be.
+    my $cmd = $rbd_cmd->(
+        $target_scfg, $target_storeid, 'create', '--image-format', '2',
+        '--size', '4K', $name,
+    );
+    push $cmd->@*, ('--data-pool', $target_scfg->{'data-pool'})
+        if $target_scfg->{'data-pool'};
+    run_rbd_command($cmd, errmsg => "rbd create '$name' error");
+
+    return $name;
+}
+
+sub copy_image_start {
+    my (
+        $class, $scfg, $storeid, $volname,
+        $target_scfg, $target_storeid, $target_volname, $snap,
+    ) = @_;
+
+    my (undef, $name) = $class->parse_volname($volname);
+
+    # Copy from the snapshot the caller asked for. Taking our own snapshot of the
+    # current image would silently copy live data when a snapshot was requested.
+    my $src_snap = $snap;
+    my $own_snap;
+    if (!defined($src_snap)) {
+        $own_snap = $rbd_copy_snap->($target_volname);
+        my $cmd = $rbd_cmd->($scfg, $storeid, 'snap', 'create', $name, '--snap', $own_snap);
+        run_rbd_command($cmd, errmsg => "rbd snap create '$name' error");
+        $src_snap = $own_snap;
+    }
+
+    my $tmp = copy_staging_name($target_volname);
+    my $parked = copy_parked_name($target_volname);
+
+    my $ok = eval {
+        # Clone format 2 so no snapshot protection is needed -- required when copying
+        # from a caller-supplied snapshot we do not own and must not protect/unprotect.
+        my @options = (
+            '--rbd-default-clone-format', '2',
+            get_rbd_path($scfg, $name), '--snap', $src_snap,
+        );
+        push @options, ('--data-pool', $target_scfg->{'data-pool'})
+            if $target_scfg->{'data-pool'};
+
+        my $cmd = $rbd_cmd->(
+            $scfg, $storeid, 'clone', @options, get_rbd_path($target_scfg, $tmp),
+        );
+        run_rbd_command($cmd, errmsg => "rbd clone '$name' error");
+
+        # Swap the clone into the reserved name. MOVE the placeholder aside rather than
+        # deleting it first: 'rbd rm' followed by 'rbd rename' leaves the disk number
+        # unreserved in between, and a concurrent allocation could take it -- after
+        # which this copy's rollback would free somebody else's volume. Parked under a
+        # prefixed name it stays invisible to list_images() while still reserving the
+        # number, so nothing can claim it. Removing it is left to copy_image_status(),
+        # since 'rbd rm' is not something to run while the caller holds a guest frozen.
+        $cmd = $rbd_cmd->($target_scfg, $target_storeid, 'rename', $target_volname, $parked);
+        run_rbd_command($cmd, errmsg => "rbd rename placeholder '$target_volname' error");
+        $cmd = $rbd_cmd->($target_scfg, $target_storeid, 'rename', $tmp, $target_volname);
+        if (!eval { run_rbd_command($cmd, errmsg => "rbd rename '$tmp' error"); 1 }) {
+            my $rerr = $@;
+            # Put the reservation back so the caller's rollback frees what it was given.
+            my $c = $rbd_cmd->($target_scfg, $target_storeid, 'rename', $parked,
+                $target_volname);
+            eval { run_rbd_command($c, errmsg => "restoring placeholder error") };
+            warn $@ if $@;
+            die $rerr;
+        }
+
+        # Hand the flatten to the Ceph manager rather than running it here: it is the
+        # actual data copy, it must not block a caller that may be holding a guest
+        # freeze, and a cluster-side task survives this worker dying. It is also
+        # cancellable, which a detached child process would not have been.
+        $cmd = ['ceph', 'rbd', 'task', 'add', 'flatten',
+            "$target_scfg->{pool}/$target_volname"];
+        run_command($cmd, errmsg => "scheduling rbd flatten failed", outfunc => sub { });
+        1;
+    };
+    if (my $err = $@) {
+        # Own cleanup: leave the source exactly as it was.
+        eval {
+            my $c = $rbd_cmd->($target_scfg, $target_storeid, 'rm', $tmp);
+            run_rbd_command($c, errmsg => "rbd rm '$tmp' error");
+        };
+        if (rbd_volume_exists($target_scfg, $target_storeid, $parked)) {
+            eval {
+                my $c = $rbd_cmd->($target_scfg, $target_storeid, 'rm', $parked);
+                run_rbd_command($c, errmsg => "rbd rm placeholder '$parked' error");
+            };
+            warn $@ if $@;
+        }
+        if ($own_snap) {
+            eval {
+                my $c = $rbd_cmd->($scfg, $storeid, 'snap', 'rm', $name, '--snap', $own_snap);
+                run_rbd_command($c, errmsg => "rbd snap rm '$name' error");
+            };
+        }
+        die $err;
+    }
+
+    return;
+}
+
+# Find the manager-side flatten task for an image, if one is queued or running.
+my $rbd_flatten_task = sub {
+    my ($scfg, $volname) = @_;
+
+    my $out = '';
+    eval {
+        run_command(['ceph', 'rbd', 'task', 'list', '--format', 'json'],
+            outfunc => sub { $out .= shift });
+    };
+    return undef if $@ || $out !~ /\S/;
+
+    my $tasks = eval { decode_json($out) } // [];
+    for my $task (@$tasks) {
+        my $refs = $task->{refs} // {};
+        next if ($refs->{action} // '') ne 'flatten';
+        next if ($refs->{image_name} // '') ne $volname;
+        next if ($refs->{pool_name} // '') ne ($scfg->{pool} // '');
+        return $task;
+    }
+
+    return undef;
+};
+
+sub copy_image_status {
+    my ($class, $scfg, $storeid, $volname, $source) = @_;
+
+    my (undef, $name) = $class->parse_volname($volname);
+
+    my $cmd = $rbd_cmd->($scfg, $storeid, 'info', $name, '--format', 'json');
+    my $info = '';
+    run_rbd_command($cmd, errmsg => "rbd info '$name' error", outfunc => sub { $info .= shift });
+
+    my $parent = eval { decode_json($info)->{parent} };
+
+    # No parent: the flatten finished and the image is INDEPENDENT -- which is the
+    # difference that matters. A clone is readable from the moment it exists, but
+    # deleting the source before the flatten completes would destroy it.
+    if (!$parent) {
+        # Drop the snapshot this copy was taken from, on the SOURCE image. Once the
+        # flatten completes the target no longer records where it came from, so this is
+        # the last chance to find it -- and leaving it behind pins space on the source
+        # and blocks deleting that volume later.
+        #
+        # Only ever our own: the name is derived from the target, so a caller-supplied
+        # snapshot can never match and is left untouched.
+        if ($source) {
+            my $own_snap = $rbd_copy_snap->($volname);
+            my (undef, $src_name) = $class->parse_volname($source->{volname});
+            eval {
+                my $c = $rbd_cmd->(
+                    $source->{scfg}, $source->{storeid},
+                    'snap', 'rm', $src_name, '--snap', $own_snap,
+                );
+                run_rbd_command($c, errmsg => "rbd snap rm error", outfunc => sub { });
+            };
+        }
+        # The placeholder copy_image_start() parked. Deliberately not removed there:
+        # 'rbd rm' walks every object and that call can run with a guest frozen. Best
+        # effort -- the copy is already complete and correct, and dying here would make
+        # the caller free it.
+        my $parked = copy_parked_name($name);
+        if (rbd_volume_exists($scfg, $storeid, $parked)) {
+            eval {
+                my $c = $rbd_cmd->($scfg, $storeid, 'rm', $parked);
+                run_rbd_command($c, errmsg => "rbd rm placeholder '$parked' error");
+            };
+            warn $@ if $@;
+        }
+
+        return { state => 'complete' };
+    }
+
+    # Still parented. Distinguish a running flatten from one that died: without this a
+    # dead flatten leaves the image parented forever and the caller polls indefinitely.
+    my $task = $rbd_flatten_task->($scfg, $name);
+    die "flatten of '$volname' is no longer running and the image is still a clone\n"
+        if !$task;
+
+    my $progress = $task->{progress};
+    return {
+        state => 'pending',
+        (defined($progress) ? (progress => int($progress * 100)) : ()),
+    };
+}
+
 sub alloc_image {
     my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
 
@@ -720,11 +995,39 @@ sub alloc_image {
     return $name;
 }
 
+# Parent (image + snapshot) of a clone, or undef if the image stands alone.
+sub rbd_volume_info_parent {
+    my ($scfg, $storeid, $volname) = @_;
+
+    my $cmd = $rbd_cmd->($scfg, $storeid, 'info', $volname, '--format', 'json');
+    my $raw = '';
+    run_rbd_command($cmd, errmsg => "rbd info '$volname' error", outfunc => sub { $raw .= shift });
+
+    return eval { decode_json($raw)->{parent} };
+}
+
 sub free_image {
     my ($class, $storeid, $scfg, $volname, $isBase) = @_;
 
     my ($vtype, $name, $vmid, undef, undef, undef) = $class->parse_volname($volname);
 
+    # Transient images from a copy to this name that died mid-flight. They are invisible
+    # to list_images(), so freeing the volume they belong to is the only occasion
+    # anything would think to look for them.
+    copy_reap_leftovers($scfg, $storeid, $name);
+
+    # An offloaded copy that never finished is still a clone of a snapshot this plugin
+    # took on the SOURCE image. Freeing the target releases the child reference but
+    # would leave that snapshot behind, pinning space and blocking deletion of the
+    # source. The clone itself is the only record of where it came from, so read the
+    # parent before removing it and drop the snapshot afterwards -- but only when it is
+    # one of ours, never a snapshot the caller supplied.
+    my $copy_parent;
+    if (my $info = eval { rbd_volume_info_parent($scfg, $storeid, $name) }) {
+        my $psnap = $info->{snapshot} // '';
+        $copy_parent = $info if $psnap =~ /^__copy_/;
+    }
+
     my $snaps = rbd_ls_snap($scfg, $storeid, $name);
     foreach my $snap (keys %$snaps) {
         if ($snaps->{$snap}->{protected}) {
@@ -741,6 +1044,17 @@ sub free_image {
     $cmd = $rbd_cmd->($scfg, $storeid, 'rm', $name);
     run_rbd_command($cmd, errmsg => "rbd rm '$name' error");
 
+    if ($copy_parent) {
+        # Best effort: another unfinished copy may still hold the same snapshot.
+        eval {
+            my $c = $rbd_cmd->(
+                $scfg, $storeid, 'snap', 'rm', $copy_parent->{image},
+                '--snap', $copy_parent->{snapshot},
+            );
+            run_rbd_command($c, errmsg => "rbd snap rm error", outfunc => sub { });
+        };
+    }
+
     return undef;
 }
 
@@ -962,6 +1276,10 @@ sub volume_has_feature {
         copy => { base => 1, current => 1, snap => 1 },
         sparseinit => { base => 1, current => 1 },
         rename => { current => 1 },
+        # The copy is produced from an RBD snapshot, so it comes from a static point
+        # in time and needs no host-side mirror to stay consistent while the source
+        # keeps changing -- see copy_image_start().
+        'copy-offload-atomic' => { base => 1, current => 1, snap => 1 },
     };
 
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) = $class->parse_volname($volname);
diff --git a/src/test/copy_offload_naming_test.pm b/src/test/copy_offload_naming_test.pm
new file mode 100644
index 0000000..9de30f1
--- /dev/null
+++ b/src/test/copy_offload_naming_test.pm
@@ -0,0 +1,82 @@
+package PVE::Storage::TestCopyOffloadNaming;
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+use PVE::Storage;
+use PVE::Storage::Plugin;
+use Test::More;
+
+# The names copy_image_start() parks its placeholder under must satisfy TWO opposing
+# properties at once, and getting either one wrong is a silent, expensive bug:
+#
+#   1. INVISIBLE to the volume lister. If the lister accepts the name, the placeholder
+#      shows up as a real disk of that VM -- a phantom the GUI offers to attach, and a
+#      permanent one if the copy dies mid-flight.
+#   2. STILL RESERVING the disk NUMBER. The caller drops the storage lock between
+#      prepare() and start(), so if the parked name stops reserving, a concurrent
+#      allocation can take it -- and the failing copy's rollback then frees somebody
+#      else's volume. That is silent third-party data loss.
+#
+# The two properties come from two different regexes, which is why a name can satisfy
+# one and not the other:
+#
+#   - listers are ANCHORED   (rbd_ls: m/^(?:vm|base)-(\d+)-/,
+#                             LvmThin list_images: m/^(vm|base)-(\d+)-/)
+#   - $get_vm_disk_number is UNANCHORED (Plugin.pm)
+#
+# So a PREFIX is invisible but still reserves, and a SUFFIX is the exact opposite on
+# both counts. This test pins that, because nothing about the naming looks load-bearing
+# at a glance and a well-meaning rename to 'vm-101-disk-0.copytmp' would reintroduce
+# both bugs at once without failing anything else.
+
+# The anchored patterns the listers actually use, kept here so a change to either one
+# is caught rather than silently narrowing what these names protect.
+my $rbd_lister_re = qr/^(?:vm|base)-(\d+)-/;
+my $lvmthin_lister_re = qr/^(vm|base)-(\d+)-/;
+
+my $tests = [
+    # [ name, listed?, reserves a disk number? ]
+    ['vm-101-disk-0', 1, 1], # the real volume: listed and reserving
+    ['copytmp-vm-101-disk-0', 0, 1], # parked placeholder
+    ['copynew-vm-101-disk-0', 0, 1], # staging clone
+    ['vm-101-disk-0.copytmp', 1, 1], # the SUFFIX form: listed => phantom. Do not use.
+];
+
+plan tests => scalar($tests->@*) * 3 + 2;
+
+for my $t ($tests->@*) {
+    my ($name, $listed, $reserves) = $t->@*;
+
+    is(!!($name =~ $rbd_lister_re), !!$listed, "rbd lister: '$name' listed = " . ($listed ? 1 : 0));
+    is(
+        !!($name =~ $lvmthin_lister_re), !!$listed,
+        "lvmthin lister: '$name' listed = " . ($listed ? 1 : 0),
+    );
+
+    # Unanchored on purpose -- this is the reservation half.
+    is(
+        !!($name =~ qr/(vm|base)-101-disk-(\d+)/), !!$reserves,
+        "'$name' reserves a disk number = " . ($reserves ? 1 : 0),
+    );
+}
+
+# The property that actually matters, through the real allocator rather than a regex:
+# with ONLY the parked placeholder present, disk 0 must not be handed out again.
+my $scfg = { type => 'rbd' };
+is(
+    PVE::Storage::Plugin::get_next_vm_diskname(['vm-101-disk-0'], 'st', 101, undef, $scfg),
+    'vm-101-disk-1',
+    'the real volume reserves its disk number',
+);
+is(
+    PVE::Storage::Plugin::get_next_vm_diskname(
+        ['copytmp-vm-101-disk-0'], 'st', 101, undef, $scfg,
+    ),
+    'vm-101-disk-1',
+    'a parked placeholder ALONE still reserves the disk number',
+);
+
+done_testing();
diff --git a/src/test/run_plugin_tests.pl b/src/test/run_plugin_tests.pl
index dda4cac..8f2472e 100755
--- a/src/test/run_plugin_tests.pl
+++ b/src/test/run_plugin_tests.pl
@@ -18,6 +18,7 @@ my $res = $harness->runtests(
     "filesystem_path_test.pm",
     "prune_backups_test.pm",
     "copy_offload_test.pm",
+    "copy_offload_naming_test.pm",
 );
 
 exit -1 if !$res || $res->{failed} || $res->{parse_errors};
-- 
2.54.0