[PATCH] Btrfs: use received_uuid of parent during send

[PATCH] Btrfs: use received_uuid of parent during send

[Josef Bacik]

Neil Horman pointed out a problem where if he did something like this

receive A
snap A B
change B
send -p A B

and then on another box do

recieve A
receive B

the receive B would fail because we use the UUID of A for the clone sources for
B.  This makes sense most of the time because normally you are sending from the
original sources, not a received source.  However when you use a recieved subvol
its UUID is going to be something completely different, so if you then try to
receive the diff on a different volume it won't find the UUID because the new A
will be something else.  The only constant is the received uuid.  So instead
check to see if we have received_uuid set on the root, and if so use that as the
clone source, as btrfs receive looks for matches either in received_uuid or
uuid.  Thanks,

Reported-by: Neil Horman <nhorman@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 fs/btrfs/send.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index a1216f9..947d91c 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2328,8 +2328,12 @@ static int send_subvol_begin(struct send_ctx *sctx)
 	TLV_PUT_U64(sctx, BTRFS_SEND_A_CTRANSID,
 		    le64_to_cpu(sctx->send_root->root_item.ctransid));
 	if (parent_root) {
-		TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
-				sctx->parent_root->root_item.uuid);
+		if (!btrfs_is_empty_uuid(parent_root->root_item.received_uuid))
+			TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
+				     parent_root->root_item.received_uuid);
+		else
+			TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
+				     parent_root->root_item.uuid);
 		TLV_PUT_U64(sctx, BTRFS_SEND_A_CLONE_CTRANSID,
 			    le64_to_cpu(sctx->parent_root->root_item.ctransid));
 	}
@@ -4508,8 +4512,21 @@ verbose_printk("btrfs: send_clone offset=%llu, len=%d, clone_root=%llu, "
 	if (ret < 0)
 		goto out;
 
-	TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
-			clone_root->root->root_item.uuid);
+	/*
+	 * If the parent we're using has a received_uuid set then use that as
+	 * our clone source as that is what we will look for when doing a
+	 * receive.
+	 *
+	 * This covers the case that we create a snapshot off of a received
+	 * subvolume and then use that as the parent and try to receive on a
+	 * different host.
+	 */
+	if (!btrfs_is_empty_uuid(clone_root->root->root_item.received_uuid))
+		TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
+			     clone_root->root->root_item.received_uuid);
+	else
+		TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
+			     clone_root->root->root_item.uuid);
 	TLV_PUT_U64(sctx, BTRFS_SEND_A_CLONE_CTRANSID,
 		    le64_to_cpu(clone_root->root->root_item.ctransid));
 	TLV_PUT_PATH(sctx, BTRFS_SEND_A_CLONE_PATH, p);
-- 
1.8.3.1

Re: [PATCH] Btrfs: use received_uuid of parent during send

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 3746 bytes --]

On Thu, Jun 04, 2015 at 05:17:25PM -0400, Josef Bacik wrote:
> Neil Horman pointed out a problem where if he did something like this
> 
> receive A
> snap A B
> change B
> send -p A B
> 
> and then on another box do
> 
> recieve A
> receive B
> 
> the receive B would fail because we use the UUID of A for the clone sources for
> B.  This makes sense most of the time because normally you are sending from the
> original sources, not a received source.  However when you use a recieved subvol
> its UUID is going to be something completely different, so if you then try to
> receive the diff on a different volume it won't find the UUID because the new A
> will be something else.  The only constant is the received uuid.  So instead
> check to see if we have received_uuid set on the root, and if so use that as the
> clone source, as btrfs receive looks for matches either in received_uuid or
> uuid.  Thanks,

   While this deals with Neil's problem, there's a few other use-cases
that people have been asking for that (I think) it won't deal with.

   I think ultimately we should be sending all three of the parent
UUID, the parent's Received UUID (if it exists), and the parent's
Parent UUID. That would have to go in the FARv2 update, though.

   However, since this patch doesn't rule out the above happening at
some future date, and I think it'll do the job as described above,

Reviewed-by: Hugo Mills <hugo@carfax.org.uk>

for whatever little it's worth.

   Hugo.

> Reported-by: Neil Horman <nhorman@redhat.com>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
>  fs/btrfs/send.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index a1216f9..947d91c 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2328,8 +2328,12 @@ static int send_subvol_begin(struct send_ctx *sctx)
>  	TLV_PUT_U64(sctx, BTRFS_SEND_A_CTRANSID,
>  		    le64_to_cpu(sctx->send_root->root_item.ctransid));
>  	if (parent_root) {
> -		TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
> -				sctx->parent_root->root_item.uuid);
> +		if (!btrfs_is_empty_uuid(parent_root->root_item.received_uuid))
> +			TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
> +				     parent_root->root_item.received_uuid);
> +		else
> +			TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
> +				     parent_root->root_item.uuid);
>  		TLV_PUT_U64(sctx, BTRFS_SEND_A_CLONE_CTRANSID,
>  			    le64_to_cpu(sctx->parent_root->root_item.ctransid));
>  	}
> @@ -4508,8 +4512,21 @@ verbose_printk("btrfs: send_clone offset=%llu, len=%d, clone_root=%llu, "
>  	if (ret < 0)
>  		goto out;
>  
> -	TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
> -			clone_root->root->root_item.uuid);
> +	/*
> +	 * If the parent we're using has a received_uuid set then use that as
> +	 * our clone source as that is what we will look for when doing a
> +	 * receive.
> +	 *
> +	 * This covers the case that we create a snapshot off of a received
> +	 * subvolume and then use that as the parent and try to receive on a
> +	 * different host.
> +	 */
> +	if (!btrfs_is_empty_uuid(clone_root->root->root_item.received_uuid))
> +		TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
> +			     clone_root->root->root_item.received_uuid);
> +	else
> +		TLV_PUT_UUID(sctx, BTRFS_SEND_A_CLONE_UUID,
> +			     clone_root->root->root_item.uuid);
>  	TLV_PUT_U64(sctx, BTRFS_SEND_A_CLONE_CTRANSID,
>  		    le64_to_cpu(clone_root->root->root_item.ctransid));
>  	TLV_PUT_PATH(sctx, BTRFS_SEND_A_CLONE_PATH, p);

