public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Ciro Iriarte <cyruspy@gmail.com>
To: pve-devel@lists.proxmox.com
Subject: [RFC PATCH qemu-server 5/5] use storage copy-offload for full clone
Date: Mon, 20 Jul 2026 07:30:23 -0700 (PDT)	[thread overview]
Message-ID: <20260720.5.copyoffload@cyruspy.gmail.com> (raw)
In-Reply-To: <20260720.0.copyoffload@cyruspy.gmail.com>

Consumer side of the pve-storage copy-offload hook.

clone_disk()'s full-clone path does two things: vdisk_alloc() the target, then fill it
(drive-mirror for a running guest, qemu-img convert otherwise). An offloaded copy
replaces BOTH, because the backend produces the target itself. So the branch sits
before the allocation rather than beside the mirror/convert calls, and jumps to
no_data_clone as the cloudinit path already does.

The running-guest case is the interesting one. Cross-disk consistency here is a
VM-level property, not a per-volume one: clone_vm() calls clone_disk() once per drive
with completion => 'skip' until the last, and BlockJob::monitor() then waits for ALL
mirror jobs to be ready, fs-freezes ONCE, and cuts every disk over at that single
instant. An offload started per-disk inside clone_disk() would have no such
rendezvous, so each disk would be captured at a different instant and a running
multi-disk guest could be cloned torn -- a database with data and journal on separate
disks being the obvious casualty.

Rather than restrict offload to stopped guests, this reuses that rendezvous. For a
running source clone_disk() only ALLOCATES the target and defers the copy's start --
the operation that fixes its point in time -- into $deferred_copies. monitor() grows
an optional on_frozen callback, run while the guest is frozen at the same instant the
mirrors are cut over, and the deferred starts happen there. Offloaded and mirrored
disks of one VM therefore share a point in time, and mixed VMs work without a second
freeze. The freeze stays short: a start is one backend call, and the data movement is
waited for afterwards with the guest running again.

Two paths have no mirror to rendezvous with. If every disk was offloaded there are no
jobs at all, so clone_vm() freezes itself, mirroring monitor()'s freeze including the
suspend/resume fallback when no guest agent is available. And if the LAST disk is a
deferred one, nothing after it would call monitor(), so that branch runs the
rendezvous itself -- otherwise earlier mirrors, left at completion => 'skip', would
never be completed.

Everything done while the guest is stopped is now inside one eval whose error is held
until after the thaw, including the block-job cancel: leaving a guest frozen or
suspended is worse than failing the clone.

Guards: run_deferred_copies() is idempotent, clone_vm()'s fallback keys on unstarted
copies rather than on %$jobs so it cannot double-start if job bookkeeping changes, and
wait_deferred_copies() dies on an unstarted copy rather than skipping it -- its target
is allocated but its content undefined, so returning it would hand out a corrupt disk.

Each deferred copy carries its $snapname and its source volid. The start is what fixes
the point in time, so without the snapname a backend would copy the live disk while
reporting success; the source lets a plugin clean up state it left there once the copy
is independent.

'bulk' still falls through to the host-side path for a running guest: it smears over a
changing source and needs the bitmap-seed + mirror convergence, which is not
implemented here.

Excluded drives: cloudinit is regenerated rather than copied, and efidisk0/tpmstate0
are allocated at a fixed size and copied with a size-limited qemu-img dd, which an
opaque backend copy cannot reproduce. Checked on both source and destination drive
names. Skipping clone_disk_check_io_uring() here is intentional: it returns early
unless $use_drive_mirror, which an offloaded copy never sets.

