* [PATCH] btrfs-progs: receive: cannot find clone source subvol when receiving in reverse direction
@ 2023-08-26 14:33 Arsenii Skvortsov
2023-08-28 13:24 ` Josef Bacik
0 siblings, 1 reply; 2+ messages in thread
From: Arsenii Skvortsov @ 2023-08-26 14:33 UTC (permalink / raw)
To: linux-btrfs
process_clone, unlike process_snapshot, only searched a subvolume
matching by received uuid. However, when earlier "receiver" side
sends, it mentions received uuid, which is for earlier "send" side
(now "receiver" side) is just uuid.
Fixes: https://github.com/kdave/btrfs-progs/issues/606
Signed-off-by: Arsenii Skvortsov <ettavolt@gmail.com>
---
cmds/receive.c | 28 +++---
tests/misc-tests/058-reverse-receive/test.sh | 98 ++++++++++++++++++++
2 files changed, 115 insertions(+), 11 deletions(-)
create mode 100755 tests/misc-tests/058-reverse-receive/test.sh
diff --git a/cmds/receive.c b/cmds/receive.c
index d16dc0a..bdd4dee 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -222,6 +222,19 @@ out:
return ret;
}
+static struct subvol_info *search_source_subvol(struct subvol_uuid_search *s,
+ const u8 *subvol_uuid, u64 transid)
+{
+ struct subvol_info *found;
+ found = subvol_uuid_search(s, 0, subvol_uuid, transid, NULL,
+ subvol_search_by_received_uuid);
+ if (IS_ERR_OR_NULL(found)) {
+ found = subvol_uuid_search(s, 0, subvol_uuid, transid, NULL,
+ subvol_search_by_uuid);
+ }
+ return found;
+}
+
static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
const u8 *parent_uuid, u64 parent_ctransid,
void *user)
@@ -284,14 +297,8 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
memset(&args_v2, 0, sizeof(args_v2));
strncpy_null(args_v2.name, path);
- parent_subvol = subvol_uuid_search(rctx->mnt_fd, 0, parent_uuid,
- parent_ctransid, NULL,
- subvol_search_by_received_uuid);
- if (IS_ERR_OR_NULL(parent_subvol)) {
- parent_subvol = subvol_uuid_search(rctx->mnt_fd, 0, parent_uuid,
- parent_ctransid, NULL,
- subvol_search_by_uuid);
- }
+ parent_subvol = search_source_subvol(rctx->mnt_fd, parent_uuid,
+ parent_ctransid);
if (IS_ERR_OR_NULL(parent_subvol)) {
if (!parent_subvol)
ret = -ENOENT;
@@ -746,9 +753,8 @@ static int process_clone(const char *path, u64 offset, u64 len,
BTRFS_UUID_SIZE) == 0) {
subvol_path = rctx->cur_subvol_path;
} else {
- si = subvol_uuid_search(rctx->mnt_fd, 0, clone_uuid, clone_ctransid,
- NULL,
- subvol_search_by_received_uuid);
+ si = search_source_subvol(rctx->mnt_fd, clone_uuid,
+ clone_ctransid);
if (IS_ERR_OR_NULL(si)) {
char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
diff --git a/tests/misc-tests/058-reverse-receive/test.sh b/tests/misc-tests/058-reverse-receive/test.sh
new file mode 100755
index 0000000..6eff560
--- /dev/null
+++ b/tests/misc-tests/058-reverse-receive/test.sh
@@ -0,0 +1,98 @@
+#!/bin/bash
+#
+# Receive in reverse direction must not throw an error if it can find an earlier "sent" parent.
+# In general, shows a backup+sync setup between two (or more) PCs with an external drive.
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+check_global_prereq dd
+
+declare -a roots
+i_pc1=1
+# An external drive used to backup and carry profile.
+i_ext=2
+i_pc2=3
+roots[$i_pc1]="$TEST_MNT/pc1"
+roots[$i_ext]="$TEST_MNT/external"
+roots[$i_pc2]="$TEST_MNT/pc2"
+
+setup_root_helper
+mkdir -p ${roots[@]}
+setup_loopdevs 3
+prepare_loopdevs
+for i in `seq 3`; do
+ TEST_DEV=${loopdevs[$i]}
+ TEST_MNT="${roots[$i]}"
+ run_check_mkfs_test_dev
+ run_check_mount_test_dev
+ run_check $SUDO_HELPER mkdir -p "$TEST_MNT/.snapshots"
+done
+
+run_check_update_file()
+{
+ run_check $SUDO_HELPER cp --reflink ${roots[$1]}/profile/$2 ${roots[$1]}/profile/staging
+ run_check $SUDO_HELPER dd if=/dev/urandom conv=notrunc bs=4K count=4 oseek=$3 "of=${roots[$1]}/profile/staging"
+ run_check $SUDO_HELPER mv ${roots[$1]}/profile/staging ${roots[$1]}/profile/$2
+}
+run_check_copy_snapshot_with_diff()
+{
+ _mktemp_local send.data
+ run_check $SUDO_HELPER "$TOP/btrfs" send -f send.data -p "${roots[$1]}/.snapshots/$2" "${roots[$1]}/.snapshots/$3"
+ run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "${roots[$4]}/.snapshots"
+}
+run_check_backup_profile()
+{
+ run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r "${roots[$1]}/profile" "${roots[$1]}/.snapshots/$3"
+ run_check_copy_snapshot_with_diff $1 $2 $3 $i_ext
+ # Don't keep old snapshot in pc
+ run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete "${roots[$1]}/.snapshots/$2"
+}
+run_check_restore_profile()
+{
+ run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot "${roots[$1]}/.snapshots/$2" "${roots[$1]}/profile"
+}
+run_check_copy_fresh_backup_and_replace_profile()
+{
+ run_check_copy_snapshot_with_diff $i_ext $2 $3 $1
+ # IRL, it would be a nice idea to make a backup snapshot before deleting.
+ run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete "${roots[$1]}/profile"
+ run_check_restore_profile $1 $3
+ # Don't keep old snapshot in pc
+ run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete "${roots[$1]}/.snapshots/$2"
+}
+
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "${roots[$i_pc1]}/profile"
+run_check $SUDO_HELPER dd if=/dev/urandom bs=4K count=16 "of=${roots[$i_pc1]}/profile/day1"
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r "${roots[$i_pc1]}/profile" "${roots[$i_pc1]}/.snapshots/day1"
+_mktemp_local send.data
+run_check $SUDO_HELPER "$TOP/btrfs" send -f send.data "${roots[$i_pc1]}/.snapshots/day1"
+run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "${roots[$i_ext]}/.snapshots"
+
+run_check_update_file $i_pc1 day1 2
+run_check_backup_profile $i_pc1 day1 day2
+
+_mktemp_local send.data
+run_check $SUDO_HELPER "$TOP/btrfs" send -f send.data "${roots[$i_ext]}/.snapshots/day2"
+run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "${roots[$i_pc2]}/.snapshots"
+run_check_restore_profile $i_pc2 day2
+run_check_update_file $i_pc2 day1 3
+run_check_backup_profile $i_pc2 day2 day3
+
+run_check_update_file $i_pc2 day1 4
+run_check_backup_profile $i_pc2 day3 day4
+
+run_check_copy_fresh_backup_and_replace_profile $i_pc1 day2 day4
+run_check_update_file $i_pc1 day1 5
+run_check_backup_profile $i_pc1 day4 day5
+
+run_check_copy_fresh_backup_and_replace_profile $i_pc2 day4 day5
+run_check_update_file $i_pc2 day1 6
+run_check_backup_profile $i_pc2 day5 day6
+
+run_check_umount_test_dev ${loopdevs[@]}
+rmdir ${roots[@]}
+rm -f send.data
+cleanup_loopdevs
--
2.41.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] btrfs-progs: receive: cannot find clone source subvol when receiving in reverse direction
2023-08-26 14:33 [PATCH] btrfs-progs: receive: cannot find clone source subvol when receiving in reverse direction Arsenii Skvortsov
@ 2023-08-28 13:24 ` Josef Bacik
0 siblings, 0 replies; 2+ messages in thread
From: Josef Bacik @ 2023-08-28 13:24 UTC (permalink / raw)
To: Arsenii Skvortsov; +Cc: linux-btrfs
On Sat, Aug 26, 2023 at 10:33:22AM -0400, Arsenii Skvortsov wrote:
> process_clone, unlike process_snapshot, only searched a subvolume
> matching by received uuid. However, when earlier "receiver" side
> sends, it mentions received uuid, which is for earlier "send" side
> (now "receiver" side) is just uuid.
>
The changelog is too terse for what you're describing, it took me a while to
figure out what exactly the problem was and what you were trying to accomplish.
Something more like
process_clone only searches the received_uuid, but could exist in an earlier
uuid that isn't the received_uuid. Mirror what process_snapshot does and search
both the received_uuid and if that fails look up by normal uuid.
Or something like that.
> Fixes: https://github.com/kdave/btrfs-progs/issues/606
>
> Signed-off-by: Arsenii Skvortsov <ettavolt@gmail.com>
> ---
> cmds/receive.c | 28 +++---
> tests/misc-tests/058-reverse-receive/test.sh | 98 ++++++++++++++++++++
> 2 files changed, 115 insertions(+), 11 deletions(-)
> create mode 100755 tests/misc-tests/058-reverse-receive/test.sh
>
> diff --git a/cmds/receive.c b/cmds/receive.c
> index d16dc0a..bdd4dee 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -222,6 +222,19 @@ out:
> return ret;
> }
>
> +static struct subvol_info *search_source_subvol(struct subvol_uuid_search *s,
> + const u8 *subvol_uuid, u64 transid)
The formatting is off right here, and in the whole patch, you need to use tabs
not spaces for indention. The code and the test are great, just fixup your
editor to use tabs. If you're using VIM let me know and I can give you a .vimrc
that will do the right thing. Thanks,
Josef
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-08-28 13:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-26 14:33 [PATCH] btrfs-progs: receive: cannot find clone source subvol when receiving in reverse direction Arsenii Skvortsov
2023-08-28 13:24 ` Josef Bacik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox