Skip to contentSkip to conversation

btrfs-progs: receive explicit parent support

PATCH Superseded Lauri Võsandi 4 participants 12 messages Apr 26, 2015 – May 21, 2015 Btrfs file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 btrfs-progs: receive explicit parent support

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead.

---
 cmds-receive.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)
cmds-receive.c +26 -8
index b7cf3f9..391d281 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
 	char *root_path;
 	char *dest_dir_path; /* relative to root_path */
 	char *full_subvol_path;
+	char *explicit_parent_path;
 	int dest_dir_chroot;
 
 	struct subvol_info *cur_subvol;
@@ -220,20 +221,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		fprintf(stderr, "receiving snapshot %s uuid=%s, "
 				"ctransid=%llu ", path, uuid_str,
 				r->cur_subvol->stransid);
-		uuid_unparse(parent_uuid, uuid_str);
-		fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-				uuid_str, parent_ctransid);
 	}
 
 	memset(&args_v2, 0, sizeof(args_v2));
 	strncpy_null(args_v2.name, path);
 
-	parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-			parent_ctransid, NULL, subvol_search_by_received_uuid);
-	if (!parent_subvol) {
+	if (r->explicit_parent_path) {
+		if (g_verbose) {
+			fprintf(stderr, "using explicit parent %s\n",
+					r->explicit_parent_path);
+		}
+		parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+			0, r->explicit_parent_path, subvol_search_by_path);
+	} else {
+		if (g_verbose) {
+			uuid_unparse(parent_uuid, uuid_str);
+			fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+					uuid_str, parent_ctransid);
+		}
 		parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-				parent_ctransid, NULL, subvol_search_by_uuid);
+			parent_ctransid, NULL, subvol_search_by_received_uuid);
+		if (!parent_subvol) {
+			parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+					parent_ctransid, NULL, subvol_search_by_uuid);
+		}
 	}
+
 	if (!parent_subvol) {
 		ret = -ENOENT;
 		fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -962,11 +975,14 @@ int cmd_receive(int argc, char **argv)
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+		c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
 		if (c < 0)
 			break;
 
 		switch (c) {
+		case 'p':
+			r.explicit_parent_path = optarg;
+			break;
 		case 'v':
 			g_verbose++;
 			break;
@@ -1028,6 +1044,8 @@ const char * const cmd_receive_usage[] = {
 	"                 in the data stream. Without this option,",
 	"                 the receiver terminates only if an error",
 	"                 is recognized or on EOF.",
+	"-p <subvol>      Disables the automatic searching for parents",
+	"                 if incremental streams are received.",
 	"-C|--chroot      confine the process to <mount> using chroot",
 	"--max-errors <N> Terminate as soon as N errors happened while",
 	"                 processing commands from the send stream.",
Signature · 1 line
1.9.1
#1 lore raw
Quoted 4 lines · Lauri Võsandi

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead.

As I said when we discussed this on IRC, I think this is absolutely
the wrong way to go about solving this problem. This -p option has two failure modes: the first is that a referenced file simply doesn't exist in the supplied parent, and the receive fails hard partway through. The second failure mode is more subtle and not checked for in any way, which is that the referenced file exists, but doesn't contain the same data as the original parent subvolume on the send side. In that case, you will end up with silent corruption of files.

This is a NAK from me.

I believe that the correct way of dealing with the issue of
non-transitive sends is twofold:

1) When sending a subvolume which already has a received-uuid field, preserve that field in the receiving side rather than overwriting it.

2) When specifying the parent to the receiving side, send both the uuid and received-uuid fields, and search against both of these in the subvolumes on the receiving side to find the parent. This will (I think) probably involve a bump of the stream format, and adding the received-uuid to the UUID tree.

Part 1) means that, writing (U, R) for UUID and Received-UUID, we
currently have the following situation:

Subvol A on source side: (A, -)
Send this to A' on target side: (A', A)
Send this back to A'' on source side: (A'', A')

and would change this behaviour to:

Subvol A on source side: (A, -)
Send this to A' on target side: (A', A)
Send this back to A'' on source side: (A'', A) <-- Note the A here, not A'

More later, when I've had a little time to play with things and
think through the semantics properly.

Hugo.

Quoted 82 lines
---
 cmds-receive.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)
cmds-receive.c +26 -8
index b7cf3f9..391d281 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
 	char *root_path;
 	char *dest_dir_path; /* relative to root_path */
 	char *full_subvol_path;
+	char *explicit_parent_path;
 	int dest_dir_chroot;
 
 	struct subvol_info *cur_subvol;
@@ -220,20 +221,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		fprintf(stderr, "receiving snapshot %s uuid=%s, "
 				"ctransid=%llu ", path, uuid_str,
 				r->cur_subvol->stransid);
-		uuid_unparse(parent_uuid, uuid_str);
-		fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-				uuid_str, parent_ctransid);
 	}
 
 	memset(&args_v2, 0, sizeof(args_v2));
 	strncpy_null(args_v2.name, path);
 