Validated on a real Ceph cluster by full-cloning a RUNNING two-disk VM where both
disks offload: the all-offloaded fallback established its own freeze, the guest was
suspended and resumed, and both clones came out independent and byte-identical.

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/API2/Qemu.pm           |  17 +++
 src/PVE/QemuServer.pm          | 201 ++++++++++++++++++++++++++++++++-
 src/PVE/QemuServer/BlockJob.pm |  32 +++++-
 3 files changed, 245 insertions(+), 5 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 28cbb9b0..1d7d427b 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -4599,6 +4599,9 @@ __PACKAGE__->register_method({
 
             my $newvollist = [];
             my $jobs = {};
+            # storage-offloaded copies whose start is deferred into the guest freeze, so
+            # that every disk of a running VM is captured at the same instant
+            my $deferred_copies = [];
 
             eval {
                 local $SIG{INT} = local $SIG{TERM} = local $SIG{QUIT} = local $SIG{HUP} =
@@ -4650,6 +4653,7 @@ __PACKAGE__->register_method({
                         $completion,
                         $oldconf->{agent},
                         $clonelimit,
+                        $deferred_copies,
                     );
 
                     $newconf->{$opt} = PVE::QemuServer::print_drive($newdrive);
@@ -4658,6 +4662,19 @@ __PACKAGE__->register_method({
                     $i++;
                 }
 
+                # Anything still unstarted means there was no mirror cutover to
+                # piggyback the freeze on -- i.e. every disk was offloaded -- so freeze
+                # here. Keyed on the copies themselves rather than on %$jobs, so it
+                # cannot double-start if the job bookkeeping changes.
+                if (grep { !$_->{started} } @$deferred_copies) {
+                    PVE::QemuServer::freeze_and_run_deferred_copies(
+                        $storecfg, $vmid, $oldconf->{agent}, $deferred_copies,
+                    );
+                }
+
+                # the guest is running again; the data movement is waited for here
+                PVE::QemuServer::wait_deferred_copies($storecfg, $deferred_copies);
+
                 delete $newconf->{lock};
 
                 # do not write pending changes
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 191ae549..3fc10d8c 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7923,8 +7923,96 @@ my sub clone_disk_check_io_uring {
         if $src_uses_io_uring && !storage_allows_io_uring_default($dst_scfg, $cache_direct);
 }
 
+# Start every deferred storage-offloaded copy. MUST be called with the guest frozen (or
+# suspended): the start is what fixes each copy's point in time, so doing them all here
+# is what makes a multi-disk clone consistent.
+#
+# Handed to the storage layer as ONE batch rather than started in a loop, so a backend
+# that can capture several volumes at a single instant (an array consistency group) does
+# so. That makes the disks mutually crash-consistent in the backend itself rather than
+# only by virtue of this freeze, and collapses N backend calls into one -- which is what
+# keeps the guest's stall short on a VM with many disks.
+#
+# Kept deliberately cheap: the guest is stalled for the duration. The data movement runs
+# afterwards and is waited for by wait_deferred_copies(), with the guest running again.
+sub run_deferred_copies {
+    my ($storecfg, $deferred_copies) = @_;
+
+    return if !$deferred_copies;
+
+    # idempotent: never start a copy twice
+    my $pending = [grep { !$_->{started} } @$deferred_copies];
+    return if !scalar(@$pending);
+
+    # vdisk_copy_start_group() flags each copy as it starts it, so a failure partway
+    # through leaves an accurate record of what is actually running.
+    PVE::Storage::vdisk_copy_start_group($storecfg, $pending);
+
+    return;
+}
+
+# Wait for the deferred copies to become independent of their sources. Runs with the
+# guest already thawed: only the starts need the freeze, not the data movement.
+sub wait_deferred_copies {
+    my ($storecfg, $deferred_copies) = @_;
+
+    return if !$deferred_copies;
+
+    for my $copy (@$deferred_copies) {
+        # Never silently accept an unstarted copy: its target is allocated but its
+        # content is undefined, so returning it would hand out a corrupt disk.
+        die "internal error - offloaded copy of '$copy->{source}' was never started\n"
+            if !$copy->{started};
+
+        print("waiting for offloaded copy of '$copy->{source}' to become independent\n");
+        PVE::Storage::vdisk_copy_wait($storecfg, $copy->{target}, $copy->{source});
+    }
+}
+
+# Freeze the guest, start the deferred copies, thaw. Used when a clone has NO mirror
+# jobs to rendezvous with -- i.e. every disk was offloaded -- so there is no
+# BlockJob::monitor() freeze to piggyback on. Mirrors what monitor() does, including the
+# suspend/resume fallback when no guest agent is available.
+sub freeze_and_run_deferred_copies {
+    my ($storecfg, $vmid, $qga, $deferred_copies) = @_;
+
+    return if !$deferred_copies || !scalar(@$deferred_copies);
+
+    my $should_fsfreeze = PVE::QemuServer::Agent::guest_fs_freeze_applicable($qga, $vmid);
+    if ($should_fsfreeze) {
+        print "issuing guest agent 'guest-fsfreeze-freeze' command\n";
+        eval { PVE::QemuServer::Agent::guest_fs_freeze($vmid); };
+        warn $@ if $@;
+    } else {
+        print "suspend vm\n";
+        eval { PVE::QemuServer::RunState::vm_suspend($vmid, 1); };
+        warn $@ if $@;
+    }
+
+    eval { run_deferred_copies($storecfg, $deferred_copies) };
+    my $err = $@;
+
+    if ($should_fsfreeze) {
+        print "issuing guest agent 'guest-fsfreeze-thaw' command\n";
+        eval { PVE::QemuServer::Agent::guest_fs_thaw($vmid); };
+        warn $@ if $@;
+    } else {
+        print "resume vm\n";
+        eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1); };
+        warn $@ if $@;
+    }
+
+    die $err if $err; # only after the guest is running again
+}
+
+# $deferred_copies is an optional arrayref collecting storage-offloaded copies whose
+# START must happen inside the caller's guest freeze; see the offload branch below and
+# run_deferred_copies(). Without it, a running source is never offloaded.
 sub clone_disk {
-    my ($storecfg, $source, $dest, $full, $newvollist, $jobs, $completion, $qga, $bwlimit) = @_;
+    my (
+        $storecfg, $source, $dest, $full, $newvollist, $jobs, $completion, $qga, $bwlimit,
+        $deferred_copies,
+    ) = @_;
 
     my ($vmid, $running) = $source->@{qw(vmid running)};
     my ($src_drivename, $drive, $snapname) = $source->@{qw(drivename drive snapname)};
@@ -7968,6 +8056,113 @@ sub clone_disk {
                 . " '$dst_format' instead\n";
         }
 
+        # Storage-offloaded full copy (pve-storage copy_image).
+        #
+        # This replaces BOTH halves of the normal full-clone path: the backend creates
+        # the target volume itself, so there is no vdisk_alloc() here and no host-side
+        # copy afterwards. That is why the check sits before the allocation rather than
+        # as another branch next to the mirror/convert calls below.
+        #
+        # Skipped for the special drives: cloudinit is regenerated rather than copied,
+        # and efidisk0/tpmstate0 are allocated at a fixed size and copied with a
+        # size-limited qemu-img dd, which an opaque backend copy cannot reproduce.
+        # Checked on both drive names, since either side being special is disqualifying.
+        my $special_re = qr/^(?:efidisk0|tpmstate0)$/;
+        my $offload_special = drive_is_cloudinit($drive)
+            || (defined($dst_drivename) && $dst_drivename =~ $special_re)
+            || (defined($src_drivename) && $src_drivename =~ $special_re);
+
+        # Multi-disk consistency is a VM-level property here, not a per-volume one:
+        # clone_vm() calls clone_disk() once per drive with completion => 'skip' until
+        # the last one, and BlockJob::monitor() then waits for ALL mirror jobs to be
+        # ready, fs-freezes ONCE, and cuts every disk over at that single instant. An
+        # offload started per-disk right here would have no such rendezvous, so each disk
+        # would be captured at a different instant and a running multi-disk guest could
+        # be cloned torn -- a database whose data and journal live on separate disks
+        # being the obvious casualty.
+        #
+        # So for a RUNNING source the target is only ALLOCATED here, and the copy's start
+        # -- which is what fixes its point in time -- is deferred to $deferred_copies.
+        # The caller starts those inside the same freeze that cuts the mirrors over, so
+        # offloaded and mirrored disks of one VM share an instant. A stopped source needs
+        # no rendezvous and is copied inline.
+        my $offload_class = $offload_special
+            ? undef
+            : PVE::Storage::copy_offload_class(
+                $storecfg, $drive->{file}, $storeid, $snapname, $running,
+            );
+
+        # 'bulk' smears over a changing source, so it needs the bitmap-seed + mirror
+        # convergence for a running guest, which is not implemented: fall through to the
+        # host-side path rather than silently producing an inconsistent copy.
+        $offload_class = undef if $offload_class && $offload_class ne 'atomic' && $running;
+
+        # deferral requires a caller that will run the rendezvous
+        $offload_class = undef if $offload_class && $running && !$deferred_copies;
+
+        if ($offload_class && $running) {
+            print("offloading full copy to storage backend ($offload_class, deferred)\n");
+
+            $newvolid = PVE::Storage::vdisk_copy_prepare(
+                $storecfg, $drive->{file}, $storeid, $newvmid, $snapname,
+                { format => $dst_format },
+            );
+            push @$newvollist, $newvolid;
+
+            # recorded as plain volids: the starts are issued as one batch, so that a
+            # backend able to group them can capture every disk at a single instant
+            push @$deferred_copies, {
+                source => $drive->{file},
+                target => $newvolid,
+                snap => $snapname,
+            };
+
+            print("allocated target volume '$newvolid', copy starts at freeze\n");
+
+            # If this is the last disk and mirror jobs are pending, the rendezvous has
+            # to happen here: nothing after us will call monitor(), and those mirrors
+            # would never be completed. Starting the deferred copies from inside that
+            # same freeze is also what puts every disk at one instant.
+            if (($completion && $completion eq 'complete') && (scalar(keys %$jobs) > 0)) {
+                PVE::QemuServer::BlockJob::monitor(
+                    vm_qmp_peer($vmid), $newvmid, $jobs, $completion, $qga, undef,
+                    sub { run_deferred_copies($storecfg, $deferred_copies) },
+                );
+            }
+
+            goto no_data_clone;
+        }
+
+        if ($offload_class) {
+            # Complete any mirror jobs from previously cloned disks before finishing
+            # here, exactly as the cloudinit path below does: this disk may be the last
+            # one, and returning without completing would leave those transfers unfinished.
+            if (($completion && $completion eq 'complete') && (scalar(keys %$jobs) > 0)) {
+                PVE::QemuServer::BlockJob::monitor(
+                    vm_qmp_peer($vmid), $newvmid, $jobs, $completion, $qga,
+                );
+            }
+
+            print("offloading full copy to storage backend ($offload_class)\n");
+
+            $newvolid = PVE::Storage::vdisk_copy(
+                $storecfg,
+                $drive->{file},
+                $storeid,
+                $newvmid,
+                $snapname,
+                { format => $dst_format },
+                sub { print("copy progress: $_[0]%\n") },
+            );
+            push @$newvollist, $newvolid;
+
+            print("created target volume '$newvolid' on the backend\n");
+
+            PVE::Storage::activate_volumes($storecfg, [$newvolid]);
+
+            goto no_data_clone;
+        }
+
         my $name = undef;
         my $size = undef;
         if (drive_is_cloudinit($drive)) {
@@ -8024,6 +8219,10 @@ sub clone_disk {
             my $mirror_opts = {};
             $mirror_opts->{'guest-agent'} = $qga;
             $mirror_opts->{bwlimit} = $bwlimit if defined($bwlimit);
+            # Start any deferred offloaded copies inside the freeze this mirror's
+            # completion takes, putting every disk of the VM at one point in time.
+            $mirror_opts->{'on-frozen'} = sub { run_deferred_copies($storecfg, $deferred_copies) }
+                if $deferred_copies && scalar(@$deferred_copies);
             PVE::QemuServer::BlockJob::mirror(
                 $source_info,
                 $dest_info,
diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 921f046c..f48b95e3 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -83,8 +83,12 @@ sub qemu_blockjobs_cancel {
 # 'cancel': wait until all jobs are ready, block-job-cancel them
 # 'skip': wait until all jobs are ready, return with block jobs in ready state
 # 'auto': wait until all jobs disappear, only use for jobs which complete automatically
+#
+# $on_frozen is an optional coderef, run while the guest is frozen (or suspended) at the
+# instant the mirror jobs are cut over. Clone uses it to start storage-offloaded copies
+# there, so that a VM's offloaded and mirrored disks share one point in time.
 sub monitor {
-    my ($qmp_peer, $vmiddst, $jobs, $completion, $qga, $op) = @_;
+    my ($qmp_peer, $vmiddst, $jobs, $completion, $qga, $op, $on_frozen) = @_;
 
     die "drive mirror: different destination is only supported when peer is main QEMU instance\n"
         if $vmiddst && $qmp_peer->{type} ne 'qmp';
@@ -179,8 +183,22 @@ sub monitor {
                         warn $@ if $@;
                     }
 
-                    # if we clone a disk for a new target vm, we don't switch the disk
-                    qemu_blockjobs_cancel($qmp_peer, $jobs);
+                    # Everything done while the guest is stopped goes in here, and its
+                    # error is held until after the thaw below. Leaving a guest frozen or
+                    # suspended is far worse than failing the clone, so nothing between
+                    # the freeze and the thaw may die -- including the job cancel, which
+                    # can fail on its own.
+                    my $frozen_err;
+                    eval {
+                        # Storage-offloaded copies start here: their point in time is
+                        # fixed by the start, so doing it inside the freeze is what puts
+                        # them at the same instant as the mirrored disks cut over below.
+                        $on_frozen->() if $on_frozen;
+
+                        # if we clone a disk for a new target vm, we don't switch the disk
+                        qemu_blockjobs_cancel($qmp_peer, $jobs);
+                    };
+                    $frozen_err = $@;
 
                     if ($should_fsfreeze) {
                         print "issuing guest agent 'guest-fsfreeze-thaw' command\n";
@@ -192,6 +210,9 @@ sub monitor {
                         warn $@ if $@;
                     }
 
+                    # re-raise only now that the guest is running again
+                    die $frozen_err if $frozen_err;
+
                     last;
                 } else {
 
@@ -281,6 +302,7 @@ sub qemu_drive_mirror {
         $qga,
         $bwlimit,
         $src_bitmap,
+        $on_frozen,
     ) = @_;
 
     my $device_id = "drive-$drive_id";
@@ -316,7 +338,7 @@ sub qemu_drive_mirror {
         die "mirroring error: $err\n";
     }
 
-    monitor(vm_qmp_peer($vmid), $vmiddst, $jobs, $completion, $qga);
+    monitor(vm_qmp_peer($vmid), $vmiddst, $jobs, $completion, $qga, undef, $on_frozen);
 }
 
 # Callers should version guard this (only available with a binary >= QEMU 8.2)
@@ -520,6 +542,7 @@ sub blockdev_mirror {
         $completion,
         $options->{'guest-agent'},
         'mirror',
+        $options->{'on-frozen'},
     );
 }
 
@@ -543,6 +566,7 @@ sub mirror {
             $options->{'guest-agent'},
             $options->{bwlimit},
             $source->{bitmap},
+            $options->{'on-frozen'},
         );
     }
 }
-- 
2.54.0




      parent reply	other threads:[~2026-07-20 14:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 14:29 [RFC PATCH storage, qemu-server 0/5] offload full clone to the storage backend Ciro Iriarte
2026-07-20 14:29 ` [RFC PATCH storage 1/5] storage: add asynchronous copy-offload hook for full copies Ciro Iriarte
2026-07-20 14:29 ` [RFC PATCH storage 2/5] rbd: implement copy-offload via snapshot + clone + flatten Ciro Iriarte
2026-07-20 14:30 ` [RFC PATCH storage 3/5] dir: implement copy-offload via reflink (FICLONE) Ciro Iriarte
2026-07-20 14:30 ` [RFC PATCH storage 4/5] btrfs, lvmthin: implement copy-offload (atomic class) Ciro Iriarte
2026-07-20 14:30 ` Ciro Iriarte [this message]

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=20260720.5.copyoffload@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal