After sending v1 some days ago, some onlist and offlist discussion has brought us to the following conclusions: - we should fix it as soon as possible - we should stick to the -p option (parent) - we should rename the -i option (because it actually means "clone source") This is what v2 of this patch set is doing, besides fixing one and a half bugs along the way: We turn -i into -c, emitting a fatal warning should anybody use -i. As argued in the v1 cover letter, we should really do this change now in order to avoid a lot of confused users that "btrfs send -i" is doing things differently in a subtle way from what "zfs send -i" is doing. We''re better off coming up with our own terminology, thus offering -p and -c options (both can be used to ask for an incremental send stream, see the usage changes in patch 1), without providing a "send -i" option at all. 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 | 74 +++++++++++++++++++++++++++++++++------------------------- send-utils.c | 4 +- 2 files changed, 44 insertions(+), 34 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Schmidt
2012-Nov-06 20:47 UTC
[PATCH v2 1/2] Btrfs-progs: correcting misnamed parameter options for btrfs send
Unfortunately, the command line options for btrfs send were misnamed. The
-i option should not be used to give "clone sources", we''ll
be using -c
instead.
Compatibily note: -i option was broken anyway, which makes it less critical
renaming it. For potential users of the old option style, we emit a fatal
warning if the -i option is used.
Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
---
cmds-send.c | 74 +++++++++++++++++++++++++++++++++-------------------------
1 files changed, 42 insertions(+), 32 deletions(-)
diff --git a/cmds-send.c b/cmds-send.c
index 9b47e70..3e2fcbe 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -234,7 +234,7 @@ out:
return ERR_PTR(ret);
}
-static int do_send(struct btrfs_send *send, u64 root_id, u64 parent_root)
+static int do_send(struct btrfs_send *send, u64 root_id, u64 parent_root_id)
{
int ret;
pthread_t t_read;
@@ -286,7 +286,7 @@ static int do_send(struct btrfs_send *send, u64 root_id, u64
parent_root)
io_send.clone_sources = (__u64*)send->clone_sources;
io_send.clone_sources_count = send->clone_sources_count;
- io_send.parent_root = parent_root;
+ io_send.parent_root = parent_root_id;
ret = ioctl(subvol_fd, BTRFS_IOC_SEND, &io_send);
if (ret) {
ret = -errno;
@@ -423,16 +423,17 @@ int cmd_send_start(int argc, char **argv)
char *snapshot_parent = NULL;
u64 root_id;
u64 parent_root_id = 0;
+ int full_send = 1;
memset(&send, 0, sizeof(send));
send.dump_fd = fileno(stdout);
- while ((c = getopt(argc, argv, "vf:i:p:")) != -1) {
+ while ((c = getopt(argc, argv, "vc:f:i:p:")) != -1) {
switch (c) {
case ''v'':
g_verbose++;
break;
- case ''i'': {
+ case ''c'':
subvol = realpath(optarg, NULL);
if (!subvol) {
ret = -errno;
@@ -454,12 +455,16 @@ int cmd_send_start(int argc, char **argv)
}
add_clone_source(&send, root_id);
free(subvol);
+ full_send = 0;
break;
- }
case ''f'':
outname = optarg;
break;
case ''p'':
+ if (snapshot_parent) {
+ fprintf(stderr, "ERROR: you cannot have more than one parent
(-p)\n");
+ return 1;
+ }
snapshot_parent = realpath(optarg, NULL);
if (!snapshot_parent) {
ret = -errno;
@@ -467,7 +472,12 @@ int cmd_send_start(int argc, char **argv)
"%s\n", optarg, strerror(-ret));
goto out;
}
+ full_send = 0;
break;
+ case ''i'':
+ fprintf(stderr,
+ "ERROR: -i was removed, use -c instead\n");
+ return 1;
case ''?'':
default:
fprintf(stderr, "ERROR: send args invalid.\n");
@@ -573,10 +583,13 @@ int cmd_send_start(int argc, char **argv)
goto out;
}
- if (!parent_root_id) {
+ if (!full_send && !parent_root_id) {
ret = find_good_parent(&send, root_id, &parent_root_id);
- if (ret < 0)
- parent_root_id = 0;
+ if (ret < 0) {
+ fprintf(stderr, "ERROR: parent determination failed for %lld\n",
+ root_id);
+ goto out;
+ }
}
ret = is_subvol_ro(&send, subvol);
@@ -597,6 +610,7 @@ int cmd_send_start(int argc, char **argv)
add_clone_source(&send, root_id);
parent_root_id = 0;
+ full_send = 0;
free(subvol);
}
@@ -614,32 +628,28 @@ static const char * const send_cmd_group_usage[] = {
};
static const char * const cmd_send_usage[] = {
- "btrfs send [-v] [-i <subvol>] [-p <parent>]
<subvol>",
+ "btrfs send [-v] [-p <parent>] [-c <clone-src>]
<subvol>",
"Send the subvolume to stdout.",
"Sends the subvolume specified by <subvol> to stdout.",
- "By default, this will send the whole subvolume. To do",
- "an incremental send, one or multiple ''-i
<clone_source>''",
- "arguments have to be specified. A ''clone source''
is",
- "a subvolume that is known to exist on the receiving",
- "side in exactly the same state as on the sending side.\n",
- "Normally, a good snapshot parent is searched automatically",
- "in the list of ''clone sources''. To override this,
use",
- "''-p <parent>'' to manually specify a snapshot
parent.",
- "A manually specified snapshot parent is also regarded",
- "as ''clone source''.\n",
- "-v Enable verbose debug output. Each",
- " occurrency of this option increases the",
- " verbose level more.",
- "-i <subvol> Informs btrfs send that this subvolume,",
- " can be taken as ''clone source''. This
can",
- " be used for incremental sends.",
- "-p <subvol> Disable automatic snaphot parent",
- " determination and use <subvol> as parent.",
- " This subvolume is also added to the list",
- " of ''clone sources'' (see -i).",
- "-f <outfile> Output is normally written to stdout.",
- " To write to a file, use this option.",
- " An alternative would be to use pipes.",
+ "By default, this will send the whole subvolume. To do an
incremental",
+ "send, use ''-p <parent>''. If you want to allow
btrfs to clone from",
+ "any additional local snapshots, use -c <clone-src> (multiple
times",
+ "where applicable). You must not specify clone sources unless you",
+ "guarantee that these snapshots are exactly in the same state on
both",
+ "sides, the sender and the receiver. It is allowed to omit the",
+ "''-p <parent>'' option when ''-c
<clone-src>'' options are given, in",
+ "which case ''btrfs send'' will determine a suitable
parent among the",
+ "clone sources itself.",
+ "\n",
+ "-v Enable verbose debug output. Each occurrency of",
+ " this option increases the verbose level more.",
+ "-p <parent> Send an incremental stream from <parent>
to",
+ " <subvol>.",
+ "-c <clone-src> Use this snapshot as a clone source for an
",
+ " incremental send (multiple allowed)",
+ "-f <outfile> Output is normally written to stdout. To write
to",
+ " a file, use this option. An alternative would be
to",
+ " use pipes.",
NULL
};
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Schmidt
2012-Nov-06 20:47 UTC
[PATCH v2 2/2] Btrfs-progs: bugfix for subvolume parent determination in btrfs send
We missed to add the default subvolume, because it has no ROOT_BACKREF_ITEM.
This made get_parent always fail for direct decendants of the default
subvolume, resulting in lots of full streams where incremental streams were
requested.
Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
Reviewed-by: Alexander Block <ablock84@googlemail.com>
---
send-utils.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/send-utils.c b/send-utils.c
index fcde5c2..d8d3972 100644
--- a/send-utils.c
+++ b/send-utils.c
@@ -240,7 +240,8 @@ int subvol_uuid_search_init(int mnt_fd, struct
subvol_uuid_search *s)
memcpy(&root_item, root_item_ptr,
sizeof(root_item));
root_item_valid = 1;
- } else if (sh->type == BTRFS_ROOT_BACKREF_KEY) {
+ } else if (sh->type == BTRFS_ROOT_BACKREF_KEY ||
+ root_item_valid) {
if (!root_item_valid)
goto skip;
@@ -274,7 +275,6 @@ int subvol_uuid_search_init(int mnt_fd, struct
subvol_uuid_search *s)
subvol_uuid_search_add(s, si);
root_item_valid = 0;
} else {
- root_item_valid = 0;
goto skip;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html