Re: [PATCH 4/5] Btrfs: maintain subvolume items in the UUID tree

[David Sterba <dsterba@suse.cz>]

On Fri, Apr 19, 2013 at 05:41:05PM +0200, Stefan Behrens wrote:
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -57,6 +57,8 @@
>  #include "send.h"
>  #include "dev-replace.h"
>  
> +static char empty_uuid[BTRFS_UUID_SIZE] = {0};

+ const

> @@ -567,9 +573,10 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
>  	 * 1 - root item
>  	 * 2 - root ref/backref
>  	 * 1 - root of snapshot
> +	 * 1 - UUID item
>  	 */
>  	ret = btrfs_subvolume_reserve_metadata(BTRFS_I(dir)->root,
> -					&pending_snapshot->block_rsv, 7,
> +					&pending_snapshot->block_rsv, 8,
>  					&pending_snapshot->qgroup_reserved);
>  	if (ret)
>  		goto out;
> @@ -580,7 +587,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
>  	pending_snapshot->dir = dir;
>  	pending_snapshot->inherit = inherit;
>  
> -	trans = btrfs_start_transaction(root, 0);
> +	trans = btrfs_start_transaction(root->fs_info->extent_root, 8);

This look suspicious in 2 ways:

* why is root switched to extent_root
* what's the reason of 8 units being reserved

>  	if (IS_ERR(trans)) {
>  		ret = PTR_ERR(trans);
>  		goto fail;
> @@ -3925,7 +3954,7 @@ static long btrfs_ioctl_set_received_subvol(struct file *file,
>  		goto out;
>  	}
>  
> -	trans = btrfs_start_transaction(root, 1);
> +	trans = btrfs_start_transaction(root, 3);

Please document what's being reserved

>  	if (IS_ERR(trans)) {
>  		ret = PTR_ERR(trans);
>  		trans = NULL;
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -34,6 +34,8 @@
>  
>  #define BTRFS_ROOT_TRANS_TAG 0
>  
> +static char empty_uuid[BTRFS_UUID_SIZE] = {0};

second empty_uuid? well, alternatively you could implement a helper
"is uuid empty" and compare to zeros directly

> +
>  void put_transaction(struct btrfs_transaction *transaction)
>  {
>  	WARN_ON(atomic_read(&transaction->use_count) == 0);