-	parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-			parent_ctransid, NULL, subvol_search_by_received_uuid);
-	if (!parent_subvol) {
+	if (r->explicit_parent_path) {
+		if (g_verbose) {
+			fprintf(stderr, "using explicit parent %s\n",
+					r->explicit_parent_path);
+		}
+		parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+			0, r->explicit_parent_path, subvol_search_by_path);
+	} else {
+		if (g_verbose) {
+			uuid_unparse(parent_uuid, uuid_str);
+			fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+					uuid_str, parent_ctransid);
+		}
 		parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-				parent_ctransid, NULL, subvol_search_by_uuid);
+			parent_ctransid, NULL, subvol_search_by_received_uuid);
+		if (!parent_subvol) {
+			parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+					parent_ctransid, NULL, subvol_search_by_uuid);
+		}
 	}
+
 	if (!parent_subvol) {
 		ret = -ENOENT;
 		fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -962,11 +975,14 @@ int cmd_receive(int argc, char **argv)
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+		c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
 		if (c < 0)
 			break;
 
 		switch (c) {
+		case 'p':
+			r.explicit_parent_path = optarg;
+			break;
 		case 'v':
 			g_verbose++;
 			break;
@@ -1028,6 +1044,8 @@ const char * const cmd_receive_usage[] = {
 	"                 in the data stream. Without this option,",
 	"                 the receiver terminates only if an error",
 	"                 is recognized or on EOF.",
+	"-p <subvol>      Disables the automatic searching for parents",
+	"                 if incremental streams are received.",
 	"-C|--chroot      confine the process to <mount> using chroot",
 	"--max-errors <N> Terminate as soon as N errors happened while",
 	"                 processing commands from the send stream.",
Signature · 4 lines
Hugo Mills             | Questions are a burden, and answers a prison for
hugo@... carfax.org.uk | oneself
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                          The Prisoner
author
#2 lore raw

Hi,

Quoted 3 lines

Subvol A on source side: (A, -)
Send this to A' on target side: (A', A)
Send this back to A'' on source side: (A'', A) <-- Note the A here, not A'

I also think your approach is the real solution to the problem, but as some pointed out on IRC this changes the behaviour of btrfs receive and will break things for someone. When I asked whose obscure workflow does it break nobody could come up with any reasonable example whereas sending snapshots back and forth seems to be the major usecase for btrfs receive and it's impossible to for example restore snapshots with current implementation.

I know that -p may fail horribly but the idea was to have *option* to replace parent lookup logic and instead of relying on UUID-s simply have it specified by userspace application.

Quoted 2 lines

More later, when I've had a little time to play with things and
think through the semantics properly.

You want to give it a try?

Signature · 4 lines
Lauri Võsandi
tel: +372 53329412
e-mail: [email protected]
blog: http://lauri.vosandi.com/
#3 lore raw
Quoted 12 lines · lauri
Quoted 3 lines

Subvol A on source side: (A, -)
Send this to A' on target side: (A', A)
Send this back to A'' on source side: (A'', A) <-- Note the A here, not A'

I also think your approach is the real solution to the problem, but as some pointed out on IRC this changes the behaviour of btrfs receive and will break things for someone. When I asked whose obscure workflow does it break nobody could come up with any reasonable example whereas sending snapshots back and forth seems to be the major usecase for btrfs receive and it's impossible to for example restore snapshots with current implementation.

Quoted 3 lines

I know that -p may fail horribly but the idea was to have *option* to replace parent lookup logic and instead of relying on UUID-s simply have it specified by userspace application.

My argument is that having this option present will mean that
people will use it. Getting send/receive right is hard enough as it is without giving someone an option which can completely screw things up in *most* cases. If you want an analogy, it's a bit like giving someone a drum of nerve toxin to get rid of the fly in their kitchen, rather than a fly swatter. (Yes, a bad analogy is like a leaky screwdriver, and this one's engaging in a bit of hyperbole, but I hope it gets my point across.)

Quoted 4 lines
Quoted 2 lines

More later, when I've had a little time to play with things and
think through the semantics properly.

You want to give it a try?

Finally, I managed to spend some time with it. What follows is long
and detailed, but I hope it's clear, and rigorous. I'd like someone with a mathematical turn of mind to give this a good kicking, because there may be some errors in it, but I think it's right.

A subvolume may be described by the tuple (u, r, p), where u is the
UUID of the subvolume itself, r is the "received UUID" set to indicate the original send source of the subvolume, and p is the UUID of the parent subvolume, set when a snapshot is taken. We indicate a read-only snapshot with a * suffix. The location of a subvolume is shown with the filesystem prefixed to it: m(u, r, p).

We use capital letters (A, B, C, ...) to indicate subvolumes, and
S, T to indicate the "source" and "target" filesystems. Note that send operations will sometimes go from T to S in the examples that follow.

There are four functions we need to consider: two for snapshots and
two for send operations. The current definitions of the functions are:

