[RFC] Btrfs: send, add calculate data size flag to allow for progress estimation
diff mbox

Message ID 20140406001838.GA9485@merlins.org
State Under Review
Headers show

Commit Message

Marc MERLIN April 6, 2014, 12:18 a.m. UTC
On Fri, Apr 04, 2014 at 04:20:41PM +0100, Filipe David Borba Manana wrote:
> This new send flag makes send calculate first the amount of new file data (in bytes)
> the send root has relatively to the parent root, or for the case of a non-incremental
> send, the total amount of file data we will send through the send stream. In other words,
> it computes the sum of the lengths of all write and clone operations that will be sent
> through the send stream.

Thanks for this patch, much appreciated.

A few questions:
1) I tried to apply to 3.14.0, and it all applied except one line:

I looked around and found nothing that looked similar enough.
Obviously it's an assert, so I can run without it, but my source being
very different from yours just made me want to check that this was most
likely ok to run with 3.14.0.

2) I saw the concerns about protocol incompatibility. Is it an issue if you run without -o
too?

3) Do you have a patch for btrfs-tools so that I can try it, or a git
tree of btrfs-tools with this that I should pull?

4) Can I run btrfs send -o snap1 snap2 >/dev/null to get quick stats
on the changes between the 2 snapshots, or is it still going to walk
through all the data blocks and send them to /dev/null afterwards?
 
Thanks,
Marc

Comments

Filipe Manana April 6, 2014, 4:57 p.m. UTC | #1
On Sun, Apr 6, 2014 at 1:18 AM, Marc MERLIN <marc@merlins.org> wrote:
> On Fri, Apr 04, 2014 at 04:20:41PM +0100, Filipe David Borba Manana wrote:
>> This new send flag makes send calculate first the amount of new file data (in bytes)
>> the send root has relatively to the parent root, or for the case of a non-incremental
>> send, the total amount of file data we will send through the send stream. In other words,
>> it computes the sum of the lengths of all write and clone operations that will be sent
>> through the send stream.
>
> Thanks for this patch, much appreciated.
>
> A few questions:
> 1) I tried to apply to 3.14.0, and it all applied except one line:
> --- fs/btrfs/send.c
> +++ fs/btrfs/send.c
> @@ -3091,6 +3121,8 @@
>         int ret;
>         u64 ancestor = 0;
>
> +       ASSERT(sctx->phase != SEND_PHASE_COMPUTE_DATA_SIZE);
> +
>         name = fs_path_alloc();
>         from_path = fs_path_alloc();
>         if (!name || !from_path) {
>
> I looked around and found nothing that looked similar enough.
> Obviously it's an assert, so I can run without it, but my source being
> very different from yours just made me want to check that this was most
> likely ok to run with 3.14.0.

This is based on top of btrfs-next plus previous send patches sent to
this list but not yet in btrfs-next.

>
> 2) I saw the concerns about protocol incompatibility. Is it an issue if you run without -o
> too?

No. See the discussion above. Using a send stream created with the new
flag won't work with an old btrfs receive. But to be able to get a
send stream with the new flag you need the patched btrfs receive
(unless you plan to write your own code to call the send ioctl
directly).

>
> 3) Do you have a patch for btrfs-tools so that I can try it, or a git
> tree of btrfs-tools with this that I should pull?

Yes, without it I couldn't test it and it would be pointless. There's
a patch in the list for btrfs-progs.  I've also pasted a link to it
(and this one too) in reply to the thread you started days ago.

>
> 4) Can I run btrfs send -o snap1 snap2 >/dev/null to get quick stats
> on the changes between the 2 snapshots, or is it still going to walk
> through all the data blocks and send them to /dev/null afterwards?

No, but it's doable and implies only extending the user space tools.
What is implemented is explained in the commit log for the btrfs-progs
patch.

Thanks

>
> Thanks,
> Marc
> --
> "A mouse is a device used to point at the xterm you want to type in" - A.S.R.
> Microsoft is to operating systems ....
>                                       .... what McDonalds is to gourmet cooking
> Home page: http://marc.merlins.org/                         | PGP 1024R/763BE901
Marc MERLIN April 6, 2014, 5:20 p.m. UTC | #2
On Sun, Apr 06, 2014 at 05:57:38PM +0100, Filipe David Manana wrote:
> > I looked around and found nothing that looked similar enough.
> > Obviously it's an assert, so I can run without it, but my source being
> > very different from yours just made me want to check that this was most
> > likely ok to run with 3.14.0.
> 
> This is based on top of btrfs-next plus previous send patches sent to
> this list but not yet in btrfs-next.
 
Right, I figured. It otherwise applied to 3.14, so I'm hopeful it'll
work since only the assert was missing.

> > 2) I saw the concerns about protocol incompatibility. Is it an issue if you run without -o
> > too?
> 
> No. See the discussion above. Using a send stream created with the new
> flag won't work with an old btrfs receive. But to be able to get a
> send stream with the new flag you need the patched btrfs receive
> (unless you plan to write your own code to call the send ioctl
> directly).

Thanks for confirming. Given that, it sounds more than fine to me.

> > 3) Do you have a patch for btrfs-tools so that I can try it, or a git
> > tree of btrfs-tools with this that I should pull?
> 
> Yes, without it I couldn't test it and it would be pointless. There's
> a patch in the list for btrfs-progs.  I've also pasted a link to it
> (and this one too) in reply to the thread you started days ago.

So yes I knew you had a patch obviously :) but I missed the Email where
you sent it, and just found it now
"[RFC PATCH] Btrfs-progs: send, calculate and report progress based on the new flag"

> > 4) Can I run btrfs send -o snap1 snap2 >/dev/null to get quick stats
> > on the changes between the 2 snapshots, or is it still going to walk
> > through all the data blocks and send them to /dev/null afterwards?
> 
> No, but it's doable and implies only extending the user space tools.
> What is implemented is explained in the commit log for the btrfs-progs
> patch.

Yes, I just read that now, my apologies for missing that other Email.

Thanks,
Marc

Patch
diff mbox

--- fs/btrfs/send.c
+++ fs/btrfs/send.c
@@ -3091,6 +3121,8 @@ 
 	int ret;
 	u64 ancestor = 0;
 
+	ASSERT(sctx->phase != SEND_PHASE_COMPUTE_DATA_SIZE);
+
 	name = fs_path_alloc();
 	from_path = fs_path_alloc();
 	if (!name || !from_path) {