From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id CD5171FF130 for ; Mon, 20 Jul 2026 16:31:14 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id BCD7E2157A; Mon, 20 Jul 2026 16:31:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784557817; x=1785162617; 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=LBEkVH+l5F9+3eNOVnmgiaae+BisbkIA0UIZRXN9DJU=; b=YgjlKM86adcwhHr1H2nDg/e4QRHaioK5FN2y3m1/aA7+efqqaEgXYil1r9Nqz15Sw/ mGemvuOi6cNks2bSQGyuFOdNSdF/+rDBpbIhAWD/SkjH93zy+46nxeC0y8BxhNImjgM4 00jIJqTrvl4DPFD2L1i8Sa0jIIsyYxm9aKmW13vtJ665qQyOat9Brjaka4WdfLqErtkX 0waJcTgdB0SwutEny6lB7XHzMb/6A83jDH/J/7Ixuf3bG/xZV+ytAVyllyi+U1SsVkke n7kD6HLnzd2VXre0lolrF/ivAFdjZDufsPF0oHuTMLH0mSk6UusA9GC87ndrrp1jrH02 Wtew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784557817; x=1785162617; 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=LBEkVH+l5F9+3eNOVnmgiaae+BisbkIA0UIZRXN9DJU=; b=ikM98FZhUT1v697qrkWvHLUtsNuBywZN4KfsIzmHMBkNlOhIY3YwhlEKPK+QRaansy 1mDs8iVjyZ3P6Sab0aoLlpqRNLqyoSLs081Y2qU3bK4z2A9+sSSm7JoVOeEeNZ7FHKvg +ZgHC3Luiksh8YkayG0tnuiAAhxGDZAScT72s+BAkKhYZI+wgQ7IHVR6gHlfuXWUWeTC Io9cMxWK4WwSFUyvr9FgleLFdRGBm160OMecv1MiMHriZIS52gy37FQuDTrNIOwOe4m0 4jy888SyV66AEgksy1FY/0CUHZ1GeQGACs+36cHlLoVzU2OMANuYX+vHempIOmNk0Rxz 5jpA== X-Gm-Message-State: AOJu0YwUS29T5mkAXTOCzH1vzdzI1EULfW/9WAQcoY8RFPCLpOUCUtfu /jcC04Zo75X0aG4dqeyTDWny8dt5QTBbU138TplmWRN63WMGiKohY8B6AYA6P9Vy X-Gm-Gg: AfdE7ckR4/Lq/rCEZq0CdZU75dGVa/YcLK5ErE1l+Tq4l7R0RsRJRuh23d2/FnRDAYk HbtpFD6D7InauF2P390b9KjaXdcvVda8z+ThGxhZb72nvs1GnKaeuVtulYSs93TumPWZ2eFriqm dBIvYqS3MIPbdCFOOFqerUdRazWE7P1WWTxwbEIB521Iwz5M3W0/Jy+Vc4HNgBRZoR+EF2TfHZ8 1iTra0WpfWgFZtc9F/KTzfxq+zj3ALejsLc7yiui1aqwEF6zv7nw36xiKIn8nh/tN+76Ly/6CJN 0P/5pXz0vlRhD98fKgmJq8Uo54zPcg6f3FrOiX8ceQCkOEFkd0bOVP9u8a45HYDr7jjzPog6Al5 +WZ4wMT8Gp9jVRi4ZQrQ2Iv4jI/SrM09hpC/0gtWLdiTpFjTQAbKIVAAsobuao18uRl/SWqg+NU zPcaZoSRk= X-Received: by 2002:a05:6a21:1506:b0:3bf:a0e5:99a0 with SMTP id adf61e73a8af0-3c3ad95b5fcmr15793538637.47.1784557815895; Mon, 20 Jul 2026 07:30:15 -0700 (PDT) Date: Mon, 20 Jul 2026 07:30:15 -0700 (PDT) From: Ciro Iriarte To: pve-devel@lists.proxmox.com Subject: [RFC PATCH storage 4/5] btrfs, lvmthin: implement copy-offload (atomic class) Message-ID: <20260720.4.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.343 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_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks 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: HKPHNVBPQAAFW2LUESZVU2K7PRW7RMFR X-Message-ID-Hash: HKPHNVBPQAAFW2LUESZVU2K7PRW7RMFR 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: Both backends already have the right primitive and PVE already uses it for linked clones -- 'btrfs subvolume snapshot' and 'lvcreate -s' on a thin LV. Both are instant, allocate nothing, and are independent of their source immediately, so they satisfy 'copy-offload-atomic' with no background work at all: copy_image_status() is complete on the first poll and the asynchronous machinery never engages. Independence is the part worth being precise about, since it is what separates these from a ZFS clone. Neither pins its source. btrfs reference counts extents and has no notion of an origin that must outlive its snapshots; the thin pool reference counts blocks, which is exactly the property already noted at the top of LvmThinPlugin.pm ("LVM thin allows deletion of such base volumes without affecting the linked clones"). So the copy can be handed back with no recorded parent, because it genuinely has none. Scope, and why: - btrfs handles 'raw' and 'subvol', the formats it stores inside subvolumes. qcow2 and vmdk are plain files there and would need the reflink path instead, so they are not advertised rather than failing later in prepare. - lvmthin requires the same VG and the same thin pool. A thin snapshot shares blocks within one pool and cannot leave it; copying elsewhere is a real data move. The target name is reserved by creating it in prepare(). start() then parks that reservation aside rather than deleting it, since neither primitive can write into a name that already exists. Details that are easy to get wrong, and were: - For btrfs 'raw' the reservation must include the disk.raw file, not just the subvolume. list_images() stats that file and skips the entry when it is missing, so a bare subvolume is invisible to find_free_diskname(). That does not merely weaken the reservation: the core prepares every disk of a VM before starting any of them, so the second prepare() for the same target picks the same name and dies on 'subvolume create'. Every multi-disk offloaded clone to btrfs raw would have failed. - The lvmthin placeholder is parked under a 'copytmp-' PREFIX. As a suffix, 'vm-101-disk-0.copytmp' satisfies both list_images()' filter and parse_volname(), so it appeared as a real disk belonging to VM 101 -- a phantom the GUI offers to attach. The prefix stays outside both patterns while still holding the disk NUMBER, because LVM's find_free_diskname() reads the raw LV list and matches unanchored. - btrfs takes its snapshot under a staging name FIRST and only then swaps, so the reserved name stays held for the duration of the copy rather than just after it. - The same-filesystem check cannot use st_dev or statfs f_fsid. btrfs gives every subvolume its own st_dev and folds the subvolume id into f_fsid, so both differ between two subvolumes of ONE filesystem and rejected every legitimate copy; 'stat -c %m' is fooled the same way. It resolves the containing mount and compares the filesystem UUID, which also permits one btrfs mounted at several points via 'subvol=', a normal PVE arrangement. btrfs swaps the finished copy into the reserved name with renameat2(RENAME_EXCHANGE) rather than a park-then-rename pair, so the name is occupied at every instant -- by either the placeholder or the copy. This matters because the reservation is NOT otherwise held for btrfs: list_images()' name pattern rejects the '.copytmp' suffix, so unlike lvmthin -- where find_free_diskname() reads the raw LV list and still sees the parked LV -- a parked subvolume reserves nothing, and a concurrent allocation could take the name and have the caller's rollback free somebody else's volume. The exchange removes that window instead of narrowing it, needs no contract change, and uses the call this plugin already makes to rotate a rollback target into place. Neither start() removes its placeholder, and lvmthin's start() no longer sets the autoactivation flag either. That call can run with a guest filesystem frozen, and both are housekeeping nothing depends on -- every LVM command takes VG metadata locks that can queue behind other activity, and for a multi-disk VM those add up inside one freeze. copy_image_status() does them, outside. It is best effort throughout, including its existence probe: dying there would make the caller free a copy that is already complete and correct. prepare() and free_image() both reap a placeholder left by a copy that died in between. Without that the leftover is not just litter -- parking refuses to write onto an existing name, so every later copy to that name fails, and the leftover is invisible to list_images() so nothing would explain why. Verified by copy_offload_functional_test.pl, added here: it builds a loop-backed btrfs and a loop-backed thin pool, drives the hooks, and compares images byte for byte -- 21 assertions, all passing. It covers a copy from a snapshot returning the SNAPSHOT's content rather than live data, the copies staying identical after the source is deleted outright, two prepares for one target VM returning different names, and a leaked placeholder not wedging the disk name, and the reserved name still being held between start() and status() -- the moment a park-then-rename would lose it. It needs root and real loop devices, so it is not part of run_plugin_tests.pl and skips cleanly when it cannot run. The unit tests pin what each plugin advertises, since advertising the wrong volume fails a clone while failing to advertise a supported one silently falls back to a byte copy. Note that RBDPlugin in this series still has the shape this replaces -- it removes the placeholder and then renames -- so the same window exists there. Fixing it properly needs identity-checked rollback (prepare returning a token the rollback verifies), which is a hook-contract change and is being raised with the maintainers rather than decided here. Generated-By: Claude (https://claude.ai) Signed-off-by: Ciro Iriarte Co-Authored-By: Claude --- src/PVE/Storage/BTRFSPlugin.pm | 250 ++++++++++++++++++++ src/PVE/Storage/LvmThinPlugin.pm | 189 +++++++++++++++ src/test/copy_offload_feature_test.pm | 98 ++++++++ src/test/copy_offload_functional_test.pl | 286 +++++++++++++++++++++++ src/test/run_plugin_tests.pl | 1 + 5 files changed, 824 insertions(+) create mode 100644 src/test/copy_offload_feature_test.pm create mode 100644 src/test/copy_offload_functional_test.pl diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm index fb47aa0..7e2b2d1 100644 --- a/src/PVE/Storage/BTRFSPlugin.pm +++ b/src/PVE/Storage/BTRFSPlugin.pm @@ -78,6 +78,8 @@ sub options { 'create-base-path' => { optional => 1 }, 'create-subdirs' => { optional => 1 }, preallocation => { optional => 1 }, + 'copy-offload' => { optional => 1 }, + 'copy-offload-timeout' => { optional => 1 }, # TODO: The new variant of mkdir with `populate` vs `create`... }; } @@ -625,6 +627,16 @@ sub volume_has_feature { rename => { current => { qcow2 => 1, raw => 1, vmdk => 1 }, }, + # 'btrfs subvolume snapshot' is instant, shares extents copy-on-write, and is + # independent of its source straight away -- btrfs reference counts extents and + # does not pin an origin the way ZFS does. Only the formats this plugin stores + # as subvolumes qualify; qcow2 and vmdk are plain files here and would need the + # reflink path instead. + 'copy-offload-atomic' => { + base => { raw => 1, subvol => 1 }, + current => { raw => 1, subvol => 1 }, + snap => { raw => 1, subvol => 1 }, + }, }; my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) = @@ -1002,4 +1014,242 @@ sub get_import_metadata { return PVE::Storage::DirPlugin::get_import_metadata(@_); } +# ---- storage-offloaded full copy via subvolume snapshot --------------------------- +# +# 'btrfs subvolume snapshot' is instant and shares extents copy-on-write. btrfs +# reference counts those extents and has no concept of an origin that must outlive its +# snapshots, so the copy is independent immediately -- deleting the source is fine. +# That satisfies 'copy-offload-atomic' with no background work, so copy_image_status() +# is complete on the first poll. +# +# This is the same primitive clone_image() uses. The difference is what PVE records +# afterwards: a linked clone carries a dependency it has to respect, while here we can +# hand back a volume with no parent, because on btrfs there genuinely is none. +# +# Only 'raw' and 'subvol' are handled: those are the formats this plugin stores inside +# subvolumes. qcow2 and vmdk are plain files here, so they would need the reflink path +# and are not advertised. + +# Resolve a volume to the subvolume that backs it, honouring $snapname. +my sub volume_subvol { + my ($class, $scfg, $volname, $snapname) = @_; + + my (undef, undef, undef, undef, undef, undef, $format) = $class->parse_volname($volname); + my $path = $class->filesystem_path($scfg, $volname, $snapname); + + return $format eq 'raw' ? raw_file_to_subvol($path) : $path; +} + +# btrfs cannot snapshot across filesystems, and the target may be a different storage. +# +# Deliberately NOT st_dev or statfs f_fsid: btrfs gives every subvolume its own st_dev, +# and folds the subvolume id into f_fsid too, so both differ between two subvolumes of +# the ONE filesystem and would reject every legitimate copy. The filesystem UUID is the +# only stable identity. This costs a command, but it runs in prepare(), never in the +# freeze-sensitive start(). +my sub btrfs_fsid { + my ($path) = @_; + + # 'btrfs filesystem show' takes a device or a mount point, not an arbitrary path, + # so resolve the containing mount first. Note this cannot be done with stat(): + # coreutils' own %m reports a btrfs subvolume as its own mount point, for the same + # st_dev reason. + my $mnt; + eval { + run_command( + ['findmnt', '--noheadings', '--output', 'TARGET', '--target', $path], + outfunc => sub { $mnt //= $_[0] if $_[0] =~ /\S/ }, + ); + }; + return (undef, " - $@") if $@; + return (undef, " - findmnt reported no mount point") if !defined($mnt); + chomp $mnt; + + # Going by UUID rather than by mount point on purpose: one btrfs filesystem can be + # mounted at several places (a 'subvol=' mount per storage is a normal PVE setup), + # and a snapshot between them is perfectly valid. + my $uuid; + my $stderr = ''; + eval { + run_command( + ['btrfs', 'filesystem', 'show', '--', $mnt], + outfunc => sub { + my ($line) = @_; + $uuid = $1 if !defined($uuid) && $line =~ m/\buuid:\s*(\S+)/i; + }, + # A non-btrfs target is an ordinary "cannot offload this" answer, not a + # fault worth printing to the task log. The reason is returned to the + # caller instead, so it still ends up in the error it raises. + errfunc => sub { $stderr .= $_[0] }, + ); + }; + return (undef, " - $@") if $@; + return (undef, $stderr =~ /\S/ ? " - $stderr" : " - no uuid in 'btrfs filesystem show'") + if !defined($uuid); + return ($uuid, undef); +} + +sub copy_image_prepare { + my ( + $class, $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_vmid, $snap, $opts, + ) = @_; + + my ($vtype, undef, undef, undef, undef, undef, $format) = $class->parse_volname($volname); + die "copy offload only handles VM images, not '$vtype'\n" if $vtype ne 'images'; + die "btrfs copy offload cannot handle format '$format'\n" + if $format ne 'raw' && $format ne 'subvol'; + + # A subvolume snapshot copies the subvolume as it is; it cannot convert formats. + my $target_format = $opts->{format} // $format; + die "btrfs copy offload cannot convert '$format' to '$target_format'\n" + if $target_format ne $format; + + my $subvol = volume_subvol($class, $scfg, $volname, $snap); + die "cannot copy '$volname': '$subvol' does not exist\n" if !-e $subvol; + + my $imagedir = $class->get_subdir($target_scfg, 'images') . "/$target_vmid"; + mkpath $imagedir; + + # Keep "could not work out which filesystem this is" distinct from "they are two + # different filesystems". Reporting a mismatch when findmnt or btrfs is missing, or + # when the path is not on btrfs at all, sends people looking in the wrong place. + # findmnt needs the directory to exist, hence the mkpath above -- so tidy it away + # again on the paths that reject, rather than littering the target storage with an + # empty directory per refused attempt. + my $reject = sub { + rmdir($imagedir); # only succeeds while empty, which is what we want + die $_[0]; + }; + my ($src_fsid, $src_err) = btrfs_fsid($subvol); + $reject->("cannot determine the btrfs filesystem of '$subvol'$src_err\n") + if !defined($src_fsid); + my ($dst_fsid, $dst_err) = btrfs_fsid($imagedir); + $reject->("cannot determine the btrfs filesystem of '$imagedir'$dst_err\n") + if !defined($dst_fsid); + $reject->("copy offload requires source and target on the same btrfs filesystem\n") + if $src_fsid ne $dst_fsid; + + # the trailing 1 adds the format suffix; without it the volname does not parse + my $name = + $class->find_free_diskname($target_storeid, $target_scfg, $target_vmid, $format, 1); + my $target_volname = "$target_vmid/$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. + # An empty subvolume costs nothing and is what free_image() already knows how to + # remove. + my $newsubvol = volume_subvol($class, $target_scfg, $target_volname, undef); + + # Reap a placeholder left by an earlier copy that died between start() and + # status(). It is not merely litter: the rename in start() refuses to park onto an + # existing path, so without this every future offloaded copy to this name fails -- + # and the leftover is invisible to list_images(), so nobody would know why. Safe + # here: we hold the target storage lock, we are outside any freeze, and the name is + # ours because find_free_diskname() just handed it out. + my $stale = "$newsubvol.copytmp"; + if (-e $stale) { + warn "removing stale copy placeholder '$stale'\n"; + $class->btrfs_cmd(['subvolume', 'delete', '--', $stale]); + } + + $class->btrfs_cmd(['subvolume', 'create', '--', $newsubvol]); + + # For 'raw' the volume is the disk.raw INSIDE the subvolume, and that file is what + # list_images() stats. Without it file_size_info() returns undef and the entry is + # skipped, which would make this reservation invisible to find_free_diskname() -- + # defeating the point of creating it, and worse: the core prepares every disk of a + # VM before starting any of them, so the second prepare() for the same target would + # pick this same name and die on 'subvolume create'. Every multi-disk offloaded + # clone would fail. Create the file too, exactly as alloc_image does. + if ($format eq 'raw') { + my $raw = "$newsubvol/disk.raw"; + my $fh; + if (!sysopen($fh, $raw, O_WRONLY | O_CREAT | O_EXCL, 0640)) { + my $err = $!; + eval { $class->btrfs_cmd(['subvolume', 'delete', '--', $newsubvol]); }; + warn $@ if $@; + die "unable to reserve '$raw' - $err\n"; + } + close($fh); + } + + return $target_volname; +} + +sub copy_image_start { + my ( + $class, $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_volname, $snap, + ) = @_; + + # Snapshot the source the caller asked for. Falling back to the current subvolume + # when a snapshot was requested would silently copy live data instead. + my $subvol = volume_subvol($class, $scfg, $volname, $snap); + my $newsubvol = volume_subvol($class, $target_scfg, $target_volname, undef); + + # copy_image_prepare() reserved the name with a placeholder subvolume, and + # 'subvolume snapshot' will not write into a path that already exists. So: snapshot + # to a staging name, then ATOMICALLY EXCHANGE staging and the reserved name. + # + # PVE::Tools::renameat2(RENAME_EXCHANGE) rather than a pair of rename(2)s. Parking + # the placeholder aside and moving the copy in afterwards leaves the reserved name + # unheld in between, and for btrfs that genuinely loses the reservation: unlike + # lvmthin -- where find_free_diskname() reads the raw LV list and still sees the + # parked LV -- btrfs goes through list_images(), whose name pattern rejects the + # '.copytmp' suffix. A concurrent allocation could take the name, and the caller's + # rollback would then free somebody else's volume. With the exchange the name is + # occupied at every instant, by either the placeholder or the finished copy, so + # that window does not exist. The placeholder simply ends up at the staging path. + my $staging = "$newsubvol.copytmp"; + + # The expensive part, entirely off to one side; the reserved name is untouched, so + # a failure here needs no unwinding beyond dropping the staging subvolume. + eval { $class->btrfs_cmd(['subvolume', 'snapshot', '--', $subvol, $staging]); }; + if (my $err = $@) { + eval { $class->btrfs_cmd(['subvolume', 'delete', '--', $staging]) if -e $staging; }; + warn $@ if $@; + die $err; + } + + # The paths are absolute, so pass -1 as the file descriptors -- same call this + # plugin already makes when rotating a rollback target into place. + if (!PVE::Tools::renameat2(-1, $staging, -1, $newsubvol, &PVE::Tools::RENAME_EXCHANGE)) { + my $rerr = $!; + # Nothing moved: the reserved name still holds the placeholder, so the caller's + # rollback frees exactly what it was given. + eval { $class->btrfs_cmd(['subvolume', 'delete', '--', $staging]); }; + warn $@ if $@; + die "unable to swap the copy into '$newsubvol' - $rerr\n"; + } + + # The displaced placeholder, now sitting at the staging path, is deliberately NOT + # removed here. This runs while the caller may hold a guest filesystem frozen, and + # removing it is pure cleanup nothing depends on -- copy_image_status() does it. + return; +} + +sub copy_image_status { + my ($class, $scfg, $storeid, $volname, $source) = @_; + + # $volname is the TARGET. The snapshot is complete and independent the moment btrfs + # returns, so there is nothing to poll; what is left is dropping the placeholder + # copy_image_start() parked, which is done here to keep it out of the freeze window. + my ($vtype, undef, undef, undef, undef, undef, $format) = + eval { $class->parse_volname($volname) }; + if (!$@ && defined($vtype) && $vtype eq 'images' && defined($format) + && ($format eq 'raw' || $format eq 'subvol')) + { + my $parked = volume_subvol($class, $scfg, $volname, undef) . '.copytmp'; + if (-e $parked) { + eval { $class->btrfs_cmd(['subvolume', 'delete', '--', $parked]); }; + warn $@ if $@; + } + } + + return { state => 'complete' }; +} + 1 diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm index cadf343..2f0fc05 100644 --- a/src/PVE/Storage/LvmThinPlugin.pm +++ b/src/PVE/Storage/LvmThinPlugin.pm @@ -54,6 +54,8 @@ sub options { disable => { optional => 1 }, content => { optional => 1 }, bwlimit => { optional => 1 }, + 'copy-offload' => { optional => 1 }, + 'copy-offload-timeout' => { optional => 1 }, }; } @@ -130,6 +132,18 @@ sub alloc_image { return $name; } +# Where copy_image_start() parks the reservation while it creates the snapshot. +# +# A PREFIX, not a suffix: list_images() selects on m/^(vm|base)-(\d+)-/ and +# parse_volname() accepts m/^((vm|base)-(\d+)-\S+)$/, so 'vm-101-disk-0.copytmp' would +# be a perfectly valid volume name and would show up as a disk belonging to VM 101 -- +# a phantom the GUI offers to attach as an unused disk, and one that outlives the copy +# if it ever leaks. Prefixing puts it outside both patterns. +my sub parked_name { + my ($volname) = @_; + return "copytmp-$volname"; +} + sub free_image { my ($class, $storeid, $scfg, $volname, $isBase) = @_; @@ -146,6 +160,13 @@ sub free_image { run_command($cmd, errmsg => "lvremove snapshot '$vg/$lv' error"); } + # a copy placeholder parked under this name, if a copy died mid-flight + my $parked = parked_name($volname); + if ($dat->{$parked}) { + my $cmd = ['/sbin/lvremove', '-f', "$vg/$parked"]; + run_command($cmd, errmsg => "lvremove copy placeholder '$vg/$parked' error"); + } + # finally remove original (if exists) if ($dat->{$volname}) { my $cmd = ['/sbin/lvremove', '-f', "$vg/$volname"]; @@ -415,6 +436,11 @@ sub volume_has_feature { copy => { base => 1, current => 1, snap => 1 }, sparseinit => { base => 1, current => 1 }, rename => { current => 1 }, + # A thin snapshot is instant, allocates nothing, and is independent of its + # origin straight away -- see the note at the top of this file: the origin can + # be deleted without affecting it. Snapshots of a snapshot work too, so all + # three keys apply. + 'copy-offload-atomic' => { base => 1, current => 1, snap => 1 }, }; my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) = $class->parse_volname($volname); @@ -507,4 +533,167 @@ sub rename_snapshot { die "rename_snapshot is not supported for $class"; } +# ---- storage-offloaded full copy via thin snapshot -------------------------------- +# +# 'lvcreate -s' on a thin LV is instant, allocates no data blocks, and -- unlike a ZFS +# clone -- does not pin its origin: the thin pool reference counts blocks, so the origin +# can be removed while the copy lives on. That is the note at the top of this file, and +# it is exactly what 'copy-offload-atomic' requires, so there is no background work and +# copy_image_status() is complete on the first poll. +# +# This is the same primitive clone_image() already uses for linked clones. The +# difference is only in what PVE believes afterwards: a linked clone records a +# dependency it must respect, while this path is free to hand back a volume with no +# recorded parent, because thin snapshots genuinely have none. + +my sub thin_lv_exists { + my ($vg, $lv) = @_; + my $lvs = PVE::Storage::LVMPlugin::lvm_list_volumes($vg); + return defined($lvs->{$vg}) && defined($lvs->{$vg}->{$lv}); +} + +sub copy_image_prepare { + my ( + $class, $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_vmid, $snap, $opts, + ) = @_; + + my $format = $opts->{format} // 'raw'; + die "lvmthin copy offload cannot produce format '$format'\n" if $format ne 'raw'; + + my ($vtype) = $class->parse_volname($volname); + die "copy offload only handles VM images, not '$vtype'\n" if $vtype ne 'images'; + + # A thin snapshot shares blocks with its origin inside one pool, so it cannot leave + # that pool. Copying to another VG or another thinpool is a real data move and has + # to take the normal host-side path. + my $vg = $scfg->{vgname}; + die "copy offload requires source and target in the same volume group\n" + if ($target_scfg->{vgname} // '') ne $vg; + die "copy offload requires source and target in the same thin pool\n" + if ($target_scfg->{thinpool} // '') ne ($scfg->{thinpool} // ''); + + my $src_lv = defined($snap) ? "snap_${volname}_$snap" : $volname; + die "cannot copy '$volname': source volume '$src_lv' does not exist\n" + if !thin_lv_exists($vg, $src_lv); + + my $name = $class->find_free_diskname($target_storeid, $target_scfg, $target_vmid); + + # Reap a placeholder left by an earlier copy that died between start() and + # status(). It is not merely litter: 'lvrename' below refuses to park onto an + # existing name, so without this every future offloaded copy to this name fails. + # Doing it here is safe -- we hold the target storage lock and are outside any + # freeze -- and the name is ours, since find_free_diskname() just handed it out. + my $stale = parked_name($name); + if (thin_lv_exists($vg, $stale)) { + warn "removing stale copy placeholder '$vg/$stale'\n"; + run_command( + ['/sbin/lvremove', '-f', "$vg/$stale"], + errmsg => "lvremove stale placeholder '$vg/$stale' error", + ); + } + + # 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. + # + # A thin LV is virtual, so this placeholder allocates no data blocks whatever size + # it claims; 1k is simply the smallest lvcreate accepts and rounds up. + my $cmd = [ + '/sbin/lvcreate', '-aly', '-V', '1k', '--name', $name, + '--thinpool', "$vg/$scfg->{thinpool}", + ]; + run_command($cmd, errmsg => "lvcreate placeholder '$vg/$name' error"); + $set_lv_autoactivation->($vg, $name, 0); + + return $name; +} + +sub copy_image_start { + my ( + $class, $scfg, $storeid, $volname, + $target_scfg, $target_storeid, $target_volname, $snap, + ) = @_; + + my $vg = $scfg->{vgname}; + + # Snapshot the source the caller asked for. Falling back to the current LV when a + # snapshot was requested would silently copy live data instead. + my $src_lv = defined($snap) ? "snap_${volname}_$snap" : $volname; + + # copy_image_prepare() reserved the name with a placeholder, and 'lvcreate -s' + # cannot write into a name that already exists. Rename the placeholder aside rather + # than removing it, so the reserved name is never momentarily free for a concurrent + # find_free_diskname() to hand out. + my $parked = parked_name($target_volname); + run_command( + ['/sbin/lvrename', $vg, $target_volname, $parked], + errmsg => "lvrename placeholder '$vg/$target_volname' error", + ); + + eval { + # ONLY the snapshot. It is what fixes the point in time; everything else this + # copy needs is done by copy_image_status(), outside the freeze. + my $cmd = ['/sbin/lvcreate', '-n', $target_volname, '-prw', '-kn', '-s', "$vg/$src_lv"]; + run_command($cmd, errmsg => "thin snapshot of '$vg/$src_lv' error"); + }; + if (my $err = $@) { + # Put the reservation back so the caller's rollback still finds the volume it + # was given, and leave the source untouched. If that fails too, remove the + # parked LV rather than leaving an orphan: the rollback frees $target_volname, + # which by then names nothing, so nothing else would ever reap it. + eval { + run_command( + ['/sbin/lvrename', $vg, $parked, $target_volname], + errmsg => "restoring placeholder '$vg/$target_volname' error", + ); + }; + if (my $rerr = $@) { + eval { run_command(['/sbin/lvremove', '-f', "$vg/$parked"]) }; + $err .= "additionally, could not restore the reserved name: $rerr"; + $err .= "and '$vg/$parked' is left behind\n" if $@; + } + die $err; + } + + # The parked placeholder is deliberately NOT removed here. This runs while the + # caller may hold a guest filesystem frozen, and every LVM command takes VG + # metadata locks that can queue behind other activity on the node -- for a + # multi-disk VM those add up inside a single freeze. Only the snapshot above fixes + # the point in time; removal is cleanup, and copy_image_status() does it outside. + return; +} + +sub copy_image_status { + my ($class, $scfg, $storeid, $volname, $source) = @_; + + # $volname is the TARGET. The thin snapshot is complete and independent the moment + # lvcreate returns, so there is nothing to poll and nothing of the source to + # release. What is left is dropping the placeholder copy_image_start() parked, + # which happens here to keep it out of the freeze window. + my $vg = $scfg->{vgname}; + + # Everything here is best effort ON PURPOSE, including the existence check. This + # runs in the caller's poll loop, and dying makes it free a copy that is already + # complete, correct and independent -- losing real data because a cleanup probe hit + # a transient LVM lock, which is exactly the contention this plugin already works + # around elsewhere. Deferring the autoactivation flag to here for the same reason it + # is not done in start(): it is metadata housekeeping that takes the same VG lock. + eval { + my $parked = parked_name($volname); + if (thin_lv_exists($vg, $parked)) { + run_command( + ['/sbin/lvremove', '-f', "$vg/$parked"], + errmsg => "lvremove placeholder '$vg/$parked' error", + ); + } + }; + warn $@ if $@; + + $set_lv_autoactivation->($vg, $volname, 0); + + return { state => 'complete' }; +} + 1; diff --git a/src/test/copy_offload_feature_test.pm b/src/test/copy_offload_feature_test.pm new file mode 100644 index 0000000..76fcecd --- /dev/null +++ b/src/test/copy_offload_feature_test.pm @@ -0,0 +1,98 @@ +package PVE::Storage::TestCopyOffloadFeature; + +use strict; +use warnings; + +use lib qw(..); + +# PVE::Storage first: it registers the plugins in an order that resolves the +# DirPlugin <-> Storage.pm circular load. Requiring BTRFSPlugin on its own fails. +use PVE::Storage; +use PVE::Storage::BTRFSPlugin; +use PVE::Storage::LvmThinPlugin; +use Test::More; + +# Which volumes a plugin advertises 'copy-offload-atomic' for. This is the gate the +# whole path hangs off: advertising a volume the plugin cannot actually copy sends the +# clone into copy_image_prepare() only to die there, and NOT advertising one it can +# copy silently falls back to a full host-side byte copy. Neither failure is visible +# from a passing clone, so the answers are pinned here. + +my $btrfs_scfg = { path => '/some/btrfs', type => 'btrfs' }; +my $lvmthin_scfg = { vgname => 'vg0', thinpool => 'tp', type => 'lvmthin' }; + +my $tests = [ + # [ description, class, scfg, volname, snapname, expected ] + + # btrfs stores raw and subvol as subvolumes, which is what it can snapshot. + [ + 'btrfs raw is advertised', + 'PVE::Storage::BTRFSPlugin', $btrfs_scfg, '100/vm-100-disk-0.raw', undef, 1, + ], + [ + 'btrfs subvol is advertised', + 'PVE::Storage::BTRFSPlugin', $btrfs_scfg, '100/subvol-100-disk-0.subvol', undef, 1, + ], + [ + 'btrfs raw is advertised from a snapshot too', + 'PVE::Storage::BTRFSPlugin', $btrfs_scfg, '100/vm-100-disk-0.raw', 'snap1', 1, + ], + # qcow2 and vmdk are plain files here, not subvolumes, so the subvolume-snapshot + # path does not apply to them. + [ + 'btrfs qcow2 is NOT advertised', + 'PVE::Storage::BTRFSPlugin', $btrfs_scfg, '100/vm-100-disk-0.qcow2', undef, undef, + ], + [ + 'btrfs vmdk is NOT advertised', + 'PVE::Storage::BTRFSPlugin', $btrfs_scfg, '100/vm-100-disk-0.vmdk', undef, undef, + ], + + # lvmthin is raw-only, and a thin snapshot does not pin its origin, so every key + # qualifies. + [ + 'lvmthin current is advertised', + 'PVE::Storage::LvmThinPlugin', $lvmthin_scfg, 'vm-100-disk-0', undef, 1, + ], + [ + 'lvmthin base is advertised', + 'PVE::Storage::LvmThinPlugin', $lvmthin_scfg, 'base-100-disk-0', undef, 1, + ], + [ + 'lvmthin snapshot is advertised', + 'PVE::Storage::LvmThinPlugin', $lvmthin_scfg, 'vm-100-disk-0', 'snap1', 1, + ], +]; + +plan tests => scalar($tests->@*) + 2; + +for my $t ($tests->@*) { + my ($desc, $class, $scfg, $volname, $snapname, $expected) = $t->@*; + + my $got = $class->volume_has_feature( + $scfg, 'copy-offload-atomic', 'store', $volname, $snapname, 0, + ); + + if (defined($expected)) { + is($got, $expected, $desc); + } else { + ok(!$got, $desc); + } +} + +# An unrelated feature must still come back from the normal table rather than being +# swallowed by the copy-offload handling. +ok( + PVE::Storage::BTRFSPlugin->volume_has_feature( + $btrfs_scfg, 'snapshot', 'store', '100/vm-100-disk-0.raw', undef, 0, + ), + 'btrfs still answers for unrelated features', +); +ok( + PVE::Storage::LvmThinPlugin->volume_has_feature( + $lvmthin_scfg, 'snapshot', 'store', 'vm-100-disk-0', undef, 0, + ), + 'lvmthin still answers for unrelated features', +); + +done_testing(); diff --git a/src/test/copy_offload_functional_test.pl b/src/test/copy_offload_functional_test.pl new file mode 100644 index 0000000..9b3e849 --- /dev/null +++ b/src/test/copy_offload_functional_test.pl @@ -0,0 +1,286 @@ +#!/usr/bin/perl + +# Functional tests for the copy-offload hooks of BTRFSPlugin and LvmThinPlugin. +# +# Unlike the rest of src/test, this one touches real storage: it builds a loop-backed +# btrfs filesystem and a loop-backed LVM thin pool, drives copy_image_prepare/start/ +# status against them, and checks the resulting images byte for byte. That needs root +# and the btrfs/lvm tools, so it is NOT part of run_plugin_tests.pl -- run it by hand: +# +# perl copy_offload_functional_test.pl +# +# It skips cleanly rather than failing when it cannot run. Everything it creates lives +# under /tmp and on its own loop devices, and is torn down at exit even on failure; it +# never touches an existing volume group or mount. +# +# What is actually being checked, and why these and not others: +# +# - a copy taken from a SNAPSHOT returns the snapshot's content, not the live volume's. +# Getting this wrong is silent: the clone is readable and passes every check, it just +# holds the wrong data. +# - the copy survives deleting the source outright. That is what 'copy-offload-atomic' +# promises and what separates these backends from a ZFS clone. +# - two prepares for the SAME target VM return different names. The core prepares every +# disk of a VM before starting any of them, so a reservation its own lister cannot see +# makes every multi-disk clone fail. + +use strict; +use warnings; + +use lib qw(..); + +use File::Path qw(mkpath rmtree); +use PVE::Storage; +use PVE::Storage::BTRFSPlugin; +use PVE::Storage::LvmThinPlugin; +use PVE::Tools qw(run_command); +use Test::More; + +my $BTRFS_MNT = '/tmp/pve-copyoffload-btrfs'; +my $BTRFS_IMG = '/tmp/pve-copyoffload-btrfs.img'; +my $LVM_IMG = '/tmp/pve-copyoffload-lvm.img'; +my $VG = 'pvecopyoffloadtest'; + +my @cleanup; + +sub sh { return scalar(qx{$_[0] 2>/dev/null}) } + +sub cleanup_all { + for my $c (reverse @cleanup) { eval { $c->() }; } + @cleanup = (); +} +END { cleanup_all() } +$SIG{INT} = $SIG{TERM} = sub { cleanup_all(); exit 1 }; + +if ($> != 0) { + plan skip_all => 'needs root to create loop devices, filesystems and volume groups'; +} +for my $tool (qw(btrfs mkfs.btrfs losetup findmnt lvcreate vgcreate pvcreate)) { + if (!sh("command -v $tool")) { + plan skip_all => "missing required tool '$tool'"; + } +} +if (sh("vgs --noheadings -o vg_name 2>/dev/null") =~ /\b\Q$VG\E\b/) { + plan skip_all => "volume group '$VG' already exists - refusing to touch it"; +} + +plan tests => 21; + +# Hash the WHOLE object, and let md5sum do its own reading. +# +# Deliberately not 'dd bs=1M count=N | md5sum': dd counts a short read as a full block, +# so it can return less than asked for, and how much depends on whether the data came +# from the page cache or off the disk. Comparing a freshly written source against a +# cold copy that way reports a mismatch between two byte-identical files. It also hides +# a missing file as the md5 of empty input. +sub md5_of { + my ($path) = @_; + + die "cannot hash '$path': not present\n" if !-e $path; + my $out = sh("md5sum '$path'"); + $out =~ s/\s.*//s; + die "md5sum of '$path' produced nothing\n" if $out !~ /^[0-9a-f]{32}$/; + return $out; +} + +sub identical { + my ($a, $b) = @_; + return system('cmp', '-s', $a, $b) == 0; +} + +# ---------------------------------------------------------------- btrfs + +my $btrfs_ok = eval { + run_command(['truncate', '-s', '2G', $BTRFS_IMG]); + push @cleanup, sub { unlink $BTRFS_IMG }; + + my $loop = sh("losetup --find --show $BTRFS_IMG"); + chomp $loop; + die "no loop device\n" if !$loop; + push @cleanup, sub { sh("losetup -d $loop") }; + + run_command(['mkfs.btrfs', '-q', '-f', $loop]); + mkpath $BTRFS_MNT; + run_command(['mount', $loop, $BTRFS_MNT]); + push @cleanup, sub { + for my $s (reverse split /\n/, sh("btrfs subvolume list -o $BTRFS_MNT | awk '{print \$NF}'")) { + sh("btrfs -q subvolume delete '$BTRFS_MNT/$s'"); + } + sh("umount $BTRFS_MNT"); + rmtree $BTRFS_MNT; + }; + mkpath "$BTRFS_MNT/images"; + 1; +}; +if (!$btrfs_ok) { + diag("btrfs setup failed: $@"); + SKIP: { skip 'btrfs setup failed', 11 } +} else { + my $C = 'PVE::Storage::BTRFSPlugin'; + my $scfg = { path => $BTRFS_MNT, type => 'btrfs', content => { images => 1 } }; + + my $src = $C->alloc_image('bt', $scfg, 100, 'raw', undef, 64 * 1024); + my $srcpath = $C->filesystem_path($scfg, $src); + sh("dd if=/dev/urandom of=$srcpath bs=1M count=16 conv=notrunc,fsync status=none"); + + $C->volume_snapshot($scfg, 'bt', $src, 'snap1'); + my $snappath = $C->filesystem_path($scfg, $src, 'snap1'); + sh("dd if=/dev/urandom of=$srcpath bs=1M count=16 conv=notrunc,fsync status=none"); + ok(!identical($srcpath, $snappath), 'btrfs: source diverged from its snapshot'); + + # THE multi-disk case: the core prepares every disk before starting any of them. + my $a = $C->copy_image_prepare($scfg, 'bt', $src, $scfg, 'bt', 201, undef, {}); + my $b = $C->copy_image_prepare($scfg, 'bt', $src, $scfg, 'bt', 201, undef, {}); + isnt($a, $b, 'btrfs: two prepares for one target VM reserve different names'); + + $C->copy_image_start($scfg, 'bt', $src, $scfg, 'bt', $a, undef); + + # After start() but BEFORE status() cleans up, the reserved name must still be held. + # start() swaps the copy in with RENAME_EXCHANGE precisely so the name is never + # momentarily free; a plain park-then-rename loses the reservation exactly here, and + # a concurrent allocation could then take the name out from under the caller's + # rollback. Checked between the two calls on purpose -- that is the fragile moment. + my ($a_name) = $a =~ m{/(.*)$}; + isnt( + $C->find_free_diskname('bt', $scfg, 201, 'raw', 1), $a_name, + 'btrfs: the reserved name is still held across the swap', + ); + + my $st = $C->copy_image_status($scfg, 'bt', $a, undef); + is($st->{state}, 'complete', 'btrfs: status complete on the first poll'); + my $apath = $C->filesystem_path($scfg, $a); + ok(identical($apath, $srcpath), 'btrfs: copy matches the source'); + + my $snapcopy = $C->copy_image_prepare($scfg, 'bt', $src, $scfg, 'bt', 202, 'snap1', {}); + $C->copy_image_start($scfg, 'bt', $src, $scfg, 'bt', $snapcopy, 'snap1'); + $C->copy_image_status($scfg, 'bt', $snapcopy, undef); + # NB: assign filesystem_path() to a scalar first. It returns ($path, $vmid, $vtype) + # in list context, and sub arguments ARE list context -- passing the call directly + # would hand identical() the vmid as its second path. + my $snapcopy_path = $C->filesystem_path($scfg, $snapcopy); + ok( + identical($snapcopy_path, $snappath), + 'btrfs: a copy from a snapshot holds the SNAPSHOT content, not live data', + ); + + # Independence: hash the copy, destroy the source outright, hash again. + my $before = md5_of($apath); + $C->free_image('bt', $scfg, $src, 0); + ok(!-e $srcpath, 'btrfs: source really is gone'); + is($before, md5_of($apath), 'btrfs: copy is unaffected by deleting the source'); + unlike( + sh("find $BTRFS_MNT -maxdepth 4 -name '*.copytmp' -o -name '*.copynew'"), qr/\S/, + 'btrfs: no parked or staging placeholder left behind', + ); + + # Failure path: same as the lvmthin case -- a placeholder left by a copy that died + # between start() and status() must not wedge the name, since rename() refuses to + # park onto an existing path and the leftover is invisible to list_images(). + my $stale = $C->copy_image_prepare($scfg, 'bt', $a, $scfg, 'bt', 203, undef, {}); + my $stale_subvol = $C->filesystem_path($scfg, $stale); + $stale_subvol =~ s|/disk\.raw$||; + rename($stale_subvol, "$stale_subvol.copytmp") + or die "could not stage the leaked-placeholder case - $!\n"; + my $reused = eval { $C->copy_image_prepare($scfg, 'bt', $a, $scfg, 'bt', 203, undef, {}) }; + ok(defined($reused), 'btrfs: a leaked placeholder does not wedge the disk name') + or diag("prepare failed: $@"); + if (defined($reused)) { + $C->copy_image_start($scfg, 'bt', $a, $scfg, 'bt', $reused, undef); + $C->copy_image_status($scfg, 'bt', $reused, undef); + my $reused_path = $C->filesystem_path($scfg, $reused); + ok(identical($reused_path, $apath), 'btrfs: the retried copy is correct'); + } else { + ok(0, 'btrfs: the retried copy is correct'); + } +} + +# ---------------------------------------------------------------- lvmthin + +my $lvm_ok = eval { + run_command(['truncate', '-s', '3G', $LVM_IMG]); + push @cleanup, sub { unlink $LVM_IMG }; + + my $loop = sh("losetup --find --show $LVM_IMG"); + chomp $loop; + die "no loop device\n" if !$loop; + # Detach as its own entry, registered IMMEDIATELY. Folding it into the entry pushed + # after vgcreate would leak the loop device whenever pvcreate or vgcreate fails -- + # the eval catches that, the test SKIPs "cleanly", and the device stays attached. + push @cleanup, sub { sh("losetup -d $loop") }; + + run_command(['pvcreate', '-qq', '-f', $loop]); + run_command(['vgcreate', '-qq', $VG, $loop]); + push @cleanup, sub { sh("vgremove -qq -f $VG"); sh("pvremove -qq -f $loop") }; + + run_command(['lvcreate', '-qq', '--type', 'thin-pool', '-L', '2G', '-n', 'tp', $VG]); + 1; +}; +if (!$lvm_ok) { + diag("lvm setup failed: $@"); + SKIP: { skip 'lvm setup failed', 10 } +} else { + my $C = 'PVE::Storage::LvmThinPlugin'; + my $scfg = { vgname => $VG, thinpool => 'tp', type => 'lvmthin', content => { images => 1 } }; + my $act = sub { sh("lvchange -ay -K $VG/$_[0]") }; + + my $src = $C->alloc_image('lt', $scfg, 100, 'raw', undef, 64 * 1024); + $act->($src); + sh("dd if=/dev/urandom of=/dev/$VG/$src bs=1M count=8 conv=fsync status=none"); + + $C->volume_snapshot($scfg, 'lt', $src, 'snap1'); + my $snapdev = "/dev/$VG/snap_${src}_snap1"; + $act->("snap_${src}_snap1"); + sh("dd if=/dev/urandom of=/dev/$VG/$src bs=1M count=8 conv=fsync status=none"); + ok(!identical("/dev/$VG/$src", $snapdev), 'lvmthin: source diverged from its snapshot'); + + my $a = $C->copy_image_prepare($scfg, 'lt', $src, $scfg, 'lt', 201, undef, {}); + my $b = $C->copy_image_prepare($scfg, 'lt', $src, $scfg, 'lt', 201, undef, {}); + isnt($a, $b, 'lvmthin: two prepares for one target VM reserve different names'); + + $C->copy_image_start($scfg, 'lt', $src, $scfg, 'lt', $a, undef); + my $st = $C->copy_image_status($scfg, 'lt', $a, undef); + is($st->{state}, 'complete', 'lvmthin: status complete on the first poll'); + $act->($a); + ok(identical("/dev/$VG/$a", "/dev/$VG/$src"), 'lvmthin: copy matches the source'); + + my $snapcopy = $C->copy_image_prepare($scfg, 'lt', $src, $scfg, 'lt', 202, 'snap1', {}); + $C->copy_image_start($scfg, 'lt', $src, $scfg, 'lt', $snapcopy, 'snap1'); + $C->copy_image_status($scfg, 'lt', $snapcopy, undef); + $act->($snapcopy); + ok( + identical("/dev/$VG/$snapcopy", $snapdev), + 'lvmthin: a copy from a snapshot holds the SNAPSHOT content, not live data', + ); + + # Independence: hash the copy, destroy the source outright, hash again. + my $before = md5_of("/dev/$VG/$a"); + $C->volume_snapshot_delete($scfg, 'lt', $src, 'snap1'); + $C->free_image('lt', $scfg, $src, 0); + ok(!-e "/dev/$VG/$src", 'lvmthin: source really is gone'); + is($before, md5_of("/dev/$VG/$a"), 'lvmthin: copy is unaffected by deleting the source'); + + unlike( + sh("lvs --noheadings -o lv_name $VG"), qr/copytmp-/, + 'lvmthin: no parked placeholder left behind', + ); + + # Failure path: a placeholder left by an earlier copy that died between start() and + # status() must not wedge the name. Without the reaping in prepare(), 'lvrename' + # cannot park onto the existing name and EVERY later copy to it fails -- and the + # leftover is invisible to list_images(), so nobody would know why. + my $stale = $C->copy_image_prepare($scfg, 'lt', $a, $scfg, 'lt', 203, undef, {}); + sh("lvrename $VG $stale copytmp-$stale"); + my $reused = eval { $C->copy_image_prepare($scfg, 'lt', $a, $scfg, 'lt', 203, undef, {}) }; + ok(defined($reused), 'lvmthin: a leaked placeholder does not wedge the disk name') + or diag("prepare failed: $@"); + if (defined($reused)) { + $C->copy_image_start($scfg, 'lt', $a, $scfg, 'lt', $reused, undef); + $C->copy_image_status($scfg, 'lt', $reused, undef); + $act->($reused); + ok(identical("/dev/$VG/$reused", "/dev/$VG/$a"), 'lvmthin: the retried copy is correct'); + } else { + ok(0, 'lvmthin: the retried copy is correct'); + } +} + +cleanup_all(); diff --git a/src/test/run_plugin_tests.pl b/src/test/run_plugin_tests.pl index 8f2472e..84995fe 100755 --- a/src/test/run_plugin_tests.pl +++ b/src/test/run_plugin_tests.pl @@ -19,6 +19,7 @@ my $res = $harness->runtests( "prune_backups_test.pm", "copy_offload_test.pm", "copy_offload_naming_test.pm", + "copy_offload_feature_test.pm", ); exit -1 if !$res || $res->{failed} || $res->{parse_errors}; -- 2.54.0