diff options
author | Filipe Manana <fdmanana@suse.com> | 2017-06-13 14:13:11 +0100 |
---|---|---|
committer | David Sterba <dsterba@suse.com> | 2017-06-21 16:53:10 +0200 |
commit | fdb1388994e2ed0e1512006244022c721c650c5b (patch) | |
tree | 119efd00f77c0269e1da2cce13045351fbc026d8 | |
parent | 72c3668fed2b3eec7c6fc059e7b54855361e3011 (diff) |
Btrfs: incremental send, fix invalid path for unlink commands
An incremental send can contain unlink operations with an invalid target
path when we rename some directory inode A, then rename some file inode B
to the old name of inode A and directory inode A is an ancestor of inode B
in the parent snapshot (but not anymore in the send snapshot).
Consider the following example scenario where this issue happens.
Parent snapshot:
. (ino 256)
|
|--- dir1/ (ino 257)
|--- dir2/ (ino 258)
| |--- file1 (ino 259)
| |--- file3 (ino 261)
|
|--- dir3/ (ino 262)
|--- file22 (ino 260)
|--- dir4/ (ino 263)
Send snapshot:
. (ino 256)
|
|--- dir1/ (ino 257)
|--- dir2/ (ino 258)
|--- dir3 (ino 260)
|--- file3/ (ino 262)
|--- dir4/ (ino 263)
|--- file11 (ino 269)
|--- file33 (ino 261)
When attempting to apply the corresponding incremental send stream, an
unlink operation contains an invalid path which makes the receiver fail.
The following is verbose output of the btrfs receive command:
receiving snapshot snap2 uuid=7d5450da-a573-e043-a451-ec85f4879f0f (...)
utimes
utimes dir1
utimes dir1/dir2
link dir1/dir3/dir4/file11 -> dir1/dir2/file1
unlink dir1/dir2/file1
utimes dir1/dir2
truncate dir1/dir3/dir4/file11 size=0
utimes dir1/dir3/dir4/file11
rename dir1/dir3 -> o262-7-0
link dir1/dir3 -> o262-7-0/file22
unlink dir1/dir3/file22
ERROR: unlink dir1/dir3/file22 failed. Not a directory
The following steps happen during the computation of the incremental send
stream the lead to this issue:
1) Before we start processing the new and deleted references for inode
260, we compute the full path of the deleted reference
("dir1/dir3/file22") and cache it in the list of deleted references
for our inode.
2) We then start processing the new references for inode 260, for which
there is only one new, located at "dir1/dir3". When processing this
new reference, we check that inode 262, which was not yet processed,
collides with the new reference and because of that we orphanize
inode 262 so its new full path becomes "o262-7-0".
3) After the orphanization of inode 262, we create the new reference for
inode 260 by issuing a link command with a target path of "dir1/dir3"
and a source path of "o262-7-0/file22".
4) We then start processing the deleted references for inode 260, for
which there is only one with the base name of "file22", and issue
an unlink operation containing the target path computed at step 1,
which is wrong because that path no longer exists and should be
replaced with "o262-7-0/file22".
So fix this issue by recomputing the full path of deleted references if
when we processed the new references for an inode we ended up orphanizing
any other inode that is an ancestor of our inode in the parent snapshot.
A test case for fstests follows soon.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
[ adjusted after prev patch removed fs_path::dir_path and dir_path_len ]
Signed-off-by: David Sterba <dsterba@suse.com>
-rw-r--r-- | fs/btrfs/send.c | 52 |
1 files changed, 47 insertions, 5 deletions
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index ddd2b786f4d5..a562dc228794 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -2776,6 +2776,13 @@ struct recorded_ref { int name_len; }; +static void set_ref_path(struct recorded_ref *ref, struct fs_path *path) +{ + ref->full_path = path; + ref->name = (char *)kbasename(ref->full_path->start); + ref->name_len = ref->full_path->end - ref->name; +} + /* * We need to process new refs before deleted refs, but compare_tree gives us * everything mixed. So we first record all refs and later process them. @@ -2792,11 +2799,7 @@ static int __record_ref(struct list_head *head, u64 dir, ref->dir = dir; ref->dir_gen = dir_gen; - ref->full_path = path; - - ref->name = (char *)kbasename(ref->full_path->start); - ref->name_len = ref->full_path->end - ref->name; - + set_ref_path(ref, path); list_add_tail(&ref->list, head); return 0; } @@ -3691,6 +3694,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) int is_orphan = 0; u64 last_dir_ino_rm = 0; bool can_rename = true; + bool orphanized_ancestor = false; btrfs_debug(fs_info, "process_recorded_refs %llu", sctx->cur_ino); @@ -3846,6 +3850,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) ow_inode, ow_gen, sctx->cur_ino, NULL); if (ret > 0) { + orphanized_ancestor = true; fs_path_reset(valid_path); ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, @@ -3971,6 +3976,43 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) if (ret < 0) goto out; if (!ret) { + /* + * If we orphanized any ancestor before, we need + * to recompute the full path for deleted names, + * since any such path was computed before we + * processed any references and orphanized any + * 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); + goto out; + } + fs_path_free(cur->full_path); + set_ref_path(cur, new_path); + } ret = send_unlink(sctx, cur->full_path); if (ret < 0) goto out; |