Ordinary snapshot:
Snap(m(u1, #, #)) -> m(u2, -, u1)

Read-only snapshot:
SnapR(m(u1, #, #)) -> m(u2, -, u1)*

Full send:
Send(m1(u1, #, #)*, m2) -> m2(u2, u1, -)*

Incremental send:
SendP(m1(u2, #, p1)*, m1(u1, r1, p1)*, m2) -> m2(u3, u2, p2)*
precondition: m2(p2, u1, #)* exists for some p2.

where # means "don't care" in all cases.

This latter function sends m1(u2, #, p1)* to m2, using m1(u1, r1,
p1)* as the parameter to the -p command option (e.g. the reference point for the incremental stream). The precondition is necessary to ensure that the reference subvolumes on each side are identical. The m1(u1, r1, p1)* is the same subvolume data as m2(p2, u1, #)* because the only way that the second field of a subvolume can be set is via one of the Send functions, which don't modify the subvolume data (or, through the read-only property, allow it to be modified).

Now, there are two use cases we want to support: 1) restore from
backups and continue with incrementals; 2) bidirectional synchronisation, where incremental sends are used to shift the state of a subvolume from one machine to another and back again. These are very similar cases -- the former case simply uses a full send from T to S, rather than the incremental send used in the latter case.

First use case: restore from backups
------------------------------------

The first use case goes something like this:

1) Create a subvolume, S(A, -, -) as the working subvol.
2) Make a backup to T:

 a  SnapR(S(A, -, -))                   ->  S(B, -, A)*
 b  Send(S(B, -, A)*, T)                ->               T(C, B, -)*
3) Subvol A is modified
4) Make an incremental backup to T
 a  SnapR(S(A, -, -))                   ->  S(D, -, A)*
 b  SendP(S(D, -, A)*, S(B, -, A)*, T)  ->               T(E, D, C)*
 (repeat 3, 4 ad libitum)
5) A meteorite strike destroys S, and it is replaced with a blank machine
6) Restore the backup, making a new working subvol G
 a  Send(T(E, D, C)*, S)                ->  S(F, E, -)*
 b  Snap(S(F, E, -)*)                   ->  S(G, -, F)
7) The new working subvol S(G, -, F) is modifed
8) Make an incremental backup to T
 a  SnapR(S(G, -, F))                   ->  S(H, -, G)*
 b  SendP(S(H, -, G)*, S(F, E, -)*, T)  ->               T(I, H, E)*

Now, with the current implementation, 8b will fail for two reasons:

- We can't immediately tell that S(H, -, G)* and S(F, E, -)* are related, and
- T(#, F, #)* doesn't exist.

However, it should be allowed to succeed, because there's a chain of snapshots connecting S(F, E, -)* to S(H, -, G)*, so one is a direct ancestor of the other. Further, a subvolume equivalent to E exists on both S and T after step 7 -- E and F respectively. The subvolume S(F, E, -)* has enough metadata to be able to correctly identify that it's equivalent to T(E, D, C)*, but the precondition on the SendP function doesn't support it.

Second use case: bidirectional synchronisation
----------------------------------------------

Let's leave that there for the moment and look at the second
use-case:

1) Create a subvolume, S(A, -, -) as the working subvol.
2) Make a backup to T:

 a  SnapR(S(A, -, -))                   ->  S(B, -, A)*
 b  Send(S(B, -, A)*, T)                ->               T(C, B, -)*
3) Subvol A is modified
4) Make an incremental backup to T
 a  SnapR(S(A, -, -))                   ->  S(D, -, A)*
 b  SendP(S(D, -, A)*, S(B, -, A)*, T)  ->               T(E, D, C)*
 (repeat 3, 4 ad libitum)
5) Make a working copy on T
    Snap(T(E, D, C))                    ->               T(F, -, E)
6) Subvol F is modified
7) Send the changes back to S
 a  SnapR(T(F, -, E))                   ->               T(G, F, -)*
 b  SendP(T(G, F, -)*, T(E, D, C)*, S)  ->  S(H, G, D)*

Again, with the current implementation, this will fail -- at step 7b this time. As before, there's two reasons why:

