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 26F161FF130 for ; Mon, 20 Jul 2026 16:30:29 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 3355B21587; Mon, 20 Jul 2026 16:30:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784557782; x=1785162582; darn=lists.proxmox.com; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=BzEE8wvMInER9OXNzH2i5LAn7YtsB0jRAb2P6/2BD8k=; b=UP0lDXWC8cTFTDa2yT/6mFNZ7nuOuFc42Yg2sPnr8XSTeownzzrX9XA1hpQsdpasgm DDLbbtuvyeISBMm8ReEXGuRyGWlcXwOV5yHwrLQ0C+9hLXCtYKP/8Sqbjeliw8kNNJAY vEizZet2yU6RLWpkgpgZP5PaVDsTgeRc7aef8j3e/7v8Acnt9gzrGhNZGJ6OsAqC2kq8 LSK2ygh64mRY/BKGbf60KqMSjfM8poNvL2KKcU6txpINN/oWuz/L4+RSsYLBIKQG85UI 00vIOuQHYZDIsI2QzhvV0b9scnb6IdawrNWglEdjD8REYjt7z3DFiYMAOfyrXAL4y5OI K0Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784557782; x=1785162582; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=BzEE8wvMInER9OXNzH2i5LAn7YtsB0jRAb2P6/2BD8k=; b=NiwzmI+fmlZwOQ4LGBY647pUJjXzTfWpz3Ha5G6ddedTtVeXOwPiCxmI7QwJ4GqNw9 nIbF5hS8fta3sdpSnI+Al2XPNx/TUkQH3J/Af12REXnK98JhWFqTQEyfssE1GnK4Ho/S yTR8mQ5zhaA7FD+B02dLMUC0k/wYpd6hp0mTaZxAqpPSjlPQG7apCyOuN2IpWo0cxTPx DUi4XlAt1iiPtxaknXomkYdqpndeTpEK2nQuMAKNWxdlZ2Dnci/2uKFJf7Ji7EAQcvxK r/h6fyuGWJljgxQ/EbOKZrW+wc0VBDw5jvw1YM14muB2A9fgzTSHZVsn4Q4E6AD1iMv8 8keA== X-Gm-Message-State: AOJu0YyWExSq33anHP1PIP6TjdF6hlNgIiPluQQjq6mPdAoLKseBE9bW g50CoUkhJzBrkAl8WdXVWjJsz7WwH95N7Hk12yHaLCcvJ6kLK7okoS7p X-Gm-Gg: AfdE7ckgCdJNlhE7IZ/HA/LZvmJuxrIKypyWYudVSJD77f5SQNkXAj8JQtl98VpE4YT C7iuQUfLq84L4+MeYaCgkis43+i0Xee4I+jKS+fPLEGGEuRbawjUjO0LhywwwVI005AhJCkptZc ohWEkfZBWVJqNKmn2ASMIwqIe0zoATNgguFvAh8suaUb/bGRhQVPiVmbxiR1RR5T7TpJJy84WB4 oeVfdyUId0jVQVL5P82SUjCoUjVr1pHOdN1h/fQ49W85yozVVGa6gskzTmaleozzwcB8sKNI37E 3Fylq+eupBDs7ESYY8YZUD4SJHIBYH1UPt7BUD6xKmT4if3cfRSjYRpUTDSiBnbGicBXEDTH90n hrNFgkuP6z6YhjhrGm24WEbHwYKYfNyLVy3aO6Q8Cd9barHi7Q5n2R0qxF8jhCNA3ihhbuNxR X-Received: by 2002:a05:6a21:4584:b0:3bf:a11c:176e with SMTP id adf61e73a8af0-3c3ada95162mr13551970637.57.1784557781780; Mon, 20 Jul 2026 07:29:41 -0700 (PDT) Date: Mon, 20 Jul 2026 07:29:41 -0700 (PDT) From: Ciro Iriarte To: pve-devel@lists.proxmox.com Subject: [RFC PATCH storage 1/5] storage: add asynchronous copy-offload hook for full copies Message-ID: <20260720.1.copyoffload@cyruspy.gmail.com> In-Reply-To: <20260720.0.copyoffload@cyruspy.gmail.com> References: <20260720.0.copyoffload@cyruspy.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.080 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy FREEMAIL_FROM 0.001 Sender email is commonly abused enduser mail provider KAM_SHORT 0.001 Use of a URL Shortener for very short URL RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no 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: WNIQ5R6PAJDRNMQLYA5E4ZT5I3UALEZC X-Message-ID-Hash: WNIQ5R6PAJDRNMQLYA5E4ZT5I3UALEZC X-MailFrom: cyruspy@gmail.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: Lets a plugin perform a full copy (e.g. a full clone) on the storage backend instead of reading and writing the data through the host with qemu-img. Following the pve-devel discussion: the base methods die rather than returning undef (they are only reached when a plugin advertised a copy-offload feature); the plugin owns cleanup; it receives $target_vmid and the TARGET storage config, since a plugin only sees its own per-instance scfg and cannot otherwise decide whether a source/target pair is copyable; offload is a plain per-storage 'copy-offload' option; and scope is two instances of the same plugin type, as the operation is opaque to PVE and has no volume_export/import-style lowering. The two capability classes are separate features rather than one feature with a class argument, so volume_has_feature()'s interface is unchanged (Fiona's suggestion). A caller that can drive both prefers atomic. The hook is split into three phases, for two independent reasons: copy_image_prepare() allocate the target - under the storage lock copy_image_start() pin the source PIT, begin - inside a guest freeze copy_image_status() poll until independent - outside both It cannot be synchronous: the core calls into the plugin under cluster_lock_storage(), and for shared storage that is a pmxcfs lock whose locked code is killed by an alarm(60) in PVE::Cluster, while a full array copy runs for minutes to hours. And allocation cannot happen inside a guest freeze, because cross-disk consistency requires holding one freeze across every disk of a VM -- so only the start, a single cheap backend call, runs frozen. copy_image_start() receives the source $snapname. It is the call that fixes the point in time, so a plugin that instead snapshotted the current image would silently copy live data when a snapshot was requested. copy_image_status() likewise receives what the copy was made FROM, so a plugin can clean up state it left on the source once the target is independent -- after that, nothing else records where the copy came from. 'complete' means INDEPENDENT of the source, not merely readable: for backends whose independence is asynchronous -- an RBD flatten, a ZFS clone still tied to its origin, an array pair that has not dissolved -- reporting completion early yields something that is not a full copy, after which the caller is free to delete the source. copy_images_start() starts a batch between one pair of storages. Its default simply starts them one by one, so there is no capability to negotiate; a backend that can capture several volumes at one instant overrides it. Grouping policy is left to the plugin, which knows about pools, group size limits and an administrator's own grouping of disks by role. What the core guarantees is documented on the method: every start in a batch happens inside one guest freeze, so consistency is backend-guaranteed WITHIN a group and freeze-guaranteed ACROSS groups. vdisk_copy_wait() bounds the wait with a deadline, exposed as the 'copy-offload-timeout' storage option, which resets on reported progress -- so it bounds a stalled copy without killing a slow one. Without it a backend whose copy died reports 'pending' forever and hangs the worker with no error. vdisk_copy_start_group() flags each copy as it starts it, so a failure partway through a batch leaves an accurate record of which copies are actually running; cleanup can otherwise neither reclaim a running copy nor stop waiting for one that never began. APIVER 15 -> 16, APIAGE 6 -> 7: both bump so the minimum accepted plugin API stays 15-6 = 16-7 = 9, since the new methods are optional and older plugins keep working. Generated-By: Claude (https://claude.ai) Signed-off-by: Ciro Iriarte Co-Authored-By: Claude --- ApiChangeLog | 24 ++++ src/PVE/Storage.pm | 237 +++++++++++++++++++++++++++++++++- src/PVE/Storage/Plugin.pm | 176 +++++++++++++++++++++++++ src/test/copy_offload_test.pm | 90 +++++++++++++ src/test/run_plugin_tests.pl | 1 + 5 files changed, 526 insertions(+), 2 deletions(-) create mode 100644 src/test/copy_offload_test.pm diff --git a/ApiChangeLog b/ApiChangeLog index 80c5994..f9505f4 100644 --- a/ApiChangeLog +++ b/ApiChangeLog @@ -6,6 +6,30 @@ without breaking anything unaware of it.) Future changes should be documented in here. +## Version 16: + +* Add optional `copy_image_prepare()`, `copy_image_start()`, `copy_image_status()` and + `copy_images_start()` plugin methods, plus a `copy-offload` storage option + + These let a plugin perform a full copy (e.g. a full clone) on the storage backend instead of + reading and writing the data through the host. They are optional: the base implementations die, + and are only reached if a plugin advertises one of the new `copy-offload-atomic` or + `copy-offload-bulk` features from `volume_has_feature()`, so existing plugins are unaffected. + + The copy is asynchronous. `copy_image_prepare()` allocates the target and returns its volname, + `copy_image_start()` fixes the copy's point in time and begins it, and `copy_image_status()` is + polled until the target is INDEPENDENT of its source -- not merely readable. Splitting the two + matters for callers that must capture several volumes at one instant: only the start needs to + happen inside a guest freeze, and it must stay cheap enough to do so. + + `copy_images_start()` starts a batch between the same pair of storages. Its default + implementation simply starts them one by one, so it needs no capability negotiation; a backend + that can capture several volumes atomically overrides it. + + Note that `copy_image_start()` receives the source `$snapname`: it is the call that fixes the + point in time, so a plugin that instead snapshots the current image would silently copy live + data when a snapshot was requested. + ## Version 15: * Add new `$snapname` parameter to the `volume_resize()` plugin method diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 64ea9da..76189d0 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -41,11 +41,11 @@ use PVE::Storage::BTRFSPlugin; use PVE::Storage::ESXiPlugin; # Storage API version. Increment it on changes in storage API interface. -use constant APIVER => 15; +use constant APIVER => 16; # Age is the number of versions we're backward compatible with. # This is like having 'current=APIVER' and age='APIAGE' in libtool, # see https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html -use constant APIAGE => 6; +use constant APIAGE => 7; our $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 'qcow2+size', 'vmdk+size', 'zfs', 'btrfs']; @@ -1098,6 +1098,239 @@ sub vdisk_clone { ); } +# Returns the copy-offload class usable for $volid -> $target_storeid, or undef. +# +# Offload requires: the target storage to have copy-offload enabled, both storages to be +# instances of the same plugin type, and the plugin to advertise a class for this volume. +# 'atomic' is preferred when a plugin advertises both, as it needs no host-side mirror. +sub copy_offload_class { + my ($cfg, $volid, $target_storeid, $snap, $running) = @_; + + my ($storeid, $volname) = parse_volume_id($volid, 1); + return undef if !$storeid; + + my $scfg = storage_config($cfg, $storeid); + my $target_scfg = storage_config($cfg, $target_storeid); + + return undef if !$target_scfg->{'copy-offload'}; + return undef if $scfg->{type} ne $target_scfg->{type}; + + for my $class (qw(atomic bulk)) { + return $class + if volume_has_feature($cfg, "copy-offload-$class", $volid, $snap, $running); + } + + return undef; +} + +# Allocate the target of an offloaded copy, returning the new volid. Its CONTENT is +# undefined until vdisk_copy_start() has been called for it. +# +# Split out from the start so a caller cloning a running guest can prepare every disk's +# target first and then start them all inside a single guest freeze; see vdisk_copy(). +# +# Only this half runs under the storage lock -- it is where the allocation happens, and +# it is bounded. The copy itself must not run under the lock: for shared storage that is +# a pmxcfs lock whose locked code is killed by an alarm(60) in PVE::Cluster, while a full +# array copy runs for minutes to hours. The host-side path is arranged the same way, +# allocating under the lock and running the long qemu-img convert outside it. +sub vdisk_copy_prepare { + my ($cfg, $volid, $target_storeid, $target_vmid, $snap, $opts) = @_; + + my ($storeid, $volname) = parse_volume_id($volid); + + my $scfg = storage_config($cfg, $storeid); + my $target_scfg = storage_config($cfg, $target_storeid); + + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); + + activate_storage($cfg, $storeid); + activate_storage($cfg, $target_storeid) if $target_storeid ne $storeid; + + my $new_volname = $plugin->cluster_lock_storage( + $target_storeid, + $target_scfg->{shared}, + undef, + sub { + return $plugin->copy_image_prepare( + $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_vmid, $snap, $opts, + ); + }, + ); + + return "$target_storeid:$new_volname"; +} + +# Pin the source's point-in-time and begin copying into a prepared target. +# +# MUST stay cheap: a caller cloning a running guest holds a filesystem freeze across +# this call for every disk of the VM. It deliberately takes no storage lock -- the +# allocation already happened in vdisk_copy_prepare(), and taking a cluster lock inside +# a guest freeze would be a bad trade. +sub vdisk_copy_start { + my ($cfg, $volid, $target_volid, $snap) = @_; + + my ($storeid, $volname) = parse_volume_id($volid); + my ($target_storeid, $target_volname) = parse_volume_id($target_volid); + + my $scfg = storage_config($cfg, $storeid); + my $target_scfg = storage_config($cfg, $target_storeid); + + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); + + return $plugin->copy_image_start( + $scfg, $storeid, $volname, $target_scfg, $target_storeid, $target_volname, $snap, + ); +} + +# Start several prepared copies, grouping them so a backend that can capture multiple +# volumes at one instant does so. $copies is an arrayref of +# { source => $volid, target => $target_volid }. +# +# Copies are grouped by their (source storage, target storage) pair, since that is the +# granularity at which a backend can act atomically; each group is handed to the +# plugin's copy_images_start(), whose default simply starts them one by one. +# +# Same constraints as vdisk_copy_start(): cheap, no locking -- the caller may hold a +# guest freeze across this. +sub vdisk_copy_start_group { + my ($cfg, $copies) = @_; + + return if !$copies || !scalar(@$copies); + + my $groups = {}; + for my $copy (@$copies) { + my ($storeid, $volname) = parse_volume_id($copy->{source}); + my ($target_storeid, $target_volname) = parse_volume_id($copy->{target}); + + my $key = "$storeid/$target_storeid"; + $groups->{$key} //= { + storeid => $storeid, + target_storeid => $target_storeid, + copies => [], + }; + push $groups->{$key}->{copies}->@*, + { source => $volname, target => $target_volname, snap => $copy->{snap} }; + push $groups->{$key}->{orig}->@*, $copy; + } + + # Mark each group's copies started as soon as THAT group returns, not after the + # whole batch. A later group can fail, and the caller must still know which copies + # are actually running -- otherwise cleanup cannot tell a started copy from one that + # never began, and would either leak a running copy or wait for one that will never + # report. + for my $key (sort keys %$groups) { + my $group = $groups->{$key}; + + my $scfg = storage_config($cfg, $group->{storeid}); + my $target_scfg = storage_config($cfg, $group->{target_storeid}); + + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); + + $plugin->copy_images_start( + $scfg, $group->{storeid}, + $target_scfg, $group->{target_storeid}, + $group->{copies}, + ); + + $_->{started} = 1 for $group->{orig}->@*; + } + + return; +} + +# Wait for a started copy to become independent of its source. Frees the target and +# re-raises on failure, so the caller never leaks a half-copied volume. +# +# $progress is an optional coderef called with a percentage. +sub vdisk_copy_wait { + my ($cfg, $target_volid, $volid, $progress) = @_; + + my ($target_storeid, $target_volname) = parse_volume_id($target_volid); + + my $target_scfg = storage_config($cfg, $target_storeid); + my $plugin = PVE::Storage::Plugin->lookup($target_scfg->{type}); + + # What the copy was made from, so the plugin can clean up state it left there. + my $source; + if ($volid) { + my ($storeid, $volname) = parse_volume_id($volid); + $source = { + scfg => storage_config($cfg, $storeid), + storeid => $storeid, + volname => $volname, + }; + } + + # A bounded wait, not a bare loop: a backend whose copy dies can otherwise report + # 'pending' forever and hang this worker with no error and no progress. The deadline + # is generous because the copy is a full data copy, and it is reset whenever the + # backend reports forward progress, so a slow-but-live copy is never killed. + my $timeout = $target_scfg->{'copy-offload-timeout'} // (24 * 3600); + my $deadline = time() + $timeout; + my $last_progress; + + eval { + while (1) { + my $status = $plugin->copy_image_status( + $target_scfg, $target_storeid, $target_volname, $source, + ); + + my $state = $status->{state} // ''; + my $done = $status->{progress}; + + if (defined($done) && (!defined($last_progress) || $done != $last_progress)) { + $progress->($done) if $progress; + $last_progress = $done; + $deadline = time() + $timeout; # forward progress: give it the full budget again + } + + last if $state eq 'complete'; + + die "copy of '$target_volid' reported unexpected state '$state'\n" + if $state ne 'pending'; + + die "copy of '$target_volid' did not finish within ${timeout}s\n" + if time() >= $deadline; + + sleep(1); + } + }; + if (my $err = $@) { + # includes a worker abort (die from the poll loop): do not leak the target + eval { vdisk_free($cfg, $target_volid) }; + warn "could not clean up target volume '$target_volid' - $@" if $@; + die $err; + } + + return $target_volid; +} + +# Offload a full copy of $volid to $target_storeid, returning the new volid. +# +# The simple path, for a source nothing is writing to: prepare, start, wait. A caller +# cloning a RUNNING guest must not use this -- it has to prepare every disk, then start +# them all inside one freeze, then wait -- see the three calls above. +# +# The caller is responsible for checking copy_offload_class() first and, for the 'bulk' +# class with a running guest, for the write-tracking/convergence the class implies. +sub vdisk_copy { + my ($cfg, $volid, $target_storeid, $target_vmid, $snap, $opts, $progress) = @_; + + my $target_volid = + vdisk_copy_prepare($cfg, $volid, $target_storeid, $target_vmid, $snap, $opts); + + eval { vdisk_copy_start($cfg, $volid, $target_volid, $snap) }; + if (my $err = $@) { + eval { vdisk_free($cfg, $target_volid) }; + warn "could not clean up target volume '$target_volid' - $@" if $@; + die $err; + } + + return vdisk_copy_wait($cfg, $target_volid, $volid, $progress); +} + sub vdisk_create_base { my ($cfg, $volid) = @_; diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 4f69f9b..18a484f 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -197,6 +197,26 @@ my $defaultData = { type => 'boolean', optional => 1, }, + 'copy-offload-timeout' => { + description => + "Maximum time in seconds to wait for an offloaded copy to become " + . "independent of its source. The timer restarts whenever the backend " + . "reports progress, so this bounds a stalled copy rather than a slow one.", + type => 'integer', + minimum => 1, + optional => 1, + default => 24 * 3600, + }, + 'copy-offload' => { + description => + "Offload full-copy operations (e.g. full clone) to the storage backend " + . "instead of copying the data through the host. Only honored by plugins that " + . "advertise a 'copy-offload-atomic' or 'copy-offload-bulk' feature; ignored " + . "otherwise. Both source and target must be instances of the same plugin type.", + type => 'boolean', + optional => 1, + default => 0, + }, subdir => { description => "Subdir to mount.", type => 'string', @@ -1046,6 +1066,150 @@ sub clone_image { return $newvol; } +# START a full copy of $volname onto the TARGET storage, returning the new volname +# there. The copy is ASYNCHRONOUS: this returns as soon as the backend has accepted the +# job, and the caller then polls copy_image_status() until the copy is complete. +# +# It must be async because a full array copy runs for minutes to hours, while the core +# calls this under cluster_lock_storage(). For shared storage that is a pmxcfs lock, +# whose locked code is killed by an alarm(60) (PVE::Cluster) -- so a blocking copy would +# be aborted mid-flight and the lock broken. Splitting start from wait also mirrors what +# the host-side path already does: it allocates under the lock and runs the long +# qemu-img convert outside it. +# +# Returning the target volname immediately (rather than only when the data is there) is +# deliberate: the caller can record it for rollback before anything can go wrong, and +# free_image() is the cleanup path if the copy later fails or is aborted. A plugin MUST +# therefore accept free_image() on a volume whose copy is still in flight, cancelling +# the backend job. +# +# Only called when the plugin advertises 'copy-offload-atomic' or 'copy-offload-bulk' +# via volume_has_feature() and the target storage has copy-offload enabled, so the base +# implementation dies rather than returning undef: reaching it means a plugin advertised +# a capability it does not implement. +# +# $target_scfg/$target_storeid describe the TARGET storage. A plugin only ever receives +# its own per-instance scfg, so it cannot judge whether a given source/target pair can +# actually be copied by the backend (same array, same pool group, ...) without them. +# Both are guaranteed to be instances of the same plugin type as the source; cross-type +# offload is out of scope, since the operation is opaque to PVE and cannot be lowered to +# volume_export/volume_import the way a generic copy can. +# +# $snap is optional: copy from that snapshot rather than the current state. +# $opts->{format} is the format the caller resolved for the target. A plugin that cannot +# produce it MUST die here rather than silently producing something else. +# +# The copy is split into prepare + start so that a caller cloning a RUNNING guest can +# hold all the disks of a VM at one point in time: it prepares every target first, then +# fs-freezes the guest once and calls copy_image_start() for each disk inside that +# freeze. Consistency across disks is a VM-level property, so the core needs a cheap +# operation it can put inside a freeze -- which allocation is not. +sub copy_image_prepare { + my ( + $class, $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_vmid, $snap, $opts, + ) = @_; + + die "storage plugin '" . $class->type() . "' does not implement copy_image_prepare\n"; +} + +# Fix the source's point-in-time and begin copying it into the target that +# copy_image_prepare() returned. Only after this returns is the copy's content defined. +# +# MUST BE FAST AND MUST NOT BLOCK: the caller may hold a guest filesystem freeze (or a +# suspended VM) across this call for every disk of a VM. Do the minimum that pins the +# source -- create the snapshot/pair -- and let the data movement proceed in the +# background, reported by copy_image_status(). +# +# For the 'copy-offload-atomic' class this call is what defines the copy's point in +# time; writes to the source after it returns MUST NOT affect the result. A plugin that +# cannot make that guarantee must advertise 'copy-offload-bulk' instead. +# +# $snap names the source snapshot to copy from, or is undef for the current state. It +# is passed HERE and not only to prepare because this call is what fixes the point in +# time: a plugin that took its own snapshot of the current image here would silently +# copy live data when the caller asked for a snapshot. +sub copy_image_start { + my ( + $class, $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_volname, $snap, + ) = @_; + + die "storage plugin '" . $class->type() . "' does not implement copy_image_start\n"; +} + +# Start several copies as ONE operation. $copies is an arrayref of +# { source => $volname, target => $target_volname, snap => $snap }, all between the +# same pair of storages. As in copy_image_start(), 'snap' names the source snapshot to +# copy from and may be undef; each copy in a batch carries its own. +# +# The default implementation just starts them one after another, which is what the +# caller would otherwise do itself -- so there is nothing to advertise and nothing to +# negotiate. A backend that can capture several volumes at one instant (an array +# consistency group) overrides this, and every caller gets the stronger guarantee for +# free. +# +# Overriding it buys two things for a multi-disk guest. The capture becomes atomic in +# the backend rather than merely bracketed by a guest freeze, so the disks are mutually +# crash-consistent even if the freeze does not apply -- e.g. no guest agent is +# available. And the freeze, when there is one, shrinks from N sequential backend calls +# to one, which for a guest with many disks is the difference between a perceptible +# stall and none. +# +# The plugin decides how to group. It may start the batch as one backend group, as +# several, or one by one -- whatever its backend supports and its own metadata calls +# for, e.g. volumes that must stay in separate groups because they live in different +# pools, or because the backend caps how many volumes a group may hold. The core does +# not model any of that and passes the whole batch for a given pair of storages. +# +# What the core does guarantee is that every start in the batch happens inside one +# guest freeze. So the consistency a caller gets is: mutually crash-consistent WITHIN +# each backend group by the backend itself, and consistent ACROSS groups only by virtue +# of that freeze. A plugin that splits a batch it could have kept in one group is +# therefore trading away the guarantee that survives when no freeze is possible -- when +# there is no guest agent and the caller has to fall back to suspending the VM. +# +# Same contract as copy_image_start(): fast, non-blocking, and for the atomic class it +# is this call that fixes the point in time of every copy in the group. +sub copy_images_start { + my ($class, $scfg, $storeid, $target_scfg, $target_storeid, $copies) = @_; + + for my $copy (@$copies) { + $class->copy_image_start( + $scfg, $storeid, $copy->{source}, + $target_scfg, $target_storeid, $copy->{target}, $copy->{snap}, + ); + } + + return; +} + +# Poll a copy started by copy_image(). Returns a hashref: +# +# { state => 'pending'|'complete', progress => $percent } +# +# 'progress' is optional and informational only. On failure this MUST die; the caller +# then frees the target volume. +# +# 'complete' means the target is INDEPENDENT of the source -- not merely that the data +# is readable. For backends where independence is asynchronous (an RBD flatten, a ZFS +# clone still tied to its origin, an array pair that has not dissolved yet) reporting +# 'complete' early produces something that is not a full copy, and the caller is then +# free to delete the source. +# +# This is called OUTSIDE the storage lock, so it may talk to the backend, but it should +# stay cheap: it runs in a poll loop. +# +# $source is { scfg, storeid, volname } describing what the copy was made FROM. A +# plugin may need it to clean up state it left on the source -- an RBD copy, for +# instance, is a clone of a snapshot taken on the source image, and once the copy is +# independent nothing else records where it came from. +sub copy_image_status { + my ($class, $scfg, $storeid, $volname, $source) = @_; + + die "storage plugin '" . $class->type() . "' does not implement copy_image_status\n"; +} + sub alloc_image { my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_; @@ -1572,6 +1736,18 @@ sub storage_can_replicate { return 0; } +# Besides the features listed below, a plugin may advertise one of the two copy-offload +# classes (see copy_image()). They are separate features rather than one feature with a +# class argument so that this interface stays unchanged; a plugin supports whichever one +# fits its backend, and a caller that can use both prefers 'copy-offload-atomic': +# +# copy-offload-atomic The backend can produce the copy from a static point-in-time +# source (snapshot/clone), so no host-side mirror is needed to +# keep it consistent while the source keeps changing. +# copy-offload-bulk The copy is a bulk data movement that smears over a changing +# source (e.g. NFS 4.2 server-side copy, zfs send/recv). For a +# running guest the caller must track writes and converge, the +# way live migration does. sub volume_has_feature { my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running, $opts) = @_; diff --git a/src/test/copy_offload_test.pm b/src/test/copy_offload_test.pm new file mode 100644 index 0000000..98aa940 --- /dev/null +++ b/src/test/copy_offload_test.pm @@ -0,0 +1,90 @@ +package PVE::Storage::TestCopyOffload; + +use strict; +use warnings; + +use lib qw(..); + +use PVE::Storage; +use Test::More; + +# copy_offload_class() decides whether a full copy may be handed to the backend, and +# which of the two classes to use. Getting it wrong either silently declines an offload +# or drives one onto a storage pair that cannot perform it, so the rules are pinned here. +# +# The checks are: the TARGET storage must have copy-offload enabled, both storages must +# be instances of the same plugin type, and the plugin must advertise a class for that +# volume. 'atomic' wins when a plugin advertises both, as it needs no host-side mirror. + +my $features = {}; # volid => { feature => 1 } + +{ + no warnings 'redefine'; + *PVE::Storage::volume_has_feature = sub { + my ($cfg, $feature, $volid, $snap, $running, $opts) = @_; + return $features->{$volid}->{$feature} ? 1 : 0; + }; +} + +my $cfg = { + ids => { + 'rbd-a' => { type => 'rbd', 'copy-offload' => 1 }, + 'rbd-b' => { type => 'rbd', 'copy-offload' => 1 }, + 'rbd-off' => { type => 'rbd' }, + 'dir-a' => { type => 'dir', 'copy-offload' => 1 }, + }, +}; + +my $volid = 'rbd-a:vm-100-disk-0'; + +my $tests = [ + # [ description, target storeid, advertised features, expected class ] + ['nothing advertised => no offload', 'rbd-b', {}, undef], + ['atomic advertised', 'rbd-b', { 'copy-offload-atomic' => 1 }, 'atomic'], + ['bulk advertised', 'rbd-b', { 'copy-offload-bulk' => 1 }, 'bulk'], + [ + 'atomic preferred when both are advertised', + 'rbd-b', + { 'copy-offload-atomic' => 1, 'copy-offload-bulk' => 1 }, + 'atomic', + ], + [ + 'target has copy-offload disabled', + 'rbd-off', + { 'copy-offload-atomic' => 1 }, + undef, + ], + [ + 'cross-type is out of scope even when advertised', + 'dir-a', + { 'copy-offload-atomic' => 1 }, + undef, + ], + [ + 'same storage as source is allowed', + 'rbd-a', + { 'copy-offload-atomic' => 1 }, + 'atomic', + ], +]; + +plan tests => scalar(@$tests) + 1; + +for my $t (@$tests) { + my ($desc, $target, $adv, $expected) = @$t; + + $features = { $volid => $adv }; + + my $got = PVE::Storage::copy_offload_class($cfg, $volid, $target, undef, 0); + is($got, $expected, $desc); +} + +# A path that is not a storage volume must not be mistaken for an offloadable one. +$features = { $volid => { 'copy-offload-atomic' => 1 } }; +is( + PVE::Storage::copy_offload_class($cfg, '/dev/sdb', 'rbd-b', undef, 0), + undef, + 'a bare path is never offloadable', +); + +done_testing(); diff --git a/src/test/run_plugin_tests.pl b/src/test/run_plugin_tests.pl index 8bce9d3..dda4cac 100755 --- a/src/test/run_plugin_tests.pl +++ b/src/test/run_plugin_tests.pl @@ -17,6 +17,7 @@ my $res = $harness->runtests( "get_subdir_test.pm", "filesystem_path_test.pm", "prune_backups_test.pm", + "copy_offload_test.pm", ); exit -1 if !$res || $res->{failed} || $res->{parse_errors}; -- 2.54.0