On Wed, 19 Dec 2007 20:37:32 +0800
"Yan Zheng" <[EMAIL PROTECTED]> wrote:

> Hello,
> 
> This patch updates btrfs-progs according to kernel module's code. It
> updates btrfs-progs to properly handle back reference and exports some
> functions in extent-tree.c

Great, thanks for doing this.  For the functions in extent-tree.c that
get exported, could you please change their name to start with btrfs_

A few other comments below:

> --- a/ctree.h Fri Dec 14 11:00:30 2007 -0500
> +++ b/ctree.h Wed Dec 19 19:52:02 2007 +0800
> @@ -607,7 +607,7 @@ BTRFS_SETGET_STACK_FUNCS(file_extent_num
>       btrfs_item_offset((leaf)->items + (slot))))
>  #define btrfs_item_ptr_offset(leaf, slot) \
>       ((unsigned long)(btrfs_leaf_data(leaf) + \
> -     btrfs_item_offset_nr(leaf, slot)))
> +     btrfs_item_offset((leaf)->items + (slot))))

Why switch away from btrfs_item_offset_nr?

> 
> diff -r a1a37f51dcb2 disk-io.c
> --- a/disk-io.c       Fri Dec 14 11:00:30 2007 -0500
> +++ b/disk-io.c       Wed Dec 19 19:52:02 2007 +0800
> @@ -298,12 +298,13 @@ int btrfs_commit_transaction(struct btrf
>       btrfs_finish_extent_commit(trans,
> root->fs_info->extent_root); btrfs_finish_extent_commit(trans,
> root->fs_info->tree_root);
> 
> +     ret = btrfs_drop_snapshot(trans, root, snap);
> +     BUG_ON(ret);
> +     ret = btrfs_del_root(trans, root->fs_info->tree_root,
> &snap_key);
> +     BUG_ON(ret);
> +     
>       root->commit_root = root->node;
>       root->node->count++;
> -     ret = btrfs_drop_snapshot(trans, root, snap);
> -     BUG_ON(ret);
> -     ret = btrfs_del_root(trans, root->fs_info->tree_root,
> &snap_key);
> -     BUG_ON(ret);
>       btrfs_free_transaction(root, trans);
>       return ret;
>  }

This code is buggy (even before your changes ;).  A new transaction
should be started to drop the snapshot and delete the old root.

-chris

_______________________________________________
Btrfs-devel mailing list
[email protected]
http://oss.oracle.com/mailman/listinfo/btrfs-devel

Reply via email to