Btrfs: incremental send, fix invalid path for link commands
authorFilipe Manana <fdmanana@suse.com>
Thu, 22 Jun 2017 19:03:51 +0000 (20:03 +0100)
committerFilipe Manana <fdmanana@suse.com>
Thu, 6 Jul 2017 22:02:18 +0000 (23:02 +0100)
In some scenarios an incremental send stream can contain link commands
with an invalid target path. Such scenarios happen after moving some
directory inode A, renaming a regular file inode B into the old name of
inode A and finally creating a new hard link for inode B at directory
inode A.

Consider the following example scenario where this issue happens.

Parent snapshot:

  .                                                      (ino 256)
  |
  |--- dir1/                                             (ino 257)
  |      |--- dir2/                                      (ino 258)
  |             |--- dir3/                               (ino 259)
  |                   |--- file1                         (ino 261)
  |                   |--- dir4/                         (ino 262)
  |
  |--- dir5/                                             (ino 260)

Send snapshot:

  .                                                      (ino 256)
  |
  |--- dir1/                                             (ino 257)
         |--- dir2/                                      (ino 258)
         |      |--- dir3/                               (ino 259)
         |            |--- dir4                          (ino 261)
         |
         |--- dir6/                                      (ino 263)
                |--- dir44/                              (ino 262)
                       |--- file11                       (ino 261)
                       |--- dir55/                       (ino 260)

When attempting to apply the corresponding incremental send stream, a
link command contains an invalid target path which makes the receiver
fail. The following is the verbose output of the btrfs receive command:

  receiving snapshot mysnap2 uuid=90076fe6-5ba6-e64a-9321-9279670ed16b (...)
  utimes
  utimes dir1
  utimes dir1/dir2/dir3
  utimes
  rename dir1/dir2/dir3/dir4 -> o262-7-0
  link dir1/dir2/dir3/dir4 -> dir1/dir2/dir3/file1
  link dir1/dir2/dir3/dir4/file11 -> dir1/dir2/dir3/file1
  ERROR: link dir1/dir2/dir3/dir4/file11 -> dir1/dir2/dir3/file1 failed: Not a directory

The following steps happen during the computation of the incremental send
stream the lead to this issue:

1) When processing inode 261, we orphanize inode 262 due to a name/location
   collision with one of the new hard links for inode 261 (created in the
   second step below).

2) We create one of the 2 new hard links for inode 261, the one whose
   location is at "dir1/dir2/dir3/dir4".

3) We then attempt to create the other new hard link for inode 261, which
   has inode 262 as its parent directory. Because the path for this new
   hard link was computed before we started processing the new references
   (hard links), it reflects the old name/location of inode 262, that is,
   it does not account for the orphanization step that happened when
   we started processing the new references for inode 261, whence it is
   no longer valid, causing the receiver to fail.

So fix this issue by recomputing the full path of new references if we
ended up orphanizing other inodes which are directories.

A test case for fstests follows soon.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
fs/btrfs/send.c

