Alexander,
yes, I will re-test and let you know. Expect to hear from me this week.
Do you plan to merge this for-alex branch into some mainline
branch, before you take off?
Thanks,
Alex.
On Sat, Jul 28, 2012 at 12:56 PM, Alexander Block
abloc...@googlemail.com wrote:
On Fri, Jul 27, 2012 at 4:37 PM, Alex Lyakas
alex.bolshoy.bt...@gmail.com wrote:
Hi Alexander,
your solution is simple and elegant. I this this issue is solved now. Thanks!
Two minor issues:
1)
/*
* We need some special handling for inodes that get processed before the
parent
* directory got created. See process_all_refs for details.
* This function does the check if we already created the dir out of order.
*/
/*
* Only creates the inode if it is:
* 1. Not a directory
* 2. Or a directory which was not created already due to out of order
*directories. See did_create_dir and process_all_refs for details.
*/
These comments tell to look at process_all_refs(), while we should
look at process_recorded_refs().
2)
* We can however not delete the orphan in case the inode
relies
* in a directory that was not created yet (see
* __record_new_ref)
*/
This part should be removed, because your new solution does not work this
way.
Whoops...corrected all comments.
If you find, time, pls look at the two attached scripts.
btrfs_test_1.sh:
it tries to explore the is_first_ref() issue and founds a problem.
Proposed patch - compare also the (dir,gen) tuple and only the name:
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 68e504c..b83ec5f 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -1597,7 +1597,7 @@ out:
static int is_first_ref(struct send_ctx *sctx,
struct btrfs_root *root,
- u64 ino, u64 dir,
+ u64 ino, u64 dir, u64 dir_gen,
const char *name, int name_len)
{
int ret;
@@ -1613,6 +1613,11 @@ static int is_first_ref(struct send_ctx *sctx,
if (ret 0)
goto out;
+ if (dir != tmp_dir || dir_gen != tmp_dir_gen) {
+ ret = 0;
+ goto out;
+ }
+
if (name_len != fs_path_len(tmp_name)) {
ret = 0;
goto out;
@@ -2834,7 +2839,7 @@ verbose_printk(btrfs: process_recorded_refs
%llu\n, sctx-cur_ino);
goto out;
if (ret) {
ret = is_first_ref(sctx, sctx-parent_root,
- ow_inode, cur-dir, cur-name,
+ ow_inode, cur-dir,
cur-dir_gen, cur-name,
cur-name_len);
if (ret 0)
goto out;
I did not apply the patch but instead added a check for dir != tmp_dir
only. The reason to not check for gen is that I have a rule in my
mind: I only pass the generation number to functions where I want to
know the *current* state. is_first_ref is for permanent state, the
return value never changes while sending. I could however not
reproduce the problem with test_1.sh, but it should fix it.
btrfs_test_2.sh
The last test exposes an interesting issue: when a directory has a
deleted reference recorded, this deleted reference is not added to the
'check_dirs' list. As a result, the upper directory (already
orphanized) is not rmdir'd.
You'll find a commit in my repo to fix this. The problem was, that
moved directories do not get into the delete_refs loop and thus the
parent of the old location is never added to the check_dirs list.
I have force pushed to for-alex, if you have time I'd be happy if you
test again :)
Thanks,
Alex.
--
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