- We can't immediately tell that T(G, F, -)* and T(E, D, C)* are related, and
- T(#, E, #)* doesn't exist.

As before, it should be allowed, because there's a chain of snapshots connecting T(E, D, C)* to T(G, F, -)*, so the former is a direct ancestor of the latter, and because S(D, -, A)* is a duplicate of the reference subvolume T(E, D, C)*.

Now, in the first case we've got S(F, E, -)* on the source side,
and T(E, D, C)* on the target side. In the second case, we have T(E, D, C)* on the source and S(D, -, A)* on the target. In both cases, we're looking for a subvolume on the target whose UUID matches the Received UUID of the source side's reference subvolume. This contrasts with the original algorithm, where we only look for a subvolume on the target whose Received UUID matches the UUID of the source side's reference.

What to do about it
-------------------

So, to support these two use-cases, our SendP function needs to
look like this:

SendP(m1(u2, #, p1)*, m1(u1, r1, p1)*, m2) -> m2(u3, u2, p2)*
precondition: m2(p2, u1, #)* exists for some p2,
or
SendP(m1(u2, #, p1)*, m1(u1, r1, #)*, m2) -> m2(u3, u2, r1)*

            precondition: m2(r1, #, #)* exists,
	              and m1(p1, -, u1)# exists.

or, in other words, the first form is where the reference subvolume has been sent to the target machine; the second form is where the reference subvolume has come *from* the target machine. The other term in the precondition simply verifies that there has been a chain of snapshots on the local machine that connects the subject of the send back to the reference subvolume. So, finally, we end up with the final form of the function as:

SendP(m1(u2, #, p1)*, m1(u1, r1, p2)*, m2) -> m2(u3, u2, p3)*

          preconditions:
              (chain rule)     m1(p1, -, u1)# exists or p1 == p2
          and (reference rule) m2(p2, u1, #)* exists or m2(r1, #, #)* exists

The chain rule can (must) be done on the send side. The reference rule means that we need both r1 and u1 to be available on the receiving side. I haven't checked, but this will probably involve a change to the stream format to send r1.

Other things
------------

As mentioned above, there's a use-case where we do something like
this:

1) S(A, -, -)
2) Send a backup

 a  SnapR(S(A, -, -))               ->  S(B, -, A)*
 b  Send(S(B, -, A)*, T)            ->               T(C, B, -)*
3) Roll back to the state of B
    Snap(S(B, -, A))                ->  S(D, -, B)
4) S(D, -, B) is used for a while
5) Send a backup
 a  SnapR(S(D, -, B))               ->  S(E, -, D)*
 b  SendP(S(E, -, D)*, S(B, -, A)*  ->               T(F, E, C)*

This is supported with the above infrastructure, but it's not going to work if there's more snapshots in between stages 3 and 4 (because the "parent" UUID is rotated out each time). We would need a generalisation of the "chain rule", so that we look for a chain of snapshots of _any_ length from the reference to the subject. However, this would either require us to keep a full history of all ancestors with each snapshot, or to find such a path in the tree of snapshot history when we run the send (and to require that all snapshots in that sequence still exist). Neither of these is a particularly attractive proposition, and I would suggest not trying to support that use case right now.

The other thing that we don't support at all is merging changes
from divergent snapshots. i.e. snapshot A to B and C, modify both B and C, and then later try to merge B and C into D. (Possibly with additional sends between all of those stages). This gets into issues of semantic merging. I feel entirely justified in telling anyone who wants to implement that kind of workflow that it's out of scope for send/receive, and referring them to a distributed revision control system like git.

Finally, there's an interesting use case of trying to rotate the
bidirectional sync through several machines in turn (S, T, Q, R and back to S). I haven't had time to look at that one in detail, and I probably won't do so, but at least the analysis of it should be tractable with the above notation, if anyone does feel moved to look into it.

Hugo.

Signature · 4 lines
Hugo Mills             | Once is happenstance; twice is coincidence; three
hugo@... carfax.org.uk | times is enemy action.
http://carfax.org.uk/  |
PGP: E2AB1DE4          |
author
#4 lore raw

Hi,

Quoted 4 lines

My argument is that having this option present will mean that people will use it. Getting send/receive right is hard enough as it is without giving someone an option which can completely screw things up in *most* cases.

I agree that providing workarounds like this is not nice, however considering the users who are already stuck with the UUID-s of current architecture should be provided a way to get back on the track with better implementation :)

Quoted 4 lines

The chain rule can (must) be done on the send side. The reference rule means that we need both r1 and u1 to be available on the receiving side. I haven't checked, but this will probably involve a change to the stream format to send r1.

I've also concluded the real solution involves bundling both subvolume UUID and received UUID with the btrfs stream. This implies btrfs send stream format change and if I've understood correctly involves kernel changes as btrfs stream is generated in kernelspace.

Quoted 8 lines

The other thing that we don't support at all is merging changes
from divergent snapshots. i.e. snapshot A to B and C, modify both B and C, and then later try to merge B and C into D. (Possibly with additional sends between all of those stages). This gets into issues of semantic merging. I feel entirely justified in telling anyone who wants to implement that kind of workflow that it's out of scope for send/receive, and referring them to a distributed revision control system like git.

I've never suggested merges, when I mentioned Git-like workflow I was referring to simply push/pull. Obviously implementing merge is out of the scope of any filesystem ;)

Signature · 4 lines
Lauri Võsandi
tel: +372 53329412
e-mail: [email protected]
blog: http://lauri.vosandi.com/
author
#5 lore raw
In reply to Hugo Mills

Even if the UUID stuff gets fixed there will be plenty of machines which are unable to adapt the new approach due to conflicting received_uuid values and having the option to specify -p would help them to work around the issue to adapt to new received_uuid scheme

#6 lore raw
In reply to Lauri Võsandi
Quoted 34 lines · Lauri Võsandi

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead.

---
 cmds-receive.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)
cmds-receive.c +1 -3
index b7cf3f9..391d281 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
 	char *root_path;
 	char *dest_dir_path; /* relative to root_path */
 	char *full_subvol_path;
+	char *explicit_parent_path;
 	int dest_dir_chroot;
 
 	struct subvol_info *cur_subvol;
@@ -220,20 +221,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		fprintf(stderr, "receiving snapshot %s uuid=%s, "
 				"ctransid=%llu ", path, uuid_str,
 				r->cur_subvol->stransid);
-		uuid_unparse(parent_uuid, uuid_str);
-		fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-				uuid_str, parent_ctransid);
 	}
 
 	memset(&args_v2, 0, sizeof(args_v2));
 	strncpy_null(args_v2.name, path);
 

cmds-receive.c static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid, · line 235

Quoted 10 lines
cmds-receive.c +7 -3
-	parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-			parent_ctransid, NULL, subvol_search_by_received_uuid);
-	if (!parent_subvol) {
+	if (r->explicit_parent_path) {
+		if (g_verbose) {
+			fprintf(stderr, "using explicit parent %s\n",
+					r->explicit_parent_path);
+		}
+		parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+			0, r->explicit_parent_path, subvol_search_by_path);

This won't work if you receive more than one snapshot, each one derived from the previous one ("btrfs send -e snap1 snap2 snap3 snap4"). You would always use the first one as the parent, not the predecessor. That's implemented differently for the -p option in
git://git.kernel.org/pub/scm/linux/kernel/git/arne/far-progs.git

cmds-receive.c static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid, · line 247

Quoted 13 lines
cmds-receive.c +11 -1
+	} else {
+		if (g_verbose) {
+			uuid_unparse(parent_uuid, uuid_str);
+			fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+					uuid_str, parent_ctransid);
+		}
 		parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-				parent_ctransid, NULL, subvol_search_by_uuid);