-- 
Hugo Mills             | ... one ping(1) to rule them all, and in the
hugo@... carfax.org.uk | darkness bind(2) them.
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                                Illiad

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Btrfs: use received_uuid of parent during send

[Josef Bacik]

On 06/11/2015 01:09 PM, Hugo Mills wrote:
> On Thu, Jun 04, 2015 at 05:17:25PM -0400, Josef Bacik wrote:
>> Neil Horman pointed out a problem where if he did something like this
>>
>> receive A
>> snap A B
>> change B
>> send -p A B
>>
>> and then on another box do
>>
>> recieve A
>> receive B
>>
>> the receive B would fail because we use the UUID of A for the clone sources for
>> B.  This makes sense most of the time because normally you are sending from the
>> original sources, not a received source.  However when you use a recieved subvol
>> its UUID is going to be something completely different, so if you then try to
>> receive the diff on a different volume it won't find the UUID because the new A
>> will be something else.  The only constant is the received uuid.  So instead
>> check to see if we have received_uuid set on the root, and if so use that as the
>> clone source, as btrfs receive looks for matches either in received_uuid or
>> uuid.  Thanks,
>
>     While this deals with Neil's problem, there's a few other use-cases
> that people have been asking for that (I think) it won't deal with.
>
>     I think ultimately we should be sending all three of the parent
> UUID, the parent's Received UUID (if it exists), and the parent's
> Parent UUID. That would have to go in the FARv2 update, though.
>
>     However, since this patch doesn't rule out the above happening at
> some future date, and I think it'll do the job as described above,
>

Yeah I'd like to send more information so we can better find the UUID 
we're looking for, but I think at least trying to keep a consistent UUID 
we carry around would be good.  Received UUID mostly accomplishes this, 
I'd like to know what other use cases aren't working so we can think 
about what we need to do for them.  Thanks,

Josef

Re: [PATCH] Btrfs: use received_uuid of parent during send

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 2470 bytes --]

On Thu, Jun 11, 2015 at 01:16:26PM -0400, Josef Bacik wrote:
> On 06/11/2015 01:09 PM, Hugo Mills wrote:
> >On Thu, Jun 04, 2015 at 05:17:25PM -0400, Josef Bacik wrote:
> >>Neil Horman pointed out a problem where if he did something like this
> >>
> >>receive A
> >>snap A B
> >>change B
> >>send -p A B
> >>
> >>and then on another box do
> >>
> >>recieve A
> >>receive B
> >>
> >>the receive B would fail because we use the UUID of A for the clone sources for
> >>B.  This makes sense most of the time because normally you are sending from the
> >>original sources, not a received source.  However when you use a recieved subvol
> >>its UUID is going to be something completely different, so if you then try to
> >>receive the diff on a different volume it won't find the UUID because the new A
> >>will be something else.  The only constant is the received uuid.  So instead
> >>check to see if we have received_uuid set on the root, and if so use that as the
> >>clone source, as btrfs receive looks for matches either in received_uuid or
> >>uuid.  Thanks,
> >
> >    While this deals with Neil's problem, there's a few other use-cases
> >that people have been asking for that (I think) it won't deal with.
> >
> >    I think ultimately we should be sending all three of the parent
> >UUID, the parent's Received UUID (if it exists), and the parent's
> >Parent UUID. That would have to go in the FARv2 update, though.
> >
> >    However, since this patch doesn't rule out the above happening at
> >some future date, and I think it'll do the job as described above,
> >
> 
> Yeah I'd like to send more information so we can better find the
> UUID we're looking for, but I think at least trying to keep a
> consistent UUID we carry around would be good.  Received UUID mostly
> accomplishes this, I'd like to know what other use cases aren't
> working so we can think about what we need to do for them.  Thanks,

   I did a write-up of this a while ago, in some detail (which is why
I weighed in here):

http://www.spinics.net/lists/linux-btrfs/msg44089.html

   I'm afraid it's rather long, but the tl;dr is the second indented
section in "What to do about it", with the notation described in
detail in the first few paragraphs.

   Hugo.

-- 
Hugo Mills             | ... one ping(1) to rule them all, and in the
hugo@... carfax.org.uk | darkness bind(2) them.
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                                Illiad

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]