[PATCH v2 0/4] btrfs-progs: fix clone from wrong subvolume

[PATCH v2 0/4] btrfs-progs: fix clone from wrong subvolume

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

This is v2 of [1]. Changes from v1:

- Split out removing commented-out code to new patch 1 and removed a
  related comment block.
- Made subvol_path const char * in patch 2.
- Added test case as patch 4.
- Fixed wrong signed-off-by.

Thanks,
Omar

1: https://lore.kernel.org/linux-btrfs/cover.1563600688.git.osandov@fb.com/T/#u

Omar Sandoval (4):
  btrfs-progs: receive: remove commented out transid checks
  btrfs-progs: receive: get rid of unnecessary strdup()
  btrfs-progs: receive: don't lookup clone root for received subvolume
  btrfs-progs: tests: add test for receiving clone from duplicate
    subvolume

 cmds/receive.c                                | 50 ++++---------------
 .../test.sh                                   | 34 +++++++++++++
 2 files changed, 45 insertions(+), 39 deletions(-)
 create mode 100755 tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh

-- 
2.22.0

[PATCH v2 1/4] btrfs-progs: receive: remove commented out transid checks

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

The checks for a subvolume being modified after it was received have
been commented out since they were added back in commit f1c24cd80dfd
("Btrfs-progs: add btrfs send/receive commands"). Let's just get rid of
the noise.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 cmds/receive.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index b97850a7..830ed082 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -344,15 +344,6 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 			parent_subvol->path[sub_len - root_len - 1] = '\0';
 		}
 	}
-	/*if (rs_args.ctransid > rs_args.rtransid) {
-		if (!r->force) {
-			ret = -EINVAL;
-			fprintf(stderr, "ERROR: subvolume %s was modified after it was received.\n", r->subvol_parent_name);
-			goto out;
-		} else {
-			fprintf(stderr, "WARNING: subvolume %s was modified after it was received.\n", r->subvol_parent_name);
-		}
-	}*/
 
 	if (*parent_subvol->path == 0)
 		args_v2.fd = dup(rctx->mnt_fd);
@@ -770,22 +761,6 @@ static int process_clone(const char *path, u64 offset, u64 len,
 			goto out;
 		}
 	} else {
-		/*if (rs_args.ctransid > rs_args.rtransid) {
-			if (!r->force) {
-				ret = -EINVAL;
-				fprintf(stderr, "ERROR: subvolume %s was "
-						"modified after it was "
-						"received.\n",
-						r->subvol_parent_name);
-				goto out;
-			} else {
-				fprintf(stderr, "WARNING: subvolume %s was "
-						"modified after it was "
-						"received.\n",
-						r->subvol_parent_name);
-			}
-		}*/
-
 		/* strip the subvolume that we are receiving to from the start of subvol_path */
 		if (rctx->full_root_path) {
 			size_t root_len = strlen(rctx->full_root_path);
-- 
2.22.0

[PATCH v2 2/4] btrfs-progs: receive: get rid of unnecessary strdup()

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

In process_clone(), we're not checking the return value of strdup().
But, there's no reason to strdup() in the first place: we just pass the
path into path_cat_out(). Get rid of the strdup().

Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 cmds/receive.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index 830ed082..dba05982 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -730,7 +730,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
 	struct btrfs_ioctl_clone_range_args clone_args;
 	struct subvol_info *si = NULL;
 	char full_path[PATH_MAX];
-	char *subvol_path = NULL;
+	const char *subvol_path;
 	char full_clone_path[PATH_MAX];
 	int clone_fd = -1;
 
@@ -751,7 +751,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
 		if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
 				BTRFS_UUID_SIZE) == 0) {
 			/* TODO check generation of extent */
-			subvol_path = strdup(rctx->cur_subvol_path);
+			subvol_path = rctx->cur_subvol_path;
 		} else {
 			if (!si)
 				ret = -ENOENT;
@@ -769,14 +769,14 @@ static int process_clone(const char *path, u64 offset, u64 len,
 			if (sub_len > root_len &&
 			    strstr(si->path, rctx->full_root_path) == si->path &&
 			    si->path[root_len] == '/') {
-				subvol_path = strdup(si->path + root_len + 1);
+				subvol_path = si->path + root_len + 1;
 			} else {
 				error("clone: source subvol path %s unreachable from %s",
 					si->path, rctx->full_root_path);
 				goto out;
 			}
 		} else {
-			subvol_path = strdup(si->path);
+			subvol_path = si->path;
 		}
 	}
 