index e937c10b8287594ff8ff1509599bfe6a0559376c..7eaccfb72b479e7d35b4251d3c4882612061739e 100644 (file)
@@ -1856,7 +1856,7 @@ out:
  */
 static int will_overwrite_ref(struct send_ctx *sctx, u64 dir, u64 dir_gen,
                              const char *name, int name_len,
-                             u64 *who_ino, u64 *who_gen)
+                             u64 *who_ino, u64 *who_gen, u64 *who_mode)
 {
        int ret = 0;
        u64 gen;
@@ -1905,7 +1905,7 @@ static int will_overwrite_ref(struct send_ctx *sctx, u64 dir, u64 dir_gen,
        if (other_inode > sctx->send_progress ||
            is_waiting_for_move(sctx, other_inode)) {
                ret = get_inode_info(sctx->parent_root, other_inode, NULL,
-                               who_gen, NULL, NULL, NULL, NULL);
+                               who_gen, who_mode, NULL, NULL, NULL);
                if (ret < 0)
                        goto out;
 
@@ -3683,6 +3683,36 @@ out:
        return ret;
 }
 
+static int update_ref_path(struct send_ctx *sctx, struct recorded_ref *ref)
+{
+       int ret;
+       struct fs_path *new_path;
+
+       /*
+        * Our reference's name member points to its full_path member string, so
+        * we use here a new path.
+        */
+       new_path = fs_path_alloc();
+       if (!new_path)
+               return -ENOMEM;
+
+       ret = get_cur_path(sctx, ref->dir, ref->dir_gen, new_path);
+       if (ret < 0) {
+               fs_path_free(new_path);
+               return ret;
+       }
+       ret = fs_path_add(new_path, ref->name, ref->name_len);
+       if (ret < 0) {
+               fs_path_free(new_path);
+               return ret;
+       }
+
+       fs_path_free(ref->full_path);
+       set_ref_path(ref, new_path);
+
+       return 0;
+}
+
 /*
  * This does all the move/link/unlink/rmdir magic.
  */
@@ -3696,10 +3726,12 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
        struct fs_path *valid_path = NULL;
        u64 ow_inode = 0;
        u64 ow_gen;
+       u64 ow_mode;
        int did_overwrite = 0;
        int is_orphan = 0;
        u64 last_dir_ino_rm = 0;
        bool can_rename = true;
+       bool orphanized_dir = false;
        bool orphanized_ancestor = false;
 
        btrfs_debug(fs_info, "process_recorded_refs %llu", sctx->cur_ino);
@@ -3798,7 +3830,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                 */
                ret = will_overwrite_ref(sctx, cur->dir, cur->dir_gen,
                                cur->name, cur->name_len,
-                               &ow_inode, &ow_gen);
+                               &ow_inode, &ow_gen, &ow_mode);
                if (ret < 0)
                        goto out;
                if (ret) {
@@ -3815,6 +3847,8 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                                                cur->full_path);
                                if (ret < 0)
                                        goto out;
+                               if (S_ISDIR(ow_mode))
+                                       orphanized_dir = true;
 
                                /*
                                 * If ow_inode has its rename operation delayed
@@ -3920,6 +3954,18 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                                if (ret < 0)
                                        goto out;
                        } else {
+                               /*
+                                * We might have previously orphanized an inode
+                                * which is an ancestor of our current inode,
+                                * so our reference's full path, which was
+                                * computed before any such orphanizations, must
+                                * be updated.
+                                */
+                               if (orphanized_dir) {
+                                       ret = update_ref_path(sctx, cur);
+                                       if (ret < 0)
+                                               goto out;
+                               }
                                ret = send_link(sctx, cur->full_path,
                                                valid_path);
                                if (ret < 0)
@@ -3990,34 +4036,9 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move)
                                 * ancestor inode.
                                 */
                                if (orphanized_ancestor) {
-                                       struct fs_path *new_path;
-
-                                       /*
-                                        * Our reference's name member points to
-                                        * its full_path member string, so we
-                                        * use here a new path.
-                                        */
-                                       new_path = fs_path_alloc();
-                                       if (!new_path) {
-                                               ret = -ENOMEM;
-                                               goto out;
-                                       }
-                                       ret = get_cur_path(sctx, cur->dir,
-                                                          cur->dir_gen,
-                                                          new_path);
-                                       if (ret < 0) {
-                                               fs_path_free(new_path);
-                                               goto out;
-                                       }
-                                       ret = fs_path_add(new_path,
-                                                         cur->name,
-                                                         cur->name_len);
-                                       if (ret < 0) {
-                                               fs_path_free(new_path);
+                                       ret = update_ref_path(sctx, cur);
+                                       if (ret < 0)
                                                goto out;
-                                       }
-                                       fs_path_free(cur->full_path);
-                                       set_ref_path(cur, new_path);
                                }
                                ret = send_unlink(sctx, cur->full_path);
                                if (ret < 0)