On fri, 27 Jul 2012 16:52:21 +0900, Hidetoshi Seto wrote:
>> # ll /mnt/1/snap0/1
>> total 0
>> [None]
>> # cd /mnt/1/snap0/1/snap0
>> [Enter a unexisted directory successfully...]
>
> I confirmed that "mkdir snap0" failed with "File exists" and
> that rmdir can remove the directory snap0. So it is a kind of
> "invisible" rather than "unexisted".
I think it is not like the typically invisible directories on linux, and in the users'
view, this directory should not exist, in other words, it is a unexisted directory to
the users, so I use "unexisted".
> (snip)
>
>> @@ -1062,17 +1068,33 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>> ret = btrfs_reloc_post_snapshot(trans, pending);
>> if (ret)
>> goto abort_trans;
>> +
>> + ret = btrfs_insert_dir_item(trans, parent_root,
>> + dentry->d_name.name, dentry->d_name.len,
>> + parent_inode, &key,
>> + BTRFS_FT_DIR, index);
>> + /* We have check the name at the beginning, so it is impossible. */
>> + BUG_ON(ret == -EEXIST);
>> + if (ret)
>> + goto abort_trans;
>> +
>> + btrfs_i_size_write(parent_inode, parent_inode->i_size +
>> + dentry->d_name.len * 2);
>> + ret = btrfs_update_inode(trans, parent_root, parent_inode);
>> + if (ret)
>> + goto abort_trans;
>> fail:
>> dput(parent);
>> trans->block_rsv = rsv;
>> no_free_objectid:
>> kfree(new_root_item);
>> root_item_alloc_fail:
>> + btrfs_free_path(path);
>> +path_alloc_fail:
>> btrfs_block_rsv_release(root, &pending->block_rsv, (u64)-1);
>> return ret;
>>
>> abort_trans:
>> - dput(parent);
>
> I think you can remove this line in your 1/2 patch.
Yes. will be modified in the next version of the patches.
>
>> btrfs_abort_transaction(trans, root, ret);
>> goto fail;
>> }
>> @@ -1386,13 +1408,13 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans,
>> */
>> mutex_lock(&root->fs_info->reloc_mutex);
>>
>> - ret = btrfs_run_delayed_items(trans, root);
>> + ret = create_pending_snapshots(trans, root->fs_info);
>> if (ret) {
>> mutex_unlock(&root->fs_info->reloc_mutex);
>> goto cleanup_transaction;
>> }
>>
>> - ret = create_pending_snapshots(trans, root->fs_info);
>> + ret = btrfs_run_delayed_items(trans, root);
>> if (ret) {
>> mutex_unlock(&root->fs_info->reloc_mutex);
>> goto cleanup_transaction;
>
> It would be nice to have a patch description to tell why you
> have to change the order here.
OK, I will add comment in the next version of the patches.
Thanks for your review.
Miao
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