@@ -814,7 +814,6 @@ out:
 		free(si->path);
 		free(si);
 	}
-	free(subvol_path);
 	if (clone_fd != -1)
 		close(clone_fd);
 	return ret;
-- 
2.22.0

[PATCH v2 3/4] btrfs-progs: receive: don't lookup clone root for received subvolume

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

When we process a clone request, we look up the source subvolume by
UUID, even if the source is the subvolume that we're currently
receiving. Usually, this is fine. However, if for some reason we
previously received the same subvolume, then this will use paths
relative to the previously received subvolume instead of the current
one. This is incorrect, since the send stream may use temporary names
for the clone source. This can be reproduced as follows:

btrfs subvolume create subvol
dd if=/dev/urandom of=subvol/foo bs=1M count=1
cp --reflink subvol/foo subvol/bar
mkdir subvol/dir
mv subvol/foo subvol/dir/
btrfs property set subvol ro true
btrfs send -f send.data subvol
mkdir first second
btrfs receive -f send.data first
btrfs receive -f send.data second

The second receive results in this error:

ERROR: cannot open first/subvol/o259-7-0/foo: No such file or directory

Fix it by always cloning from the current subvolume if its UUID matches.
This has the nice side effect of avoiding unnecessary UUID tree lookups
in that case.

Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 cmds/receive.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index dba05982..1e6a29dd 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -744,15 +744,14 @@ static int process_clone(const char *path, u64 offset, u64 len,
 	if (ret < 0)
 		goto out;
 
-	si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
-				NULL,
-				subvol_search_by_received_uuid);
-	if (IS_ERR_OR_NULL(si)) {
-		if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
-				BTRFS_UUID_SIZE) == 0) {
-			/* TODO check generation of extent */
-			subvol_path = rctx->cur_subvol_path;
-		} else {
+	if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
+		   BTRFS_UUID_SIZE) == 0) {
+		subvol_path = rctx->cur_subvol_path;
+	} else {
+		si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
+					NULL,
+					subvol_search_by_received_uuid);
+		if (IS_ERR_OR_NULL(si)) {
 			if (!si)
 				ret = -ENOENT;
 			else
@@ -760,7 +759,6 @@ static int process_clone(const char *path, u64 offset, u64 len,
 			error("clone: did not find source subvol");
 			goto out;
 		}
-	} else {
 		/* strip the subvolume that we are receiving to from the start of subvol_path */
 		if (rctx->full_root_path) {
 			size_t root_len = strlen(rctx->full_root_path);
-- 
2.22.0

[PATCH v2 4/4] btrfs-progs: tests: add test for receiving clone from duplicate subvolume

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

This test case is the reproducer for the previous fix.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 .../test.sh                                   | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100755 tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh

diff --git a/tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh b/tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh
new file mode 100755
index 00000000..be648605
--- /dev/null
+++ b/tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh
@@ -0,0 +1,34 @@
+#!/bin/bash
+# Test that when receiving a subvolume whose received UUID already exists in
+# the filesystem, we clone from the correct source (the subvolume that we are
+# receiving, not the existing subvolume). This is a regression test for
+# "btrfs-progs: receive: don't lookup clone root for received subvolume".
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs
+check_prereq mkfs.btrfs
+
+setup_root_helper
+
+rm -f disk
+run_check truncate -s 1G disk
+chmod a+w disk
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f disk
+run_check $SUDO_HELPER mount -o loop disk "$TEST_MNT"
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subvol"
+run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/subvol/foo" \
+	bs=1M count=1 status=none
+run_check $SUDO_HELPER cp --reflink "$TEST_MNT/subvol/foo" "$TEST_MNT/subvol/bar"
+run_check $SUDO_HELPER mkdir "$TEST_MNT/subvol/dir"
+run_check $SUDO_HELPER mv "$TEST_MNT/subvol/foo" "$TEST_MNT/subvol/dir"
+run_check $SUDO_HELPER "$TOP/btrfs" property set "$TEST_MNT/subvol" ro true
+run_check $SUDO_HELPER "$TOP/btrfs" send -f send.data "$TEST_MNT/subvol"
+
+run_check $SUDO_HELPER mkdir "$TEST_MNT/first" "$TEST_MNT/second"
+run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "$TEST_MNT/first"
+run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "$TEST_MNT/second"
+
+run_check $SUDO_HELPER umount "$TEST_MNT"
+rm -f disk send.data
-- 
2.22.0

Re: [PATCH v2 3/4] btrfs-progs: receive: don't lookup clone root for received subvolume

[Filipe Manana]

On Tue, Jul 23, 2019 at 3:25 AM Omar Sandoval <osandov@osandov.com> wrote:
>
> From: Omar Sandoval <osandov@fb.com>
>
> When we process a clone request, we look up the source subvolume by
> UUID, even if the source is the subvolume that we're currently
> receiving. Usually, this is fine. However, if for some reason we
> previously received the same subvolume, then this will use paths
> relative to the previously received subvolume instead of the current
> one. This is incorrect, since the send stream may use temporary names
> for the clone source. This can be reproduced as follows:
>
> btrfs subvolume create subvol
> dd if=/dev/urandom of=subvol/foo bs=1M count=1
> cp --reflink subvol/foo subvol/bar
> mkdir subvol/dir
> mv subvol/foo subvol/dir/
> btrfs property set subvol ro true
> btrfs send -f send.data subvol
> mkdir first second
> btrfs receive -f send.data first
> btrfs receive -f send.data second
>
> The second receive results in this error:
>
> ERROR: cannot open first/subvol/o259-7-0/foo: No such file or directory
>
> Fix it by always cloning from the current subvolume if its UUID matches.
> This has the nice side effect of avoiding unnecessary UUID tree lookups
> in that case.
>
> Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks!

> ---
>  cmds/receive.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/cmds/receive.c b/cmds/receive.c
> index dba05982..1e6a29dd 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -744,15 +744,14 @@ static int process_clone(const char *path, u64 offset, u64 len,
>         if (ret < 0)
>                 goto out;
>
> -       si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
> -                               NULL,
> -                               subvol_search_by_received_uuid);
> -       if (IS_ERR_OR_NULL(si)) {
> -               if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
> -                               BTRFS_UUID_SIZE) == 0) {
> -                       /* TODO check generation of extent */
> -                       subvol_path = rctx->cur_subvol_path;
> -               } else {
> +       if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
> +                  BTRFS_UUID_SIZE) == 0) {
> +               subvol_path = rctx->cur_subvol_path;
> +       } else {
> +               si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
> +                                       NULL,
> +                                       subvol_search_by_received_uuid);
> +               if (IS_ERR_OR_NULL(si)) {
>                         if (!si)
>                                 ret = -ENOENT;
>                         else
> @@ -760,7 +759,6 @@ static int process_clone(const char *path, u64 offset, u64 len,
>                         error("clone: did not find source subvol");
>                         goto out;
>                 }
> -       } else {
>                 /* strip the subvolume that we are receiving to from the start of subvol_path */
>                 if (rctx->full_root_path) {
>                         size_t root_len = strlen(rctx->full_root_path);
> --
> 2.22.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH v2 4/4] btrfs-progs: tests: add test for receiving clone from duplicate subvolume

[Filipe Manana]

On Tue, Jul 23, 2019 at 3:25 AM Omar Sandoval <osandov@osandov.com> wrote:
>
> From: Omar Sandoval <osandov@fb.com>
>
> This test case is the reproducer for the previous fix.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, thanks!

> ---
>  .../test.sh                                   | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100755 tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh
>
> diff --git a/tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh b/tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh
> new file mode 100755
> index 00000000..be648605
> --- /dev/null
> +++ b/tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh
> @@ -0,0 +1,34 @@
> +#!/bin/bash
> +# Test that when receiving a subvolume whose received UUID already exists in
> +# the filesystem, we clone from the correct source (the subvolume that we are
> +# receiving, not the existing subvolume). This is a regression test for
> +# "btrfs-progs: receive: don't lookup clone root for received subvolume".
> +
> +source "$TEST_TOP/common"
> +
> +check_prereq btrfs
> +check_prereq mkfs.btrfs
> +
> +setup_root_helper
> +
> +rm -f disk
> +run_check truncate -s 1G disk
> +chmod a+w disk
> +run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f disk
> +run_check $SUDO_HELPER mount -o loop disk "$TEST_MNT"
> +
> +run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subvol"
> +run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/subvol/foo" \
> +       bs=1M count=1 status=none
> +run_check $SUDO_HELPER cp --reflink "$TEST_MNT/subvol/foo" "$TEST_MNT/subvol/bar"
> +run_check $SUDO_HELPER mkdir "$TEST_MNT/subvol/dir"
> +run_check $SUDO_HELPER mv "$TEST_MNT/subvol/foo" "$TEST_MNT/subvol/dir"
> +run_check $SUDO_HELPER "$TOP/btrfs" property set "$TEST_MNT/subvol" ro true
> +run_check $SUDO_HELPER "$TOP/btrfs" send -f send.data "$TEST_MNT/subvol"
> +
> +run_check $SUDO_HELPER mkdir "$TEST_MNT/first" "$TEST_MNT/second"
> +run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "$TEST_MNT/first"
> +run_check $SUDO_HELPER "$TOP/btrfs" receive -f send.data "$TEST_MNT/second"
> +
> +run_check $SUDO_HELPER umount "$TEST_MNT"
> +rm -f disk send.data
> --
> 2.22.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH v2 1/4] btrfs-progs: receive: remove commented out transid checks