+			parent_ctransid, NULL, subvol_search_by_received_uuid);
+		if (!parent_subvol) {
+			parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+					parent_ctransid, NULL, subvol_search_by_uuid);
+		}

This used to be a search for the received_uuid only. Why is this code changed like this in the branch that is executed when -p is not specified, in a patch that has the goal to add -p with new functionality if -p is specified?

Quoted 31 lines
cmds-receive.c +7 -1
 	}
+
 	if (!parent_subvol) {
 		ret = -ENOENT;
 		fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -962,11 +975,14 @@ int cmd_receive(int argc, char **argv)
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+		c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
 		if (c < 0)
 			break;
 
 		switch (c) {
+		case 'p':
+			r.explicit_parent_path = optarg;
+			break;
 		case 'v':
 			g_verbose++;
 			break;
@@ -1028,6 +1044,8 @@ const char * const cmd_receive_usage[] = {
 	"                 in the data stream. Without this option,",
 	"                 the receiver terminates only if an error",
 	"                 is recognized or on EOF.",
+	"-p <subvol>      Disables the automatic searching for parents",
+	"                 if incremental streams are received.",
 	"-C|--chroot      confine the process to <mount> using chroot",
 	"--max-errors <N> Terminate as soon as N errors happened while",
 	"                 processing commands from the send stream.",
author
#7 lore raw

Hi,

Quoted 5 lines

This won't work if you receive more than one snapshot, each one derived from the previous one ("btrfs send -e snap1 snap2 snap3 snap4"). You would always use the first one as the parent, not the predecessor. That's implemented differently for the -p option in
git://git.kernel.org/pub/scm/linux/kernel/git/arne/far-progs.git

I'll take another look on the multi-subvolume stream mode.

Quoted 4 lines

This used to be a search for the received_uuid only. Why is this code changed like this in the branch that is executed when -p is not specified, in a patch that has the goal to add -p with new functionality if -p is specified?

Many things have changed since far-progs, take a look in the original source I used as basis for my patch:

https://github.com/kdave/btrfs-progs/blob/1b7dd327f43777bcdd217a0500e6bda78128a290/cmds-receive.c#L233

I am pretty sure lookup logic isn't affected if -p is not used.

2015-04-27 9:28 GMT+02:00 Stefan Behrens <[email protected]>:

Quoted 105 lines · Lauri Võsandi
Quoted 34 lines

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead.

---
 cmds-receive.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)
cmds-receive.c +1 -3
index b7cf3f9..391d281 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
      char *root_path;
      char *dest_dir_path; /* relative to root_path */
      char *full_subvol_path;
+     char *explicit_parent_path;
      int dest_dir_chroot;
 
      struct subvol_info *cur_subvol;
@@ -220,20 +221,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
              fprintf(stderr, "receiving snapshot %s uuid=%s, "
                              "ctransid=%llu ", path, uuid_str,
                              r->cur_subvol->stransid);
-             uuid_unparse(parent_uuid, uuid_str);
-             fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-                             uuid_str, parent_ctransid);
      }
 
      memset(&args_v2, 0, sizeof(args_v2));
      strncpy_null(args_v2.name, path);
 

cmds-receive.c static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid, · line 235

Quoted 10 lines
cmds-receive.c +7 -3
-     parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-                     parent_ctransid, NULL, subvol_search_by_received_uuid);
-     if (!parent_subvol) {
+     if (r->explicit_parent_path) {
+             if (g_verbose) {
+                     fprintf(stderr, "using explicit parent %s\n",
+                                     r->explicit_parent_path);
+             }
+             parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+                     0, r->explicit_parent_path, subvol_search_by_path);

This won't work if you receive more than one snapshot, each one derived from the previous one ("btrfs send -e snap1 snap2 snap3 snap4"). You would always use the first one as the parent, not the predecessor. That's implemented differently for the -p option in
git://git.kernel.org/pub/scm/linux/kernel/git/arne/far-progs.git

cmds-receive.c static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid, · line 247

Quoted 13 lines
cmds-receive.c +11 -1
+     } else {
+             if (g_verbose) {
+                     uuid_unparse(parent_uuid, uuid_str);
+                     fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+                                     uuid_str, parent_ctransid);
+             }
              parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-                             parent_ctransid, NULL, subvol_search_by_uuid);
