Hi, Chris
I think the snapshot should be the image of the fs tree before it was created,
so the metadata of the snapshot should not exist in the its tree. But now, we
found the directory item and directory name index is in both the snapshot tree
and the fs tree.
Besides that, it also makes the users feel strange. For example:
# mkfs.btrfs /dev/sda1
# mount /dev/sda1 /mnt
# mkdir /mnt/1
# cd /mnt/1
# btrfs subvolume snapshot /mnt snap0
# ll /mnt/1
total 0
drwxr-xr-x 1 root root 10 Jun 30 15:01 1
^^^
# ll /mnt/1/snap0/
total 0
drwxr-xr-x 1 root root 10 Jun 30 15:01 1
^^^
It is also 10, but...
# ll /mnt/1/snap0/1
total 0
[None]
There is nothing in the directory 1 in snap0, but btrfs told the length of
this directory is 10, it is strange.
So I think we should insert directory item and directory name index and update
the parent inode as the last step of snapshot creation.
Thanks
Miao
On Fri, 17 Jun 2011 17:12:28 -0400, Chris Mason wrote:
> commit e999376f094162aa425ae749aa1df95ab928d010
> Author: Chris Mason <chris.ma...@oracle.com>
> Date: Fri Jun 17 16:14:09 2011 -0400
>
> Btrfs: avoid delayed metadata items during commits
>
> Snapshot creation has two phases. One is the initial snapshot setup,
> and the second is done during commit, while nobody is allowed to modify
> the root we are snapshotting.
>
> The delayed metadata insertion code can break that rule, it does a
> delayed inode update on the inode of the parent of the snapshot,
> and delayed directory item insertion.
>
> This makes sure to run the pending delayed operations before we
> record the snapshot root, which avoids corruptions.
>
> Signed-off-by: Chris Mason <chris.ma...@oracle.com>
>
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index fc515b7..f1cbd02 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -1237,6 +1237,13 @@ again:
> return 0;
> }
>
> +void btrfs_assert_delayed_root_empty(struct btrfs_root *root)
> +{
> + struct btrfs_delayed_root *delayed_root;
> + delayed_root = btrfs_get_delayed_root(root);
> + WARN_ON(btrfs_first_delayed_node(delayed_root));
> +}
> +
> void btrfs_balance_delayed_items(struct btrfs_root *root)
> {
> struct btrfs_delayed_root *delayed_root;
> diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h
> index cb79b67..d1a6a29 100644
> --- a/fs/btrfs/delayed-inode.h
> +++ b/fs/btrfs/delayed-inode.h
> @@ -137,4 +137,8 @@ int btrfs_readdir_delayed_dir_index(struct file *filp,
> void *dirent,
> /* for init */
> int __init btrfs_delayed_inode_init(void);
> void btrfs_delayed_inode_exit(void);
> +
> +/* for debugging */
> +void btrfs_assert_delayed_root_empty(struct btrfs_root *root);
> +
> #endif
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index c073d85..51dcec8 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -957,6 +957,15 @@ static noinline int create_pending_snapshot(struct
> btrfs_trans_handle *trans,
> ret = btrfs_update_inode(trans, parent_root, parent_inode);
> BUG_ON(ret);
>
> + /*
> + * pull in the delayed directory update
> + * and the delayed inode item
> + * otherwise we corrupt the FS during
> + * snapshot
> + */
> + ret = btrfs_run_delayed_items(trans, root);
> + BUG_ON(ret);
> +
> record_root_in_trans(trans, root);
> btrfs_set_root_last_snapshot(&root->root_item, trans->transid);
> memcpy(new_root_item, &root->root_item, sizeof(*new_root_item));
> @@ -1018,14 +1027,6 @@ static noinline int create_pending_snapshots(struct
> btrfs_trans_handle *trans,
> int ret;
>
> list_for_each_entry(pending, head, list) {
> - /*
> - * We must deal with the delayed items before creating
> - * snapshots, or we will create a snapthot with inconsistent
> - * information.
> - */
> - ret = btrfs_run_delayed_items(trans, fs_info->fs_root);
> - BUG_ON(ret);
> -
> ret = create_pending_snapshot(trans, fs_info, pending);
> BUG_ON(ret);
> }
> @@ -1319,15 +1320,21 @@ int btrfs_commit_transaction(struct
> btrfs_trans_handle *trans,
> */
> mutex_lock(&root->fs_info->reloc_mutex);
>
> - ret = create_pending_snapshots(trans, root->fs_info);
> + ret = btrfs_run_delayed_items(trans, root);
> BUG_ON(ret);
>
> - ret = btrfs_run_delayed_items(trans, root);
> + ret = create_pending_snapshots(trans, root->fs_info);
> BUG_ON(ret);
>
> ret = btrfs_run_delayed_refs(trans, root, (unsigned long)-1);
> BUG_ON(ret);
>
> + /*
> + * make sure none of the code above managed to slip in a
> + * delayed item
> + */
> + btrfs_assert_delayed_root_empty(root);
> +
> WARN_ON(cur_trans != trans->transaction);
>
> btrfs_scrub_pause(root);
>
--
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