[David Sterba]

On Mon, Jul 22, 2019 at 12:15:02PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> The checks for a subvolume being modified after it was received have
> been commented out since they were added back in commit f1c24cd80dfd
> ("Btrfs-progs: add btrfs send/receive commands"). Let's just get rid of
> the noise.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>  cmds/receive.c | 25 -------------------------
>  1 file changed, 25 deletions(-)
> 
> diff --git a/cmds/receive.c b/cmds/receive.c
> index b97850a7..830ed082 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -344,15 +344,6 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
>  			parent_subvol->path[sub_len - root_len - 1] = '\0';
>  		}
>  	}
> -	/*if (rs_args.ctransid > rs_args.rtransid) {

This looks like a (missing) sanity check, don't we want it?

Re: [PATCH v2 1/4] btrfs-progs: receive: remove commented out transid checks

[Filipe Manana]

On Wed, Jul 24, 2019 at 12:23 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Mon, Jul 22, 2019 at 12:15:02PM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > The checks for a subvolume being modified after it was received have
> > been commented out since they were added back in commit f1c24cd80dfd
> > ("Btrfs-progs: add btrfs send/receive commands"). Let's just get rid of
> > the noise.
> >
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> > ---
> >  cmds/receive.c | 25 -------------------------
> >  1 file changed, 25 deletions(-)
> >
> > diff --git a/cmds/receive.c b/cmds/receive.c
> > index b97850a7..830ed082 100644
> > --- a/cmds/receive.c
> > +++ b/cmds/receive.c
> > @@ -344,15 +344,6 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
> >                       parent_subvol->path[sub_len - root_len - 1] = '\0';
> >               }
> >       }
> > -     /*if (rs_args.ctransid > rs_args.rtransid) {
>
> This looks like a (missing) sanity check, don't we want it?

Un-commenting that will cause receive to fail if we deduplicate into
readonly subvolumes and snaphosts and then use them for send
operations, breaking existing use cases.



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH v2 0/4] btrfs-progs: fix clone from wrong subvolume

[Omar Sandoval]

On Mon, Jul 22, 2019 at 12:15:01PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> This is v2 of [1]. Changes from v1:
> 
> - Split out removing commented-out code to new patch 1 and removed a
>   related comment block.
> - Made subvol_path const char * in patch 2.
> - Added test case as patch 4.
> - Fixed wrong signed-off-by.
> 
> Thanks,
> Omar
> 
> 1: https://lore.kernel.org/linux-btrfs/cover.1563600688.git.osandov@fb.com/T/#u
> 
> Omar Sandoval (4):
>   btrfs-progs: receive: remove commented out transid checks
>   btrfs-progs: receive: get rid of unnecessary strdup()
>   btrfs-progs: receive: don't lookup clone root for received subvolume
>   btrfs-progs: tests: add test for receiving clone from duplicate
>     subvolume
> 
>  cmds/receive.c                                | 50 ++++---------------
>  .../test.sh                                   | 34 +++++++++++++
>  2 files changed, 45 insertions(+), 39 deletions(-)
>  create mode 100755 tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh

Ping. Looks like only patch 2 is in the devel branch.

Re: [PATCH v2 0/4] btrfs-progs: fix clone from wrong subvolume

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1346 bytes --]



On 2019/9/5 上午4:35, Omar Sandoval wrote:
> On Mon, Jul 22, 2019 at 12:15:01PM -0700, Omar Sandoval wrote:
>> From: Omar Sandoval <osandov@fb.com>
>>
>> This is v2 of [1]. Changes from v1:
>>
>> - Split out removing commented-out code to new patch 1 and removed a
>>   related comment block.
>> - Made subvol_path const char * in patch 2.
>> - Added test case as patch 4.
>> - Fixed wrong signed-off-by.
>>
>> Thanks,
>> Omar
>>
>> 1: https://lore.kernel.org/linux-btrfs/cover.1563600688.git.osandov@fb.com/T/#u
>>
>> Omar Sandoval (4):
>>   btrfs-progs: receive: remove commented out transid checks
>>   btrfs-progs: receive: get rid of unnecessary strdup()
>>   btrfs-progs: receive: don't lookup clone root for received subvolume
>>   btrfs-progs: tests: add test for receiving clone from duplicate
>>     subvolume
>>
>>  cmds/receive.c                                | 50 ++++---------------
>>  .../test.sh                                   | 34 +++++++++++++
>>  2 files changed, 45 insertions(+), 39 deletions(-)
>>  create mode 100755 tests/misc-tests/038-receive-clone-from-current-subvolume/test.sh
> 
> Ping. Looks like only patch 2 is in the devel branch.
> 

Since it looks good to me, I'll include the remaining patches in my next
pull to David, and hopes he accepts them.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 3/4] btrfs-progs: receive: don't lookup clone root for received subvolume

[Filipe Manana]

On Tue, Jul 23, 2019 at 12:19 PM Filipe Manana <fdmanana@gmail.com> wrote:
>
> On Tue, Jul 23, 2019 at 3:25 AM Omar Sandoval <osandov@osandov.com> wrote:
> >
> > From: Omar Sandoval <osandov@fb.com>
> >
> > When we process a clone request, we look up the source subvolume by
> > UUID, even if the source is the subvolume that we're currently
> > receiving. Usually, this is fine. However, if for some reason we
> > previously received the same subvolume, then this will use paths
> > relative to the previously received subvolume instead of the current
> > one. This is incorrect, since the send stream may use temporary names
> > for the clone source. This can be reproduced as follows:
> >
> > btrfs subvolume create subvol
> > dd if=/dev/urandom of=subvol/foo bs=1M count=1
> > cp --reflink subvol/foo subvol/bar
> > mkdir subvol/dir
> > mv subvol/foo subvol/dir/
> > btrfs property set subvol ro true
> > btrfs send -f send.data subvol
> > mkdir first second
> > btrfs receive -f send.data first
> > btrfs receive -f send.data second
> >
> > The second receive results in this error:
> >
> > ERROR: cannot open first/subvol/o259-7-0/foo: No such file or directory
> >
> > Fix it by always cloning from the current subvolume if its UUID matches.
> > This has the nice side effect of avoiding unnecessary UUID tree lookups
> > in that case.
> >
> > Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>

I can't find this patch in btrfs-progs. Any reason why it was never applied?

thanks

>
> Thanks!
>
> > ---
> >  cmds/receive.c | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/cmds/receive.c b/cmds/receive.c
> > index dba05982..1e6a29dd 100644
> > --- a/cmds/receive.c
> > +++ b/cmds/receive.c
> > @@ -744,15 +744,14 @@ static int process_clone(const char *path, u64 offset, u64 len,
> >         if (ret < 0)
> >                 goto out;
> >
> > -       si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
> > -                               NULL,
> > -                               subvol_search_by_received_uuid);
> > -       if (IS_ERR_OR_NULL(si)) {
> > -               if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
> > -                               BTRFS_UUID_SIZE) == 0) {
> > -                       /* TODO check generation of extent */
> > -                       subvol_path = rctx->cur_subvol_path;
> > -               } else {
> > +       if (memcmp(clone_uuid, rctx->cur_subvol.received_uuid,
> > +                  BTRFS_UUID_SIZE) == 0) {
> > +               subvol_path = rctx->cur_subvol_path;
> > +       } else {
> > +               si = subvol_uuid_search(&rctx->sus, 0, clone_uuid, clone_ctransid,
> > +                                       NULL,
> > +                                       subvol_search_by_received_uuid);
> > +               if (IS_ERR_OR_NULL(si)) {
> >                         if (!si)
> >                                 ret = -ENOENT;
> >                         else
> > @@ -760,7 +759,6 @@ static int process_clone(const char *path, u64 offset, u64 len,
> >                         error("clone: did not find source subvol");
> >                         goto out;
> >                 }
> > -       } else {
> >                 /* strip the subvolume that we are receiving to from the start of subvol_path */
> >                 if (rctx->full_root_path) {
> >                         size_t root_len = strlen(rctx->full_root_path);
> > --
> > 2.22.0
> >
>
>
> --
> Filipe David Manana,
>
> “Whether you think you can, or you think you can't — you're right.”



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH v2 3/4] btrfs-progs: receive: don't lookup clone root for received subvolume

[Omar Sandoval]

On Mon, Feb 24, 2020 at 02:53:51PM +0000, Filipe Manana wrote:
> On Tue, Jul 23, 2019 at 12:19 PM Filipe Manana <fdmanana@gmail.com> wrote:
> >
> > On Tue, Jul 23, 2019 at 3:25 AM Omar Sandoval <osandov@osandov.com> wrote:
> > >
> > > From: Omar Sandoval <osandov@fb.com>
> > >
> > > When we process a clone request, we look up the source subvolume by
> > > UUID, even if the source is the subvolume that we're currently
> > > receiving. Usually, this is fine. However, if for some reason we
> > > previously received the same subvolume, then this will use paths
> > > relative to the previously received subvolume instead of the current
> > > one. This is incorrect, since the send stream may use temporary names
> > > for the clone source. This can be reproduced as follows:
> > >
> > > btrfs subvolume create subvol
> > > dd if=/dev/urandom of=subvol/foo bs=1M count=1
> > > cp --reflink subvol/foo subvol/bar
> > > mkdir subvol/dir
> > > mv subvol/foo subvol/dir/
> > > btrfs property set subvol ro true
> > > btrfs send -f send.data subvol
> > > mkdir first second
> > > btrfs receive -f send.data first
> > > btrfs receive -f send.data second
> > >
> > > The second receive results in this error:
> > >
> > > ERROR: cannot open first/subvol/o259-7-0/foo: No such file or directory
> > >
> > > Fix it by always cloning from the current subvolume if its UUID matches.
> > > This has the nice side effect of avoiding unnecessary UUID tree lookups
> > > in that case.
> > >
> > > Fixes: f1c24cd80dfd ("Btrfs-progs: add btrfs send/receive commands")
> > > Signed-off-by: Omar Sandoval <osandov@fb.com>
> >
> > Reviewed-by: Filipe Manana <fdmanana@suse.com>
> 
> I can't find this patch in btrfs-progs. Any reason why it was never applied?
> 
> thanks

It must have gotten lost at some point. I'll resend it.