+                     parent_ctransid, NULL, subvol_search_by_received_uuid);
+             if (!parent_subvol) {
+                     parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+                                     parent_ctransid, NULL, subvol_search_by_uuid);
+             }

This used to be a search for the received_uuid only. Why is this code changed like this in the branch that is executed when -p is not specified, in a patch that has the goal to add -p with new functionality if -p is specified?

Quoted 31 lines
cmds-receive.c +7 -1
      }
+
      if (!parent_subvol) {
              ret = -ENOENT;
              fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -962,11 +975,14 @@ int cmd_receive(int argc, char **argv)
                      { NULL, 0, NULL, 0 }
              };
 
-             c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+             c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
              if (c < 0)
                      break;
 
              switch (c) {
+             case 'p':
+                     r.explicit_parent_path = optarg;
+                     break;
              case 'v':
                      g_verbose++;
                      break;
@@ -1028,6 +1044,8 @@ const char * const cmd_receive_usage[] = {
      "                 in the data stream. Without this option,",
      "                 the receiver terminates only if an error",
      "                 is recognized or on EOF.",
+     "-p <subvol>      Disables the automatic searching for parents",
+     "                 if incremental streams are received.",
      "-C|--chroot      confine the process to <mount> using chroot",
      "--max-errors <N> Terminate as soon as N errors happened while",
      "                 processing commands from the send stream.",
Signature · 4 lines
Lauri Võsandi
tel: +372 53329412
e-mail: [email protected]
blog: http://lauri.vosandi.com/
#8 lore raw
Quoted 15 lines · lauri

Hi,

Quoted 5 lines

This won't work if you receive more than one snapshot, each one derived from the previous one ("btrfs send -e snap1 snap2 snap3 snap4"). You would always use the first one as the parent, not the predecessor. That's implemented differently for the -p option in
git://git.kernel.org/pub/scm/linux/kernel/git/arne/far-progs.git

I'll take another look on the multi-subvolume stream mode.

Quoted 4 lines

This used to be a search for the received_uuid only. Why is this code changed like this in the branch that is executed when -p is not specified, in a patch that has the goal to add -p with new functionality if -p is specified?

Quoted 6 lines

Many things have changed since far-progs, take a look in the original source I used as basis for my patch:

https://github.com/kdave/btrfs-progs/blob/1b7dd327f43777bcdd217a0500e6bda78128a290/cmds-receive.c#L233

I am pretty sure lookup logic isn't affected if -p is not used.

You are right! My bad, not looking precisely enough. Before the patch, the code did the two lookups as well.

But the first issue is real. One possible fix is to update explicit_parent_path after each received snapshot.

Quoted 106 lines

2015-04-27 9:28 GMT+02:00 Stefan Behrens <[email protected]>:

Quoted 104 lines · Lauri Võsandi
Quoted 34 lines

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead.

---
  cmds-receive.c | 34 ++++++++++++++++++++++++++--------
  1 file changed, 26 insertions(+), 8 deletions(-)
cmds-receive.c +1 -3
index b7cf3f9..391d281 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
       char *root_path;
       char *dest_dir_path; /* relative to root_path */
       char *full_subvol_path;
+     char *explicit_parent_path;
       int dest_dir_chroot;
 
       struct subvol_info *cur_subvol;
@@ -220,20 +221,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
               fprintf(stderr, "receiving snapshot %s uuid=%s, "
                               "ctransid=%llu ", path, uuid_str,
                               r->cur_subvol->stransid);
-             uuid_unparse(parent_uuid, uuid_str);
-             fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-                             uuid_str, parent_ctransid);
       }
 
       memset(&args_v2, 0, sizeof(args_v2));
       strncpy_null(args_v2.name, path);
 

cmds-receive.c static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid, · line 235

Quoted 10 lines
cmds-receive.c +7 -3
-     parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-                     parent_ctransid, NULL, subvol_search_by_received_uuid);
-     if (!parent_subvol) {
+     if (r->explicit_parent_path) {
+             if (g_verbose) {
+                     fprintf(stderr, "using explicit parent %s\n",
+                                     r->explicit_parent_path);
+             }
+             parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+                     0, r->explicit_parent_path, subvol_search_by_path);

This won't work if you receive more than one snapshot, each one derived from the previous one ("btrfs send -e snap1 snap2 snap3 snap4"). You would always use the first one as the parent, not the predecessor. That's implemented differently for the -p option in
git://git.kernel.org/pub/scm/linux/kernel/git/arne/far-progs.git

cmds-receive.c static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid, · line 247

Quoted 13 lines
cmds-receive.c +11 -1
+     } else {
+             if (g_verbose) {
+                     uuid_unparse(parent_uuid, uuid_str);
+                     fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+                                     uuid_str, parent_ctransid);
+             }
               parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-                             parent_ctransid, NULL, subvol_search_by_uuid);
+                     parent_ctransid, NULL, subvol_search_by_received_uuid);
+             if (!parent_subvol) {
+                     parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+                                     parent_ctransid, NULL, subvol_search_by_uuid);
+             }

