[PATCH 0/2] Btrfs-progs: urgent fixes for btrfs send

2012-11-01 Thread Jan Schmidt
Hi everybody,

We made a bad mistake with btrfs send command line arguments and we'd
better fix it before it's being widely used (read: *now*).

When using btrfs send as in the current master, the -i option does
*not* give you an incremental stream as you'd expect. There are two
problems in the back:

- The -i option adds clone sources and btrfs determines itself
  whether any of these should be used to generate an incremental
  stream.

- That determination is broken.

There is however a -p option to force generation of an incremental
stream. This is a must change in my opinion. We should be using send
-i in the same way zfs send is using it. I expect anything else to
cause wide confusion. Therefore, these 2 patches do the following:

- Turn -i into -r, which stands for remote or receiving
  side. The option is meant to tell btrfs which subvolumes
  exist on the receiver.

- Turn -p into -i. Yes, this is a clash.

- Fix the parent determination (required in combination with
  the -r option) in a way, that we never overwrite a base for
  an incremental snapshot given with -i.

Outcome for people who are already used to the current way btrfs send
works:

- btrfs send -p [base] [subvol]
  This command now prints a fatal error message to use -i
  instead.
  NEW: btrfs send -i [base] [subvol]

- btrfs send -i [snap] [subvol]
  This command now does exactly what one would have expected
  from the previous documentation: it should have automatically
  determined [snap] as base for an incremental stream. Now it
  really selects [snap] as base.

- btrfs send -p [base] -i [snap1] -i [snap2] [subvol]
  Prints the same error message as the first example.
  NEW: btrfs send -i [base] -r [snap1] -r [snap2] [subvol]

- btrfs send -i [snap1] -i [snap2] [subvol]
  Previously, determination of the best base was likely to fail,
  resulting in a full stream. Now you get a fatal error message
  for specifying -i multiple times.
  NEW: btrfs send -r [snap1] -r [snap2] [subvol]

Although this gives a change in command line options rather late in the
game, I'll emphasis again that I think it's a no-go to leave it as it
currently is. The outcome as outlined should be acceptable for anyone, I
don't see a case where this change does something completely different
after the change. Users will have to adapt to the corrected switches,
though.

For ease of management, you can fetch these patches from my git repo,
based on top of the current cmason/master:

git://git.jan-o-sch.net/btrfs-progs for-chris

-Jan

Jan Schmidt (2):
  Btrfs-progs: correcting misnamed parameter options for btrfs send
  Btrfs-progs: bugfix for subvolume parent determination in btrfs send

 cmds-send.c  |   97 +++---
 send-utils.c |4 +-
 2 files changed, 54 insertions(+), 47 deletions(-)

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] Btrfs-progs: urgent fixes for btrfs send

2012-11-01 Thread Chris Mason
On Thu, Nov 01, 2012 at 09:01:24AM -0600, Jan Schmidt wrote:
 Hi everybody,
 
 We made a bad mistake with btrfs send command line arguments and we'd
 better fix it before it's being widely used (read: *now*).

Ok, I do agree that -i was confusing.  I didn't end up using it in my
backup scripts here.

How about:

Make -p and -i mean the same thing.  Add -r for what -i should have
done.  

This has the advantage of not breaking the people that did get working
btrfs send setups ;)

-chris
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] Btrfs-progs: urgent fixes for btrfs send

2012-11-01 Thread Jan Schmidt
On Thu, November 01, 2012 at 16:07 (+0100), Chris Mason wrote:
 On Thu, Nov 01, 2012 at 09:01:24AM -0600, Jan Schmidt wrote:
 Hi everybody,

 We made a bad mistake with btrfs send command line arguments and we'd
 better fix it before it's being widely used (read: *now*).
 
 Ok, I do agree that -i was confusing.  I didn't end up using it in my
 backup scripts here.

Good we agree here :-)

 How about:
 
 Make -p and -i mean the same thing.  Add -r for what -i should have
 done.  
 
 This has the advantage of not breaking the people that did get working
 btrfs send setups ;)

I'd carefully argue that we're still in the position to break things, because
the 3.7 kernel isn't released and you cannot use btrfs send without it. The
number of users should be really small.

I prefer having a clean and painful cut over suffering from bad decisions
forever. That may not be the most popular opinion in the world. In the end, I
could live with -p and -i doing the same thing.

-Jan
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] Btrfs-progs: urgent fixes for btrfs send

2012-11-01 Thread Chris Mason
On Thu, Nov 01, 2012 at 09:48:25AM -0600, Jan Schmidt wrote:
 On Thu, November 01, 2012 at 16:07 (+0100), Chris Mason wrote:
  On Thu, Nov 01, 2012 at 09:01:24AM -0600, Jan Schmidt wrote:
  Hi everybody,
 
  We made a bad mistake with btrfs send command line arguments and we'd
  better fix it before it's being widely used (read: *now*).
  
  Ok, I do agree that -i was confusing.  I didn't end up using it in my
  backup scripts here.
 
 Good we agree here :-)
 
  How about:
  
  Make -p and -i mean the same thing.  Add -r for what -i should have
  done.  
  
  This has the advantage of not breaking the people that did get working
  btrfs send setups ;)
 
 I'd carefully argue that we're still in the position to break things, because
 the 3.7 kernel isn't released and you cannot use btrfs send without it. The
 number of users should be really small.
 
 I prefer having a clean and painful cut over suffering from bad decisions
 forever. That may not be the most popular opinion in the world. In the end, I
 could live with -p and -i doing the same thing.

But we have a working -p, I'm not sure why we'd rename it to -i?  I'm
even fine with just flat out removing -i.  This is mostly because
--parent makes a lot of sense to me, but I'm more than open to other
ideas.

-chris

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html