I'm currently working on another solution for the initial problem. I
will create a for-alex branch for you to test later.
On Thu, Jul 26, 2012 at 4:04 PM, Alex Lyakas
<alex.bolshoy.bt...@gmail.com> wrote:
> Alexander,
> (pls let me know when this gets annoying:).
>
> Parent:
> /mnt/src/v2_snap0/
> └── [ 257] file1
>
> Send:
> /mnt/src/v2_snap1
> └── [ 259] dir1
> └── [ 258] dir2
> └── [ 257] file1
>
> I encountered two problems:
> 1) process_recorded_refs_if_needed() if needed does not call
> process_recorded_refs() if both new_refs and deleted_refs() are empty.
> But in this case, we need to get to finish_outoforder_dir() by dir2 to
> move file1 under it (this is before dir1 is created).
>
> @@ -4199,8 +4227,25 @@ static int
> process_recorded_refs_if_needed(struct send_ctx *sctx, int at_end)
> if (!at_end && sctx->cur_ino == sctx->cmp_key->objectid &&
> sctx->cmp_key->type <= BTRFS_INODE_REF_KEY)
> goto out;
> - if (list_empty(&sctx->new_refs) && list_empty(&sctx->deleted_refs))
> - goto out;
> + if (list_empty(&sctx->new_refs) && list_empty(&sctx->deleted_refs) &&
> + /*
> + * If this is a new directory, still do the
> finish_outoforder_dir() thing,
> + * even though it has no references recorded. This
> means that the directory's
> + * parent has higher inode and was not created yet
> (thus we should have
> + * sctx->cur_inode_first_ref_orphan flag set).
> + * Note that after a call to process_recorded_refs(),
> new_refs and deleted_refs
> + * become empty, which prevents further calls to
> process_recorded_refs(),
> + * but here we need something else to prevent it, so
> look at send_progress too.
> + */
> + !(S_ISDIR(sctx->cur_inode_mode) && sctx->cur_inode_new &&
> + sctx->cur_inode_first_ref_orphan &&
> sctx->send_progress == sctx->cur_ino))
> + goto out;
>
> ret = process_recorded_refs(sctx);
>
> Then I encountered another problem that finish_outoforder_dir() does
> not check for itself the cur_inode_first_ref_orphan flag:
> @@ -2736,7 +2754,17 @@ static int finish_outoforder_dir(struct
> send_ctx *sctx, u64 dir, u64 dir_gen)
> }
> fctx.dir_ino = dir;
>
> - ret = get_cur_path(sctx, dir, dir_gen, fctx.dir_path, 1/*do_print*/);
> + /*
> + * If the current directory itself has a parent, which was not
> + * created yet, we need to use gen_unique_name().
> + */
> + BUG_ON(sctx->cur_ino != dir || sctx->cur_inode_gen != dir_gen);
> + if (sctx->cur_inode_first_ref_orphan)
> + ret = gen_unique_name(sctx, dir, dir_gen, fctx.dir_path);
> + else
> + ret = get_cur_path(sctx, dir, dir_gen, fctx.dir_path);
>
> Finally, the send_truncate(), send_chmod(), send_chown(),send_utimes()
> need the following check:
>
> if (sctx->cur_ino == ino && sctx->cur_inode_first_ref_orphan) {
> WARN_ON(sctx->cur_inode_gen != gen);
> ret = gen_unique_name(sctx, ino, gen, p);
> } else {
> ret = get_cur_path(sctx, ino, gen, p);
> }
>
> All of them except utimes() are used with cur_ino only, so for those
> this check is redundant (and probably makes sense to drop ino/gen
> parameters of them?).
>
> Thanks,
> Alex.
>
>
> On Thu, Jul 26, 2012 at 1:52 PM, Alex Lyakas
> <alex.bolshoy.bt...@gmail.com> wrote:
>> Hi Alexander,
>> I did some very initial testing, and there is still an issue.
>> The logic of finish_outoforder_dir works as expected. But then problem
>> is that later, when we process xattr/extents or finish the inode, the
>> code still uses get_cur_path(), which brings an incorrect name.
>>
>> Consider the following simple scenario:
>>
>> Parent tree:
>> /mnt/src/v2
>> └── [ 260] file1
>>
>> Send tree:
>> /mnt/src/v2
>> └── [ 262] dir1
>> └── [ 260] file1
>>
>> So when file1 is being processed, it is first renamed, as expected:
>> C_RENAME: A_PATH=file1, A_PATH_TO=o260-511-0
>> But then, when we finish it, we do:
>> C_TRUNCATE: A_PATH=o262-517-0/file1, A_SIZE=16
>>
>> So in some functions like send_truncate(), send_write(), send_utimes()
>> etc, we need:
>>
>> - ret = get_cur_path(sctx, ino, gen, p, 0/*do_print*/);
>> + if (sctx->cur_inode_first_ref_orphan)
>> + ret = gen_unique_name(sctx, ino, gen, p);
>> + else
>> + ret = get_cur_path(sctx, ino, gen, p, 0/*do_print*/);
>> if (ret < 0)
>> goto out;
>>
>> I will continue testing more complicated cases now.
>>
>> Thanks,
>> Alex.
>>
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Jul 24, 2012 at 11:26 PM, Alexander Block
>> <abloc...@googlemail.com> wrote:
>>> On Wed, Jul 18, 2012 at 7:45 PM, Alex Lyakas
>>> <alex.bolshoy.bt...@gmail.com> wrote:
>>>> Hi Alexander,
>>>> I am testing different scenarios in order to better understand the
>>>> non-trivial magic of
>>>> get_cur_path()/will_overwrite_ref()/did_overwrite_ref()/did_overwrite_first_ref().
>>>> I hit the following issue, when testing full-send:
>>>>
>>>> This is my source subvolume (inode numbers are written):
>>>> tree -A --inodes --noreport /mnt/src/tmp/
>>>> /mnt/src/tmp/
>>>> └── [ 270] dir2
>>>> └── [ 268] file1_nod
>>>>
>>>> As you see, the ino(file1_nod) < ino(dir2). It is very easy to
>>>> achieve: first create the file, then the dir, and then move the file
>>>> to dir.
>>>>
>>>> During send the following happens (I augmented the send code with many
>>>> prints):
>>>>
>>>> file1_nod is sent first. Since its a new inode, it is sent as an
>>>> orphan. When recording its reference, __record_new_ref() calls
>>>> get_cur_path() for its parent (270). Then __get_cur_name_and_parent()
>>>> is called on 270, which calls is_inode_existent(), which calls
>>>> get_cur_inode_state(), and the state of the parent is "will_create".
>>>> So __get_cur_name_and_parent() creates an orphan name for it, and
>>>> finally the new reference for 268 is recorded as:
>>>> o270-136-0/file1_nod:
>>>>
>>>> [changed_cb:4102] key(256 INODE_ITEM 0) : NEW
>>>> [changed_cb:4102] key(256 INODE_REF 256) : NEW
>>>> [changed_cb:4102] key(268 INODE_ITEM 0) : NEW
>>>> [send_create_inode:2407] NEW ino(268,135) type=0100000, path=[o268-135-0]
>>>> [changed_cb:4102] key(268 INODE_REF 270) : NEW
>>>> [get_cur_inode_state:1475] (270,136): L(EX,136)
>>>> R(NE,18446744072099047770) sp=268 ==> will_create
>>>> [is_inode_existent:1498] (270,136): NOT existent
>>>> [__get_cur_name_and_parent:1918] ino(270,136) not existent => unique
>>>> name [o270-136-0]
>>>> [get_cur_path:2051] ino(0,0) cur_path=[o270-136-0]
>>>> [__record_new_ref:2911] record new ref [o270-136-0/file1_nod]
>>>>
>>>> Then process_recorded_refs() sees that 268 is still orphan, so it
>>>> sends "rename" to its valid place, but the problem is that its parent
>>>> dir was not sent yet (and its parent dir is also an orphan):
>>>> [process_recorded_refs:2601] ino(268,135): start with refs
>>>> [28118.347602] [process_recorded_refs:2651] ino(268,135): new=1,
>>>> did_overwrite_first_ref=0, is_orphan=1, valid_path=[o268-135-0]
>>>> [28118.347605] [process_recorded_refs:2701] ino(268,135): is orphan,
>>>> move it: [o268-135-0]=>[o270-136-0/file1_nod]
>>>> [28118.347610] [process_recorded_refs:2837] checking dir(270,136)
>>>> [28118.347612] [process_recorded_refs:2869] ino(268,135) done with refs
>>>>
>>>> Now the parent dir is processed:
>>>> [changed_cb:4102] key(270 INODE_ITEM 0) : NEW
>>>> [send_create_inode:2407] NEW ino(270,136) type=040000, path=[o270-136-0]
>>>> [changed_cb:4102] key(270 INODE_REF 256) : NEW
>>>> [get_cur_path:2051] ino(256,133) cur_path=[]
>>>> [__record_new_ref:2911] record new ref [dir2]
>>>> [process_recorded_refs:2601] ino(270,136): start with refs
>>>> [process_recorded_refs:2651] ino(270,136): new=1,
>>>> did_overwrite_first_ref=0, is_orphan=1, valid_path=[o270-136-0]
>>>> [process_recorded_refs:2701] ino(270,136): is orphan, move it:
>>>> [o270-136-0]=>[dir2]
>>>> [process_recorded_refs:2837] checking dir(256,133)
>>>> [get_cur_inode_state:1475] (256,133): L(EX,133)
>>>> R(NE,18446612135413283512) sp=270 ==> did_create
>>>> [process_recorded_refs:2869] ino(270,136) done with refs
>>>>
>>>> Nothing special here, the parent is first sent as an orphan, and then
>>>> renamed to its valid name, but it's too late.
>>>>
>>>> During receive:
>>>> ERROR: rename o268-135-0 -> o270-136-0/file1_nod failed. No such file
>>>> or directory
>>>>
>>>> I am not yet sure where is the proper place to fix this, I just wanted
>>>> to report it first. Basically, I think that when sending any kind of
>>>> A_PATH, it is needed to ensure that path components exist, either as
>>>> orphan or real path (by sending them out-of-order if needed?). But I
>>>> am not yet sure where is the core place that should ensure this.
>>>>
>>>> Thanks,
>>>> Alex.
>>>
>>> I have pushed a fix for this case. Basically, the solution is to
>>> postpone the processing of refs in not created dirs until the dir is
>>> created. Big thanks for investigating this one.
--
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
