On Sun, Apr 26, 2015 at 12:12:45PM +0200, Lauri Võsandi wrote:
> 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.

> Signed-off-by: Lauri Võsandi <lauri.vosa...@gmail.com>
> ---
>  cmds-receive.c | 34 ++++++++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/cmds-receive.c b/cmds-receive.c
> index b7cf3f9..391d281 100644
> --- a/cmds-receive.c
> +++ b/cmds-receive.c
> @@ -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.",

-- 
Hugo Mills             | Questions are a burden, and answers a prison for
hugo@... carfax.org.uk | oneself
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                          The Prisoner

Attachment: signature.asc
Description: Digital signature

Reply via email to