[PATCH] receive: strip root subvol path during process_clone

[PATCH] receive: strip root subvol path during process_clone

[Benedikt Morbach]

otherwise we get

    ERROR: cannot open <subvol_path>: No such file or directory

because <full_root_path>/<subvol_path> doesn't exist, so openat() will fail below.

Signed-off-by: Benedikt Morbach <benedikt.morbach@googlemail.com>
---
 cmds-receive.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/cmds-receive.c b/cmds-receive.c
index f4a3a4f..a975fdd 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -753,6 +753,17 @@ static int process_clone(const char *path, u64 offset, u64 len,
 		subvol_path = strdup(si->path);
 	}
 
+	/* strip the subvolume that we are receiving to from the start of subvol_path */
+	if (r->full_root_path &&
+		strstr(subvol_path, r->full_root_path) == subvol_path) {
+		size_t root_len = strlen(r->full_root_path);
+		size_t sub_len = strlen(subvol_path);
+
+		memmove(subvol_path,
+			subvol_path + root_len + 1,
+			sub_len - root_len);
+	}
+
 	ret = path_cat_out(full_clone_path, subvol_path, clone_path);
 	if (ret < 0) {
 		error("clone: target path invalid: %s", clone_path);
-- 
2.8.4

Re: [PATCH] receive: strip root subvol path during process_clone

[Benedikt Morbach]

Hi all,

this fixes http://thread.gmane.org/gmane.comp.file-systems.btrfs/56902 
for me.
I got to this via gdb + good old debug printf and tbh I'm not entirely
clear about the semantics of process_clone, so some error handling
might be missing here?

Cheers
Benedikt

[PATCH] btrfs-progs: receive: handle root subvol path in clone

[Benedikt Morbach]

otherwise we get

    ERROR: cannot open <subvol_path>: No such file or directory

because <full_root_path>/<subvol_path> doesn't exist, so openat() will fail below.

Signed-off-by: Benedikt Morbach <benedikt.morbach@googlemail.com>
---


resend with 'btrfs-progs:' in the subject.
Sorry for the noise

cheers

 cmds-receive.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/cmds-receive.c b/cmds-receive.c
index f4a3a4f..a975fdd 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -753,6 +753,17 @@ static int process_clone(const char *path, u64 offset, u64 len,
 		subvol_path = strdup(si->path);
 	}
 
+	/* strip the subvolume that we are receiving to from the start of subvol_path */
+	if (r->full_root_path &&
+		strstr(subvol_path, r->full_root_path) == subvol_path) {
+		size_t root_len = strlen(r->full_root_path);
+		size_t sub_len = strlen(subvol_path);
+
+		memmove(subvol_path,
+			subvol_path + root_len + 1,
+			sub_len - root_len);
+	}
+
 	ret = path_cat_out(full_clone_path, subvol_path, clone_path);
 	if (ret < 0) {
 		error("clone: target path invalid: %s", clone_path);
-- 
2.8.4

Re: [PATCH] receive: strip root subvol path during process_clone

[David Sterba]

Hi,

On Wed, Jun 15, 2016 at 01:06:46AM +0200, Benedikt Morbach wrote:
> this fixes http://thread.gmane.org/gmane.comp.file-systems.btrfs/56902 
> for me.
> I got to this via gdb + good old debug printf and tbh I'm not entirely
> clear about the semantics of process_clone, so some error handling
> might be missing here?

thanks for the fix. The mail thread holds enough information to
reproduce the issue, I'll get to that on Monday. I'm not sure about the
use of strstr to match the subvolume name so I want to write the tests
first and craft some paths to see how it works.

Re: [PATCH] receive: strip root subvol path during process_clone

[Benedikt Morbach]

Hi David,

On Fri, Jun 17, 2016 at 7:18 PM, David Sterba <dsterba@suse.cz> wrote:
> thanks for the fix. The mail thread holds enough information to
> reproduce the issue, I'll get to that on Monday. I'm not sure about the
> use of strstr to match the subvolume name so I want to write the tests
> first and craft some paths to see how it works.

any update on this? I've been running this patch for a few weeks now
with my hourly backups.
I could also take a stab at writing a test case but I think I'd need
some pointers for that. ;-)

I took a look at the send/receive patches that Filipe sent earlier and
afaict this isn't fixed in those.

Re: [PATCH] receive: strip root subvol path during process_clone

[David Sterba]

On Mon, Jul 11, 2016 at 04:07:25PM +0200, Benedikt Morbach wrote:
> Hi David,
> 
> On Fri, Jun 17, 2016 at 7:18 PM, David Sterba <dsterba@suse.cz> wrote:
> > thanks for the fix. The mail thread holds enough information to
> > reproduce the issue, I'll get to that on Monday. I'm not sure about the
> > use of strstr to match the subvolume name so I want to write the tests
> > first and craft some paths to see how it works.
> 
> any update on this? I've been running this patch for a few weeks now
> with my hourly backups.
> I could also take a stab at writing a test case but I think I'd need
> some pointers for that. ;-)

Sorry, I got distracted. The things with paths are a bit complicated,
there's chroot support, explicit mountpoint (-m), so stripping the
root_path would not work in all cases.

Apparently the full_clone_path is wrong there as it's not relative to
root_path and thus openat(mnt_fd, ...) will fail, so instead of
stripping the path, we would need to provide the clone path relative to
root_path directly.

> I took a look at the send/receive patches that Filipe sent earlier and
> afaict this isn't fixed in those.

No, this is not related to kernel.