This used to be a search for the received_uuid only. Why is this code changed like this in the branch that is executed when -p is not specified, in a patch that has the goal to add -p with new functionality if -p is specified?

Quoted 31 lines
cmds-receive.c +7 -1
       }
+
       if (!parent_subvol) {
               ret = -ENOENT;
               fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -962,11 +975,14 @@ int cmd_receive(int argc, char **argv)
                       { NULL, 0, NULL, 0 }
               };
 
-             c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+             c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
               if (c < 0)
                       break;
 
               switch (c) {
+             case 'p':
+                     r.explicit_parent_path = optarg;
+                     break;
               case 'v':
                       g_verbose++;
                       break;
@@ -1028,6 +1044,8 @@ const char * const cmd_receive_usage[] = {
       "                 in the data stream. Without this option,",
       "                 the receiver terminates only if an error",
       "                 is recognized or on EOF.",
+     "-p <subvol>      Disables the automatic searching for parents",
+     "                 if incremental streams are received.",
       "-C|--chroot      confine the process to <mount> using chroot",
       "--max-errors <N> Terminate as soon as N errors happened while",
       "                 processing commands from the send stream.",
#9 lore raw

PATCH v2 1/1 This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead. Works also with multiple incremental snapshots.

In reply to Lauri Võsandi
---
 cmds-receive.c | 50 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 8 deletions(-)
cmds-receive.c +42 -8
index b7cf3f9..21a6f56 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
 	char *root_path;
 	char *dest_dir_path; /* relative to root_path */
 	char *full_subvol_path;
+	char *explicit_parent_path;
 	int dest_dir_chroot;
 
 	struct subvol_info *cur_subvol;
@@ -152,6 +153,12 @@ static int process_subvol(const char *path, const u8 *uuid, u64 ctransid,
 	if (ret < 0)
 		goto out;
 
+	if (r->explicit_parent_path) {
+		free(r->explicit_parent_path);
+		r->explicit_parent_path = NULL;
+		fprintf(stderr, "WARNING: Receiving full snapshot, no need to specify explicit parent!\n");
+	}
+
 	r->cur_subvol = calloc(1, sizeof(*r->cur_subvol));
 
 	if (strlen(r->dest_dir_path) == 0)
@@ -220,20 +227,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		fprintf(stderr, "receiving snapshot %s uuid=%s, "
 				"ctransid=%llu ", path, uuid_str,
 				r->cur_subvol->stransid);
-		uuid_unparse(parent_uuid, uuid_str);
-		fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-				uuid_str, parent_ctransid);
 	}
 
 	memset(&args_v2, 0, sizeof(args_v2));
 	strncpy_null(args_v2.name, path);
 
-	parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-			parent_ctransid, NULL, subvol_search_by_received_uuid);
-	if (!parent_subvol) {
+	if (r->explicit_parent_path) {
+		if (g_verbose) {
+			fprintf(stderr, "using explicit parent %s\n",
+					r->explicit_parent_path);
+		}
+		parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+			0, r->explicit_parent_path, subvol_search_by_path);
+	} else {
+		if (g_verbose) {
+			uuid_unparse(parent_uuid, uuid_str);
+			fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+					uuid_str, parent_ctransid);
+		}
 		parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-				parent_ctransid, NULL, subvol_search_by_uuid);
+			parent_ctransid, NULL, subvol_search_by_received_uuid);
+		if (!parent_subvol) {
+			parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+					parent_ctransid, NULL, subvol_search_by_uuid);
+		}
 	}
+
 	if (!parent_subvol) {
 		ret = -ENOENT;
 		fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -276,6 +295,12 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		goto out;
 	}
 
+	if (r->explicit_parent_path) {
+		free(r->explicit_parent_path);
+		r->explicit_parent_path = strdup(r->cur_subvol->path);
+		printf("New explicit parent is %s\n", r->explicit_parent_path);
+	}
+
 out:
 	if (parent_subvol) {
 		free(parent_subvol->path);
@@ -922,6 +947,10 @@ out:
 	r->full_subvol_path = NULL;
 	r->dest_dir_path = NULL;
 	free(dest_dir_full_path);
+	if (r->explicit_parent_path) {
+		free(r->explicit_parent_path);
+		r->explicit_parent_path = NULL;
+	}
 	if (r->cur_subvol) {
 		free(r->cur_subvol->path);
 		free(r->cur_subvol);
@@ -962,11 +991,14 @@ int cmd_receive(int argc, char **argv)
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+		c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
 		if (c < 0)
 			break;
 
 		switch (c) {
+		case 'p':
+			r.explicit_parent_path = strdup(optarg);
+			break;
 		case 'v':
 			g_verbose++;
 			break;
@@ -1028,6 +1060,8 @@ const char * const cmd_receive_usage[] = {
 	"                 in the data stream. Without this option,",
 	"                 the receiver terminates only if an error",
 	"                 is recognized or on EOF.",
+	"-p <subvol>      Disables the automatic searching for parents",
+	"                 if incremental streams are received.",
 	"-C|--chroot      confine the process to <mount> using chroot",
 	"--max-errors <N> Terminate as soon as N errors happened while",
 	"                 processing commands from the send stream.",
Signature · 1 line
1.9.1
#10 lore raw

PATCH v2 1/1 btrfs-progs: receive explicit parent support

In reply to Lauri Võsandi

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead. Works also with multiple incremental snapshots.

---
 cmds-receive.c | 50 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 8 deletions(-)
cmds-receive.c +42 -8
index b7cf3f9..21a6f56 100644
@@ -61,6 +61,7 @@ struct btrfs_receive
 	char *root_path;
 	char *dest_dir_path; /* relative to root_path */
 	char *full_subvol_path;
+	char *explicit_parent_path;
 	int dest_dir_chroot;
 
 	struct subvol_info *cur_subvol;
@@ -152,6 +153,12 @@ static int process_subvol(const char *path, const u8 *uuid, u64 ctransid,
 	if (ret < 0)
 		goto out;
 
+	if (r->explicit_parent_path) {
+		free(r->explicit_parent_path);
+		r->explicit_parent_path = NULL;
+		fprintf(stderr, "WARNING: Receiving full snapshot, no need to specify explicit parent!\n");
+	}
+
 	r->cur_subvol = calloc(1, sizeof(*r->cur_subvol));
 
 	if (strlen(r->dest_dir_path) == 0)
@@ -220,20 +227,32 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		fprintf(stderr, "receiving snapshot %s uuid=%s, "
 				"ctransid=%llu ", path, uuid_str,
 				r->cur_subvol->stransid);
-		uuid_unparse(parent_uuid, uuid_str);
-		fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
-				uuid_str, parent_ctransid);
 	}
 
 	memset(&args_v2, 0, sizeof(args_v2));
 	strncpy_null(args_v2.name, path);
 
-	parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-			parent_ctransid, NULL, subvol_search_by_received_uuid);
-	if (!parent_subvol) {
+	if (r->explicit_parent_path) {
+		if (g_verbose) {
+			fprintf(stderr, "using explicit parent %s\n",
+					r->explicit_parent_path);
+		}
+		parent_subvol = subvol_uuid_search(&r->sus, 0, NULL,
+			0, r->explicit_parent_path, subvol_search_by_path);
+	} else {
+		if (g_verbose) {
+			uuid_unparse(parent_uuid, uuid_str);
+			fprintf(stderr, "parent_uuid=%s, parent_ctransid=%llu\n",
+					uuid_str, parent_ctransid);
+		}
 		parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
-				parent_ctransid, NULL, subvol_search_by_uuid);
+			parent_ctransid, NULL, subvol_search_by_received_uuid);
+		if (!parent_subvol) {
+			parent_subvol = subvol_uuid_search(&r->sus, 0, parent_uuid,
+					parent_ctransid, NULL, subvol_search_by_uuid);
+		}
 	}
+
 	if (!parent_subvol) {
 		ret = -ENOENT;
 		fprintf(stderr, "ERROR: could not find parent subvolume\n");
@@ -276,6 +295,12 @@ static int process_snapshot(const char *path, const u8 *uuid, u64 ctransid,
 		goto out;
 	}
 
+	if (r->explicit_parent_path) {
+		free(r->explicit_parent_path);
+		r->explicit_parent_path = strdup(r->cur_subvol->path);
+		printf("New explicit parent is %s\n", r->explicit_parent_path);
+	}
+
 out:
 	if (parent_subvol) {
 		free(parent_subvol->path);
@@ -922,6 +947,10 @@ out:
 	r->full_subvol_path = NULL;
 	r->dest_dir_path = NULL;
 	free(dest_dir_full_path);
+	if (r->explicit_parent_path) {
+		free(r->explicit_parent_path);
+		r->explicit_parent_path = NULL;
+	}
 	if (r->cur_subvol) {
 		free(r->cur_subvol->path);
 		free(r->cur_subvol);
@@ -962,11 +991,14 @@ int cmd_receive(int argc, char **argv)
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "Cevf:", long_opts, NULL);
+		c = getopt_long(argc, argv, "Cevf:p:", long_opts, NULL);
 		if (c < 0)
 			break;
 
 		switch (c) {
+		case 'p':
+			r.explicit_parent_path = strdup(optarg);
+			break;
 		case 'v':
 			g_verbose++;
 			break;
@@ -1028,6 +1060,8 @@ const char * const cmd_receive_usage[] = {
 	"                 in the data stream. Without this option,",
 	"                 the receiver terminates only if an error",
 	"                 is recognized or on EOF.",
+	"-p <subvol>      Disables the automatic searching for parents",
+	"                 if incremental streams are received.",
 	"-C|--chroot      confine the process to <mount> using chroot",
 	"--max-errors <N> Terminate as soon as N errors happened while",
 	"                 processing commands from the send stream.",
Signature · 1 line
1.9.1
#11 lore raw
Quoted 5 lines · Lauri Võsandi

This patch adds command-line flag -p to btrfs receive which makes it possible to disable automatic parent search for incremental snapshots and use explicitly specified path instead. Works also with multiple incremental snapshots.

Can you please describe the usecase(s) in detail? I don't have a clear picture at the moment and given that there are objections I'd like to be sure that we can document the expected behaviour possibly with a list of "do not do this".