[PATCH v2 0/3] Allow rmdir(2) to delete a subvolume

[PATCH v2 0/3] Allow rmdir(2) to delete a subvolume

[Misono Tomohiro]

changelog:
  v1 -> v2 ... split the patch to hopefully make review easier

1st patch is a preparation work just moving the declaration of
may_destroy_subvol().

2nd patch is the main part. New function btrfs_delete_subvolume() is
introduced and used in btrfs_rmdir() when a direcoty is an empty
subvolume. The function is almost the copy of second half of
btrfs_ioctl_snap_destroy().
The code path for "sub delete" is not changed yet.

3rd patch is a cleanup of btrfs_ioctl_snap_destroy() and uses 
brrfs_delete_subvolume() for "sub delete" too.

Tomohiro Misono (3):
  btrfs: move may_destroy_subvol() from ioctl.c to inode.c
  btrfs: Allow rmdir(2) to delete a subvolume
  btrfs: cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume()

 fs/btrfs/ctree.h |   5 +-
 fs/btrfs/inode.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/btrfs/ioctl.c | 185 +--------------------------------------------------
 3 files changed, 200 insertions(+), 189 deletions(-)

-- 
2.14.3

[PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c

[Misono Tomohiro]

This is a preparation work to allow rmdir(2) to delete a subvolume.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 fs/btrfs/ctree.h |  1 +
 fs/btrfs/inode.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.c | 54 ------------------------------------------------------
 3 files changed, 55 insertions(+), 54 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index da308774b8a4..2dbead106aab 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3166,6 +3166,7 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
 			struct btrfs_root *root,
 			struct inode *dir, u64 objectid,
 			const char *name, int name_len);
+noinline int may_destroy_subvol(struct btrfs_root *root);
 int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 			int front);
 int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f53470112670..db66fa4fede6 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4333,6 +4333,60 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
+/*
+ * helper to check if the subvolume references other subvolumes
+ */
+noinline int may_destroy_subvol(struct btrfs_root *root)
+{
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_path *path;
+	struct btrfs_dir_item *di;
+	struct btrfs_key key;
+	u64 dir_id;
+	int ret;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	/* Make sure this root isn't set as the default subvol */
+	dir_id = btrfs_super_root_dir(fs_info->super_copy);
+	di = btrfs_lookup_dir_item(NULL, fs_info->tree_root, path,
+				   dir_id, "default", 7, 0);
+	if (di && !IS_ERR(di)) {
+		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &key);
+		if (key.objectid == root->root_key.objectid) {
+			ret = -EPERM;
+			btrfs_err(fs_info,
+				  "deleting default subvolume %llu is not allowed",
+				  key.objectid);
+			goto out;
+		}
+		btrfs_release_path(path);
+	}
+
+	key.objectid = root->root_key.objectid;
+	key.type = BTRFS_ROOT_REF_KEY;
+	key.offset = (u64)-1;
+
+	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
+	if (ret < 0)
+		goto out;
+	BUG_ON(ret == 0);
+
+	ret = 0;
+	if (path->slots[0] > 0) {
+		path->slots[0]--;
+		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+		if (key.objectid == root->root_key.objectid &&
+		    key.type == BTRFS_ROOT_REF_KEY)
+			ret = -ENOTEMPTY;
+	}
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 111ee282b777..11af9eb449ef 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1843,60 +1843,6 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file,
 	return ret;
 }
 
-/*
- * helper to check if the subvolume references other subvolumes
- */
-static noinline int may_destroy_subvol(struct btrfs_root *root)
-{
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_path *path;
-	struct btrfs_dir_item *di;
-	struct btrfs_key key;
-	u64 dir_id;
-	int ret;
-
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
-	/* Make sure this root isn't set as the default subvol */
-	dir_id = btrfs_super_root_dir(fs_info->super_copy);
-	di = btrfs_lookup_dir_item(NULL, fs_info->tree_root, path,
-				   dir_id, "default", 7, 0);
-	if (di && !IS_ERR(di)) {
-		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &key);
-		if (key.objectid == root->root_key.objectid) {
-			ret = -EPERM;
-			btrfs_err(fs_info,
-				  "deleting default subvolume %llu is not allowed",
-				  key.objectid);
-			goto out;
-		}
-		btrfs_release_path(path);
-	}
-
-	key.objectid = root->root_key.objectid;
-	key.type = BTRFS_ROOT_REF_KEY;
-	key.offset = (u64)-1;
-
-	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
-	BUG_ON(ret == 0);
-
-	ret = 0;
-	if (path->slots[0] > 0) {
-		path->slots[0]--;
-		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
-		if (key.objectid == root->root_key.objectid &&
-		    key.type == BTRFS_ROOT_REF_KEY)
-			ret = -ENOTEMPTY;
-	}
-out:
-	btrfs_free_path(path);
-	return ret;
-}
-
 static noinline int key_in_sk(struct btrfs_key *key,
 			      struct btrfs_ioctl_search_key *sk)
 {
-- 
2.14.3

Re: [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c

[Nikolay Borisov]

On 26.03.2018 11:28, Misono Tomohiro wrote:
> This is a preparation work to allow rmdir(2) to delete a subvolume.
> 
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> ---
>  fs/btrfs/ctree.h |  1 +
>  fs/btrfs/inode.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/ioctl.c | 54 ------------------------------------------------------
>  3 files changed, 55 insertions(+), 54 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index da308774b8a4..2dbead106aab 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3166,6 +3166,7 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
>  			struct btrfs_root *root,
>  			struct inode *dir, u64 objectid,
>  			const char *name, int name_len);
> +noinline int may_destroy_subvol(struct btrfs_root *root);

Why the explicit noinline? If it's for "documentation"/appearing in
stack traces purposes then you should use noinline_for_stack. But I
don't think it's really needed.

>  int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
>  			int front);
>  int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index f53470112670..db66fa4fede6 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4333,6 +4333,60 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
>  	return ret;
>  }
>  
> +/*
> + * helper to check if the subvolume references other subvolumes
> + */
> +noinline int may_destroy_subvol(struct btrfs_root *root)
> +{
> +	struct btrfs_fs_info *fs_info = root->fs_info;
> +	struct btrfs_path *path;
> +	struct btrfs_dir_item *di;
> +	struct btrfs_key key;
> +	u64 dir_id;
> +	int ret;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	/* Make sure this root isn't set as the default subvol */
> +	dir_id = btrfs_super_root_dir(fs_info->super_copy);
> +	di = btrfs_lookup_dir_item(NULL, fs_info->tree_root, path,
> +				   dir_id, "default", 7, 0);
> +	if (di && !IS_ERR(di)) {
> +		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &key);
> +		if (key.objectid == root->root_key.objectid) {
> +			ret = -EPERM;
> +			btrfs_err(fs_info,
> +				  "deleting default subvolume %llu is not allowed",
> +				  key.objectid);
> +			goto out;
> +		}
> +		btrfs_release_path(path);
> +	}
> +
> +	key.objectid = root->root_key.objectid;
> +	key.type = BTRFS_ROOT_REF_KEY;
> +	key.offset = (u64)-1;
> +
> +	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
> +	if (ret < 0)
> +		goto out;
> +	BUG_ON(ret == 0);
> +
> +	ret = 0;
> +	if (path->slots[0] > 0) {
> +		path->slots[0]--;
> +		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +		if (key.objectid == root->root_key.objectid &&
> +		    key.type == BTRFS_ROOT_REF_KEY)
> +			ret = -ENOTEMPTY;
> +	}
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
>  static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
>  {
>  	struct inode *inode = d_inode(dentry);
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 111ee282b777..11af9eb449ef 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1843,60 +1843,6 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file,
>  	return ret;
>  }
>  
> -/*
> - * helper to check if the subvolume references other subvolumes
> - */
> -static noinline int may_destroy_subvol(struct btrfs_root *root)
> -{
> -	struct btrfs_fs_info *fs_info = root->fs_info;
> -	struct btrfs_path *path;
> -	struct btrfs_dir_item *di;
> -	struct btrfs_key key;
> -	u64 dir_id;
> -	int ret;
> -
> -	path = btrfs_alloc_path();
> -	if (!path)
> -		return -ENOMEM;
> -
> -	/* Make sure this root isn't set as the default subvol */
> -	dir_id = btrfs_super_root_dir(fs_info->super_copy);
> -	di = btrfs_lookup_dir_item(NULL, fs_info->tree_root, path,
> -				   dir_id, "default", 7, 0);
> -	if (di && !IS_ERR(di)) {
> -		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &key);
> -		if (key.objectid == root->root_key.objectid) {
> -			ret = -EPERM;
> -			btrfs_err(fs_info,
> -				  "deleting default subvolume %llu is not allowed",
> -				  key.objectid);
> -			goto out;
> -		}
> -		btrfs_release_path(path);
> -	}
> -
> -	key.objectid = root->root_key.objectid;
> -	key.type = BTRFS_ROOT_REF_KEY;
> -	key.offset = (u64)-1;
> -
> -	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
> -	if (ret < 0)
> -		goto out;
> -	BUG_ON(ret == 0);
> -
> -	ret = 0;
> -	if (path->slots[0] > 0) {
> -		path->slots[0]--;
> -		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> -		if (key.objectid == root->root_key.objectid &&
> -		    key.type == BTRFS_ROOT_REF_KEY)
> -			ret = -ENOTEMPTY;
> -	}
> -out:
> -	btrfs_free_path(path);
> -	return ret;
> -}
> -
>  static noinline int key_in_sk(struct btrfs_key *key,
>  			      struct btrfs_ioctl_search_key *sk)
>  {
> 

Re: [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c

[Misono Tomohiro]

On 2018/03/26 17:51, Nikolay Borisov wrote:
> 
> 
> On 26.03.2018 11:28, Misono Tomohiro wrote:
>> This is a preparation work to allow rmdir(2) to delete a subvolume.
>>
>> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
>> ---
>>  fs/btrfs/ctree.h |  1 +
>>  fs/btrfs/inode.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  fs/btrfs/ioctl.c | 54 ------------------------------------------------------
>>  3 files changed, 55 insertions(+), 54 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index da308774b8a4..2dbead106aab 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -3166,6 +3166,7 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
>>  			struct btrfs_root *root,
>>  			struct inode *dir, u64 objectid,
>>  			const char *name, int name_len);
>> +noinline int may_destroy_subvol(struct btrfs_root *root);
> 
> Why the explicit noinline? If it's for "documentation"/appearing in
> stack traces purposes then you should use noinline_for_stack. But I
> don't think it's really needed.

Just to keep the original declaration.
Also, the third patch returns this function into static again (in ioctl.c).

> 
>>  int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
>>  			int front);
>>  int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index f53470112670..db66fa4fede6 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -4333,6 +4333,60 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
>>  	return ret;
>>  }
>>  
>> +/*
>> + * helper to check if the subvolume references other subvolumes
>> + */
>> +noinline int may_destroy_subvol(struct btrfs_root *root)
>> +{
>> +	struct btrfs_fs_info *fs_info = root->fs_info;
>> +	struct btrfs_path *path;
>> +	struct btrfs_dir_item *di;
>> +	struct btrfs_key key;
>> +	u64 dir_id;
>> +	int ret;
>> +
>> +	path = btrfs_alloc_path();
>> +	if (!path)
>> +		return -ENOMEM;
>> +
>> +	/* Make sure this root isn't set as the default subvol */
>> +	dir_id = btrfs_super_root_dir(fs_info->super_copy);
>> +	di = btrfs_lookup_dir_item(NULL, fs_info->tree_root, path,
>> +				   dir_id, "default", 7, 0);
>> +	if (di && !IS_ERR(di)) {
>> +		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &key);
>> +		if (key.objectid == root->root_key.objectid) {
>> +			ret = -EPERM;
>> +			btrfs_err(fs_info,
>> +				  "deleting default subvolume %llu is not allowed",
>> +				  key.objectid);
>> +			goto out;
>> +		}
>> +		btrfs_release_path(path);
>> +	}
>> +
>> +	key.objectid = root->root_key.objectid;
>> +	key.type = BTRFS_ROOT_REF_KEY;
>> +	key.offset = (u64)-1;
>> +
>> +	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
>> +	if (ret < 0)
>> +		goto out;
>> +	BUG_ON(ret == 0);
>> +
>> +	ret = 0;
>> +	if (path->slots[0] > 0) {
>> +		path->slots[0]--;
>> +		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +		if (key.objectid == root->root_key.objectid &&
>> +		    key.type == BTRFS_ROOT_REF_KEY)
>> +			ret = -ENOTEMPTY;
>> +	}
>> +out:
>> +	btrfs_free_path(path);
>> +	return ret;
>> +}
>> +
>>  static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
>>  {
>>  	struct inode *inode = d_inode(dentry);
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 111ee282b777..11af9eb449ef 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -1843,60 +1843,6 @@ static noinline int btrfs_ioctl_subvol_setflags(struct file *file,
>>  	return ret;
>>  }
>>  
>> -/*
>> - * helper to check if the subvolume references other subvolumes
>> - */
>> -static noinline int may_destroy_subvol(struct btrfs_root *root)
>> -{
>> -	struct btrfs_fs_info *fs_info = root->fs_info;
>> -	struct btrfs_path *path;
>> -	struct btrfs_dir_item *di;
>> -	struct btrfs_key key;
>> -	u64 dir_id;
>> -	int ret;
>> -
>> -	path = btrfs_alloc_path();
>> -	if (!path)
>> -		return -ENOMEM;
>> -
>> -	/* Make sure this root isn't set as the default subvol */
>> -	dir_id = btrfs_super_root_dir(fs_info->super_copy);
>> -	di = btrfs_lookup_dir_item(NULL, fs_info->tree_root, path,
>> -				   dir_id, "default", 7, 0);
>> -	if (di && !IS_ERR(di)) {
>> -		btrfs_dir_item_key_to_cpu(path->nodes[0], di, &key);
>> -		if (key.objectid == root->root_key.objectid) {
>> -			ret = -EPERM;
>> -			btrfs_err(fs_info,
>> -				  "deleting default subvolume %llu is not allowed",
>> -				  key.objectid);
>> -			goto out;
>> -		}
>> -		btrfs_release_path(path);
>> -	}
>> -
>> -	key.objectid = root->root_key.objectid;
>> -	key.type = BTRFS_ROOT_REF_KEY;
>> -	key.offset = (u64)-1;
>> -
>> -	ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0, 0);
>> -	if (ret < 0)
>> -		goto out;
>> -	BUG_ON(ret == 0);
>> -
>> -	ret = 0;
>> -	if (path->slots[0] > 0) {
>> -		path->slots[0]--;
>> -		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> -		if (key.objectid == root->root_key.objectid &&
>> -		    key.type == BTRFS_ROOT_REF_KEY)
>> -			ret = -ENOTEMPTY;
>> -	}
>> -out:
>> -	btrfs_free_path(path);
>> -	return ret;
>> -}
>> -
>>  static noinline int key_in_sk(struct btrfs_key *key,
>>  			      struct btrfs_ioctl_search_key *sk)
>>  {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume

[Misono Tomohiro]

This patch changes the behavior of rmdir(2) to allow it to delete
an empty subvolume by default, unless it is not a default subvolume
and send is not in progress.

New function btrfs_delete_subvolume() is almost equal to the second half
of btrfs_ioctl_snap_destroy(). This function requires inode_lock for both
@dir and inode of @dentry. For rmdir(2) it is already acquired in vfs
layer before calling btrfs_rmdir().

Note that while a non-privileged user cannot delete a read-only subvolume
by "btrfs subvolume delete" when user_subvol_rm_allowd mount option is
enabled, rmdir(2) can delete an empty read-only subvolume.
(However, rm -r cannot use for read-only subvolume containing files.)

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 fs/btrfs/inode.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index db66fa4fede6..b778776eee8e 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4387,6 +4387,147 @@ noinline int may_destroy_subvol(struct btrfs_root *root)
 	return ret;
 }
 
+static int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry)
+{
+	struct btrfs_fs_info *fs_info = btrfs_sb(dentry->d_sb);
+	struct btrfs_root *root = BTRFS_I(dir)->root;
+	struct inode *inode = d_inode(dentry);
+	struct btrfs_root *dest = BTRFS_I(inode)->root;
+	struct btrfs_trans_handle *trans;
+	struct btrfs_block_rsv block_rsv;
+	u64 root_flags;
+	u64 qgroup_reserved;
+	int ret;
+	int err;
+
+	/*
+	 * Don't allow to delete a subvolume with send in progress. This is
+	 * inside the i_mutex so the error handling that has to drop the bit
+	 * again is not run concurrently.
+	 */
+	spin_lock(&dest->root_item_lock);
+	root_flags = btrfs_root_flags(&dest->root_item);
+	if (dest->send_in_progress == 0) {
+		btrfs_set_root_flags(&dest->root_item,
+				root_flags | BTRFS_ROOT_SUBVOL_DEAD);
+		spin_unlock(&dest->root_item_lock);
+	} else {
+		spin_unlock(&dest->root_item_lock);
+		btrfs_warn(fs_info,
+			   "Attempt to delete subvolume %llu during send",
+			   dest->root_key.objectid);
+		err = -EPERM;
+		return err;
+	}
+
+	down_write(&fs_info->subvol_sem);
+
+	err = may_destroy_subvol(dest);
+	if (err)
+		goto out_up_write;
+
+	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
+	/*
+	 * One for dir inode, two for dir entries, two for root
+	 * ref/backref.
+	 */
+	err = btrfs_subvolume_reserve_metadata(root, &block_rsv,
+					       5, &qgroup_reserved, true);
+	if (err)
+		goto out_up_write;
+
+	trans = btrfs_start_transaction(root, 0);
+	if (IS_ERR(trans)) {
+		err = PTR_ERR(trans);
+		goto out_release;
+	}
+	trans->block_rsv = &block_rsv;
+	trans->bytes_reserved = block_rsv.size;
+
+	btrfs_record_snapshot_destroy(trans, BTRFS_I(dir));
+
+	ret = btrfs_unlink_subvol(trans, root, dir,
+				dest->root_key.objectid,
+				dentry->d_name.name,
+				dentry->d_name.len);
+	if (ret) {
+		err = ret;
+		btrfs_abort_transaction(trans, ret);
+		goto out_end_trans;
+	}
+
+	btrfs_record_root_in_trans(trans, dest);
+
+	memset(&dest->root_item.drop_progress, 0,
+		sizeof(dest->root_item.drop_progress));
+	dest->root_item.drop_level = 0;
+	btrfs_set_root_refs(&dest->root_item, 0);
+
+	if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &dest->state)) {
+		ret = btrfs_insert_orphan_item(trans,
+					fs_info->tree_root,
+					dest->root_key.objectid);
+		if (ret) {
+			btrfs_abort_transaction(trans, ret);
+			err = ret;
+			goto out_end_trans;
+		}
+	}
+
+	ret = btrfs_uuid_tree_rem(trans, fs_info, dest->root_item.uuid,
+				  BTRFS_UUID_KEY_SUBVOL,
+				  dest->root_key.objectid);
+	if (ret && ret != -ENOENT) {
+		btrfs_abort_transaction(trans, ret);
+		err = ret;
+		goto out_end_trans;
+	}
+	if (!btrfs_is_empty_uuid(dest->root_item.received_uuid)) {
+		ret = btrfs_uuid_tree_rem(trans, fs_info,
+					  dest->root_item.received_uuid,
+					  BTRFS_UUID_KEY_RECEIVED_SUBVOL,
+					  dest->root_key.objectid);
+		if (ret && ret != -ENOENT) {
+			btrfs_abort_transaction(trans, ret);
+			err = ret;
+			goto out_end_trans;
+		}
+	}
+
+out_end_trans:
+	trans->block_rsv = NULL;
+	trans->bytes_reserved = 0;
+	ret = btrfs_end_transaction(trans);
+	if (ret && !err)
+		err = ret;
+	inode->i_flags |= S_DEAD;
+out_release:
+	btrfs_subvolume_release_metadata(fs_info, &block_rsv);
+out_up_write:
+	up_write(&fs_info->subvol_sem);
+	if (err) {
+		spin_lock(&dest->root_item_lock);
+		root_flags = btrfs_root_flags(&dest->root_item);
+		btrfs_set_root_flags(&dest->root_item,
+				root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
+		spin_unlock(&dest->root_item_lock);
+	}
+
+	if (!err) {
+		d_invalidate(dentry);
+		btrfs_invalidate_inodes(dest);
+		ASSERT(dest->send_in_progress == 0);
+
+		/* the last ref */
+		if (dest->ino_cache_inode) {
+			iput(dest->ino_cache_inode);
+			dest->ino_cache_inode = NULL;
+		}
+	}
+
+	return err;
+}
+
 static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
@@ -4398,7 +4539,7 @@ static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
 	if (inode->i_size > BTRFS_EMPTY_DIR_SIZE)
 		return -ENOTEMPTY;
 	if (btrfs_ino(BTRFS_I(inode)) == BTRFS_FIRST_FREE_OBJECTID)
-		return -EPERM;
+		return btrfs_delete_subvolume(dir, dentry);
 
 	trans = __unlink_start_trans(dir);
 	if (IS_ERR(trans))
-- 
2.14.3

Re: [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume

[Nikolay Borisov]

On 26.03.2018 11:30, Misono Tomohiro wrote:
> This patch changes the behavior of rmdir(2) to allow it to delete
> an empty subvolume by default, unless it is not a default subvolume
> and send is not in progress.
> 
> New function btrfs_delete_subvolume() is almost equal to the second half
> of btrfs_ioctl_snap_destroy(). This function requires inode_lock for both
> @dir and inode of @dentry. For rmdir(2) it is already acquired in vfs
> layer before calling btrfs_rmdir().
> 
> Note that while a non-privileged user cannot delete a read-only subvolume
> by "btrfs subvolume delete" when user_subvol_rm_allowd mount option is
> enabled, rmdir(2) can delete an empty read-only subvolume.
> (However, rm -r cannot use for read-only subvolume containing files.)
> 
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> ---
>  fs/btrfs/inode.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 142 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index db66fa4fede6..b778776eee8e 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4387,6 +4387,147 @@ noinline int may_destroy_subvol(struct btrfs_root *root)
>  	return ret;
>  }
>  
> +static int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry)
> +{
> +	struct btrfs_fs_info *fs_info = btrfs_sb(dentry->d_sb);
> +	struct btrfs_root *root = BTRFS_I(dir)->root;
> +	struct inode *inode = d_inode(dentry);
> +	struct btrfs_root *dest = BTRFS_I(inode)->root;
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_block_rsv block_rsv;
> +	u64 root_flags;
> +	u64 qgroup_reserved;
> +	int ret;
> +	int err;
> +
> +	/*
> +	 * Don't allow to delete a subvolume with send in progress. This is
> +	 * inside the i_mutex so the error handling that has to drop the bit
> +	 * again is not run concurrently.
> +	 */
> +	spin_lock(&dest->root_item_lock);
> +	root_flags = btrfs_root_flags(&dest->root_item);
> +	if (dest->send_in_progress == 0) {
> +		btrfs_set_root_flags(&dest->root_item,
> +				root_flags | BTRFS_ROOT_SUBVOL_DEAD);
> +		spin_unlock(&dest->root_item_lock);
> +	} else {
> +		spin_unlock(&dest->root_item_lock);
> +		btrfs_warn(fs_info,
> +			   "Attempt to delete subvolume %llu during send",
> +			   dest->root_key.objectid);
> +		err = -EPERM;
> +		return err;
> +	}
> +
> +	down_write(&fs_info->subvol_sem);
> +
> +	err = may_destroy_subvol(dest);
> +	if (err)
> +		goto out_up_write;
> +
> +	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
> +	/*
> +	 * One for dir inode, two for dir entries, two for root
> +	 * ref/backref.
> +	 */
> +	err = btrfs_subvolume_reserve_metadata(root, &block_rsv,
> +					       5, &qgroup_reserved, true);
> +	if (err)
> +		goto out_up_write;
> +
> +	trans = btrfs_start_transaction(root, 0);
> +	if (IS_ERR(trans)) {
> +		err = PTR_ERR(trans);
> +		goto out_release;
> +	}
> +	trans->block_rsv = &block_rsv;
> +	trans->bytes_reserved = block_rsv.size;
> +
> +	btrfs_record_snapshot_destroy(trans, BTRFS_I(dir));
> +
> +	ret = btrfs_unlink_subvol(trans, root, dir,
> +				dest->root_key.objectid,
> +				dentry->d_name.name,
> +				dentry->d_name.len);
> +	if (ret) {
> +		err = ret;
> +		btrfs_abort_transaction(trans, ret);
> +		goto out_end_trans;
> +	}
> +
> +	btrfs_record_root_in_trans(trans, dest);
> +
> +	memset(&dest->root_item.drop_progress, 0,
> +		sizeof(dest->root_item.drop_progress));
> +	dest->root_item.drop_level = 0;
> +	btrfs_set_root_refs(&dest->root_item, 0);
> +
> +	if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &dest->state)) {
> +		ret = btrfs_insert_orphan_item(trans,
> +					fs_info->tree_root,
> +					dest->root_key.objectid);
> +		if (ret) {
> +			btrfs_abort_transaction(trans, ret);
> +			err = ret;
> +			goto out_end_trans;
> +		}
> +	}
> +
> +	ret = btrfs_uuid_tree_rem(trans, fs_info, dest->root_item.uuid,
> +				  BTRFS_UUID_KEY_SUBVOL,
> +				  dest->root_key.objectid);
> +	if (ret && ret != -ENOENT) {
> +		btrfs_abort_transaction(trans, ret);
> +		err = ret;
> +		goto out_end_trans;
> +	}
> +	if (!btrfs_is_empty_uuid(dest->root_item.received_uuid)) {
> +		ret = btrfs_uuid_tree_rem(trans, fs_info,
> +					  dest->root_item.received_uuid,
> +					  BTRFS_UUID_KEY_RECEIVED_SUBVOL,
> +					  dest->root_key.objectid);
> +		if (ret && ret != -ENOENT) {
> +			btrfs_abort_transaction(trans, ret);
> +			err = ret;
> +			goto out_end_trans;
> +		}
> +	}
> +
> +out_end_trans:
> +	trans->block_rsv = NULL;
> +	trans->bytes_reserved = 0;
> +	ret = btrfs_end_transaction(trans);
> +	if (ret && !err)
> +		err = ret;
> +	inode->i_flags |= S_DEAD;
> +out_release:
> +	btrfs_subvolume_release_metadata(fs_info, &block_rsv);
> +out_up_write:
> +	up_write(&fs_info->subvol_sem);
> +	if (err) {
> +		spin_lock(&dest->root_item_lock);
> +		root_flags = btrfs_root_flags(&dest->root_item);
> +		btrfs_set_root_flags(&dest->root_item,
> +				root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
> +		spin_unlock(&dest->root_item_lock);
> +	}
> +
> +	if (!err) {

nit: Why 2 if branches and not a single if {} else {} construct?

> +		d_invalidate(dentry);
> +		btrfs_invalidate_inodes(dest);
> +		ASSERT(dest->send_in_progress == 0);
> +
> +		/* the last ref */
> +		if (dest->ino_cache_inode) {
> +			iput(dest->ino_cache_inode);
> +			dest->ino_cache_inode = NULL;
> +		}
> +	}
> +
> +	return err;
> +}
> +
>  static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
>  {
>  	struct inode *inode = d_inode(dentry);
> @@ -4398,7 +4539,7 @@ static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
>  	if (inode->i_size > BTRFS_EMPTY_DIR_SIZE)
>  		return -ENOTEMPTY;
>  	if (btrfs_ino(BTRFS_I(inode)) == BTRFS_FIRST_FREE_OBJECTID)
> -		return -EPERM;
> +		return btrfs_delete_subvolume(dir, dentry);
>  
>  	trans = __unlink_start_trans(dir);
>  	if (IS_ERR(trans))
> 

Re: [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume

[Misono Tomohiro]

On 2018/03/27 21:39, Nikolay Borisov wrote:
> 
> 
> On 26.03.2018 11:30, Misono Tomohiro wrote:
>> This patch changes the behavior of rmdir(2) to allow it to delete
>> an empty subvolume by default, unless it is not a default subvolume
>> and send is not in progress.
>>
>> New function btrfs_delete_subvolume() is almost equal to the second half
>> of btrfs_ioctl_snap_destroy(). This function requires inode_lock for both
>> @dir and inode of @dentry. For rmdir(2) it is already acquired in vfs
>> layer before calling btrfs_rmdir().
>>
>> Note that while a non-privileged user cannot delete a read-only subvolume
>> by "btrfs subvolume delete" when user_subvol_rm_allowd mount option is
>> enabled, rmdir(2) can delete an empty read-only subvolume.
>> (However, rm -r cannot use for read-only subvolume containing files.)
>>
>> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
>> ---
>>  fs/btrfs/inode.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index db66fa4fede6..b778776eee8e 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -4387,6 +4387,147 @@ noinline int may_destroy_subvol(struct btrfs_root *root)
>>  	return ret;
>>  }
>>  
>> +static int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry)
>> +{
>> +	struct btrfs_fs_info *fs_info = btrfs_sb(dentry->d_sb);
>> +	struct btrfs_root *root = BTRFS_I(dir)->root;
>> +	struct inode *inode = d_inode(dentry);
>> +	struct btrfs_root *dest = BTRFS_I(inode)->root;
>> +	struct btrfs_trans_handle *trans;
>> +	struct btrfs_block_rsv block_rsv;
>> +	u64 root_flags;
>> +	u64 qgroup_reserved;
>> +	int ret;
>> +	int err;
>> +
>> +	/*
>> +	 * Don't allow to delete a subvolume with send in progress. This is
>> +	 * inside the i_mutex so the error handling that has to drop the bit
>> +	 * again is not run concurrently.
>> +	 */
>> +	spin_lock(&dest->root_item_lock);
>> +	root_flags = btrfs_root_flags(&dest->root_item);
>> +	if (dest->send_in_progress == 0) {
>> +		btrfs_set_root_flags(&dest->root_item,
>> +				root_flags | BTRFS_ROOT_SUBVOL_DEAD);
>> +		spin_unlock(&dest->root_item_lock);
>> +	} else {
>> +		spin_unlock(&dest->root_item_lock);
>> +		btrfs_warn(fs_info,
>> +			   "Attempt to delete subvolume %llu during send",
>> +			   dest->root_key.objectid);
>> +		err = -EPERM;
>> +		return err;
>> +	}
>> +
>> +	down_write(&fs_info->subvol_sem);
>> +
>> +	err = may_destroy_subvol(dest);
>> +	if (err)
>> +		goto out_up_write;
>> +
>> +	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
>> +	/*
>> +	 * One for dir inode, two for dir entries, two for root
>> +	 * ref/backref.
>> +	 */
>> +	err = btrfs_subvolume_reserve_metadata(root, &block_rsv,
>> +					       5, &qgroup_reserved, true);
>> +	if (err)
>> +		goto out_up_write;
>> +
>> +	trans = btrfs_start_transaction(root, 0);
>> +	if (IS_ERR(trans)) {
>> +		err = PTR_ERR(trans);
>> +		goto out_release;
>> +	}
>> +	trans->block_rsv = &block_rsv;
>> +	trans->bytes_reserved = block_rsv.size;
>> +
>> +	btrfs_record_snapshot_destroy(trans, BTRFS_I(dir));
>> +
>> +	ret = btrfs_unlink_subvol(trans, root, dir,
>> +				dest->root_key.objectid,
>> +				dentry->d_name.name,
>> +				dentry->d_name.len);
>> +	if (ret) {
>> +		err = ret;
>> +		btrfs_abort_transaction(trans, ret);
>> +		goto out_end_trans;
>> +	}
>> +
>> +	btrfs_record_root_in_trans(trans, dest);
>> +
>> +	memset(&dest->root_item.drop_progress, 0,
>> +		sizeof(dest->root_item.drop_progress));
>> +	dest->root_item.drop_level = 0;
>> +	btrfs_set_root_refs(&dest->root_item, 0);
>> +
>> +	if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &dest->state)) {
>> +		ret = btrfs_insert_orphan_item(trans,
>> +					fs_info->tree_root,
>> +					dest->root_key.objectid);
>> +		if (ret) {
>> +			btrfs_abort_transaction(trans, ret);
>> +			err = ret;
>> +			goto out_end_trans;
>> +		}
>> +	}
>> +
>> +	ret = btrfs_uuid_tree_rem(trans, fs_info, dest->root_item.uuid,
>> +				  BTRFS_UUID_KEY_SUBVOL,
>> +				  dest->root_key.objectid);
>> +	if (ret && ret != -ENOENT) {
>> +		btrfs_abort_transaction(trans, ret);
>> +		err = ret;
>> +		goto out_end_trans;
>> +	}
>> +	if (!btrfs_is_empty_uuid(dest->root_item.received_uuid)) {
>> +		ret = btrfs_uuid_tree_rem(trans, fs_info,
>> +					  dest->root_item.received_uuid,
>> +					  BTRFS_UUID_KEY_RECEIVED_SUBVOL,
>> +					  dest->root_key.objectid);
>> +		if (ret && ret != -ENOENT) {
>> +			btrfs_abort_transaction(trans, ret);
>> +			err = ret;
>> +			goto out_end_trans;
>> +		}
>> +	}
>> +
>> +out_end_trans:
>> +	trans->block_rsv = NULL;
>> +	trans->bytes_reserved = 0;
>> +	ret = btrfs_end_transaction(trans);
>> +	if (ret && !err)
>> +		err = ret;
>> +	inode->i_flags |= S_DEAD;
>> +out_release:
>> +	btrfs_subvolume_release_metadata(fs_info, &block_rsv);
>> +out_up_write:
>> +	up_write(&fs_info->subvol_sem);
>> +	if (err) {
>> +		spin_lock(&dest->root_item_lock);
>> +		root_flags = btrfs_root_flags(&dest->root_item);
>> +		btrfs_set_root_flags(&dest->root_item,
>> +				root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
>> +		spin_unlock(&dest->root_item_lock);
>> +	}
>> +
>> +	if (!err) {
> 
> nit: Why 2 if branches and not a single if {} else {} construct?

Thanks, I will update in v2.

> 
>> +		d_invalidate(dentry);
>> +		btrfs_invalidate_inodes(dest);
>> +		ASSERT(dest->send_in_progress == 0);
>> +
>> +		/* the last ref */
>> +		if (dest->ino_cache_inode) {
>> +			iput(dest->ino_cache_inode);
>> +			dest->ino_cache_inode = NULL;
>> +		}
>> +	}
>> +
>> +	return err;
>> +}
>> +
>>  static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
>>  {
>>  	struct inode *inode = d_inode(dentry);
>> @@ -4398,7 +4539,7 @@ static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
>>  	if (inode->i_size > BTRFS_EMPTY_DIR_SIZE)
>>  		return -ENOTEMPTY;
>>  	if (btrfs_ino(BTRFS_I(inode)) == BTRFS_FIRST_FREE_OBJECTID)
>> -		return -EPERM;
>> +		return btrfs_delete_subvolume(dir, dentry);
>>  
>>  	trans = __unlink_start_trans(dir);
>>  	if (IS_ERR(trans))
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v2 3/3] btrfs: Cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume()

[Misono Tomohiro]

Use btrfs_delete_subvolume() in btrfs_ioctl_snap_destroy() too to
cleanup the code. Call of d_delete() is still required since
btrfs_delete_subvolume() does not call it (for rmdir(2), vfs layer later
calls it).

As a result, btrfs_unlink_subvol() and may_destroy_subvol()
become static functions. No functional change happens.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 fs/btrfs/ctree.h |   6 +--
 fs/btrfs/inode.c |   6 +--
 fs/btrfs/ioctl.c | 131 +------------------------------------------------------
 3 files changed, 6 insertions(+), 137 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 2dbead106aab..6f23f0694ac3 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3162,11 +3162,7 @@ int btrfs_unlink_inode(struct btrfs_trans_handle *trans,
 int btrfs_add_link(struct btrfs_trans_handle *trans,
 		   struct btrfs_inode *parent_inode, struct btrfs_inode *inode,
 		   const char *name, int name_len, int add_backref, u64 index);
-int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
-			struct btrfs_root *root,
-			struct inode *dir, u64 objectid,
-			const char *name, int name_len);
-noinline int may_destroy_subvol(struct btrfs_root *root);
+int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry);
 int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 			int front);
 int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b778776eee8e..c23e41e6cdfe 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4252,7 +4252,7 @@ static int btrfs_unlink(struct inode *dir, struct dentry *dentry)
 	return ret;
 }
 
-int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
+static int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
 			struct btrfs_root *root,
 			struct inode *dir, u64 objectid,
 			const char *name, int name_len)
@@ -4336,7 +4336,7 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
 /*
  * helper to check if the subvolume references other subvolumes
  */
-noinline int may_destroy_subvol(struct btrfs_root *root)
+static noinline int may_destroy_subvol(struct btrfs_root *root)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_path *path;
@@ -4387,7 +4387,7 @@ noinline int may_destroy_subvol(struct btrfs_root *root)
 	return ret;
 }
 
-static int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry)
+int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(dentry->d_sb);
 	struct btrfs_root *root = BTRFS_I(dir)->root;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 11af9eb449ef..7559a1a82e6d 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2266,12 +2266,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 	struct btrfs_root *root = BTRFS_I(dir)->root;
 	struct btrfs_root *dest = NULL;
 	struct btrfs_ioctl_vol_args *vol_args;
-	struct btrfs_trans_handle *trans;
-	struct btrfs_block_rsv block_rsv;
-	u64 root_flags;
-	u64 qgroup_reserved;
 	int namelen;
-	int ret;
 	int err = 0;
 
 	if (!S_ISDIR(dir->i_mode))
@@ -2355,133 +2350,11 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 	}
 
 	inode_lock(inode);
-
-	/*
-	 * Don't allow to delete a subvolume with send in progress. This is
-	 * inside the i_mutex so the error handling that has to drop the bit
-	 * again is not run concurrently.
-	 */
-	spin_lock(&dest->root_item_lock);
-	root_flags = btrfs_root_flags(&dest->root_item);
-	if (dest->send_in_progress == 0) {
-		btrfs_set_root_flags(&dest->root_item,
-				root_flags | BTRFS_ROOT_SUBVOL_DEAD);
-		spin_unlock(&dest->root_item_lock);
-	} else {
-		spin_unlock(&dest->root_item_lock);
-		btrfs_warn(fs_info,
-			   "Attempt to delete subvolume %llu during send",
-			   dest->root_key.objectid);
-		err = -EPERM;
-		goto out_unlock_inode;
-	}
-
-	down_write(&fs_info->subvol_sem);
-
-	err = may_destroy_subvol(dest);
-	if (err)
-		goto out_up_write;
-
-	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
-	/*
-	 * One for dir inode, two for dir entries, two for root
-	 * ref/backref.
-	 */
-	err = btrfs_subvolume_reserve_metadata(root, &block_rsv,
-					       5, &qgroup_reserved, true);
-	if (err)
-		goto out_up_write;
-
-	trans = btrfs_start_transaction(root, 0);
-	if (IS_ERR(trans)) {
-		err = PTR_ERR(trans);
-		goto out_release;
-	}
-	trans->block_rsv = &block_rsv;
-	trans->bytes_reserved = block_rsv.size;
-
-	btrfs_record_snapshot_destroy(trans, BTRFS_I(dir));
-
-	ret = btrfs_unlink_subvol(trans, root, dir,
-				dest->root_key.objectid,
-				dentry->d_name.name,
-				dentry->d_name.len);
-	if (ret) {
-		err = ret;
-		btrfs_abort_transaction(trans, ret);
-		goto out_end_trans;
-	}
-
-	btrfs_record_root_in_trans(trans, dest);
-
-	memset(&dest->root_item.drop_progress, 0,
-		sizeof(dest->root_item.drop_progress));
-	dest->root_item.drop_level = 0;
-	btrfs_set_root_refs(&dest->root_item, 0);
-
-	if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, &dest->state)) {
-		ret = btrfs_insert_orphan_item(trans,
-					fs_info->tree_root,
-					dest->root_key.objectid);
-		if (ret) {
-			btrfs_abort_transaction(trans, ret);
-			err = ret;
-			goto out_end_trans;
-		}
-	}
-
-	ret = btrfs_uuid_tree_rem(trans, fs_info, dest->root_item.uuid,
-				  BTRFS_UUID_KEY_SUBVOL,
-				  dest->root_key.objectid);
-	if (ret && ret != -ENOENT) {
-		btrfs_abort_transaction(trans, ret);
-		err = ret;
-		goto out_end_trans;
-	}
-	if (!btrfs_is_empty_uuid(dest->root_item.received_uuid)) {
-		ret = btrfs_uuid_tree_rem(trans, fs_info,
-					  dest->root_item.received_uuid,
-					  BTRFS_UUID_KEY_RECEIVED_SUBVOL,
-					  dest->root_key.objectid);
-		if (ret && ret != -ENOENT) {
-			btrfs_abort_transaction(trans, ret);
-			err = ret;
-			goto out_end_trans;
-		}
-	}
-
-out_end_trans:
-	trans->block_rsv = NULL;
-	trans->bytes_reserved = 0;
-	ret = btrfs_end_transaction(trans);
-	if (ret && !err)
-		err = ret;
-	inode->i_flags |= S_DEAD;
-out_release:
-	btrfs_subvolume_release_metadata(fs_info, &block_rsv);
-out_up_write:
-	up_write(&fs_info->subvol_sem);
-	if (err) {
-		spin_lock(&dest->root_item_lock);
-		root_flags = btrfs_root_flags(&dest->root_item);
-		btrfs_set_root_flags(&dest->root_item,
-				root_flags & ~BTRFS_ROOT_SUBVOL_DEAD);
-		spin_unlock(&dest->root_item_lock);
-	}
-out_unlock_inode:
+	err = btrfs_delete_subvolume(dir, dentry);
 	inode_unlock(inode);
-	if (!err) {
-		d_invalidate(dentry);
-		btrfs_invalidate_inodes(dest);
+	if (!err)
 		d_delete(dentry);
-		ASSERT(dest->send_in_progress == 0);
 
-		/* the last ref */
-		if (dest->ino_cache_inode) {
-			iput(dest->ino_cache_inode);
-			dest->ino_cache_inode = NULL;
-		}
-	}
 out_dput:
 	dput(dentry);
 out_unlock_dir:
-- 
2.14.3

Re: [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume

[Goffredo Baroncelli]

Hi Misono,

On 03/26/2018 10:28 AM, Misono Tomohiro wrote:
> changelog:
>   v1 -> v2 ... split the patch to hopefully make review easier
> 
> 1st patch is a preparation work just moving the declaration of
> may_destroy_subvol().
> 
> 2nd patch is the main part. New function btrfs_delete_subvolume() is
> introduced and used in btrfs_rmdir() when a direcoty is an empty
> subvolume. The function is almost the copy of second half of
> btrfs_ioctl_snap_destroy().
> The code path for "sub delete" is not changed yet.
> 
> 3rd patch is a cleanup of btrfs_ioctl_snap_destroy() and uses 
> brrfs_delete_subvolume() for "sub delete" too.
> 
> Tomohiro Misono (3):
>   btrfs: move may_destroy_subvol() from ioctl.c to inode.c
>   btrfs: Allow rmdir(2) to delete a subvolume
>   btrfs: cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume()
> 
>  fs/btrfs/ctree.h |   5 +-
>  fs/btrfs/inode.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  fs/btrfs/ioctl.c | 185 +--------------------------------------------------
>  3 files changed, 200 insertions(+), 189 deletions(-)
> 

I checked this patch, and it works. You can add
Tested-by: Goffredo Baroncelli <kreijack@inwind.it>

---------

ghigo@venice:/tmp$ btrfs sub cre a
Create subvolume './a'
ghigo@venice:/tmp$ btrfs sub cre a/b
Create subvolume 'a/b'
ghigo@venice:/tmp$ btrfs sub cre a/b/d
Create subvolume 'a/b/d'
ghigo@venice:/tmp$ touch a/b/d/c
ghigo@venice:/tmp$ rm -rf a
ghigo@venice:/tmp$ ls -la
total 4
drwxrwxrwt 1 root  root  492 Mar 26 21:18 .
drwxr-xr-x 1 root  root  196 Mar 14 19:41 ..
drwxrwxrwt 1 root  root    0 Mar 26 21:17 .font-unix
drwxrwxrwt 1 root  root    8 Mar 26 21:18 .ICE-unix
drwxrwxrwt 1 root  root    0 Mar 26 21:17 .Test-unix
-r--r--r-- 1 root  root   11 Mar 26 21:17 .X0-lock
drwxrwxrwt 1 root  root    4 Mar 26 21:17 .X11-unix
drwxrwxrwt 1 root  root    0 Mar 26 21:17 .XIM-unix



ghigo@venice:/tmp$ btrfs sub crea a
Create subvolume './a'
ghigo@venice:/tmp$ touch a/b
ghigo@venice:/tmp$ rmdir a
rmdir: failed to remove 'a': Directory not empty
ghigo@venice:/tmp$ rm a/b 
ghigo@venice:/tmp$ rmdir a



ghigo@venice:/tmp$ btrfs sub cre d
Create subvolume './d'
ghigo@venice:/tmp$ su - pippo
Password: 
pippo@venice:~$ cd /tmp/
pippo@venice:/tmp$ rmdir d
rmdir: failed to remove 'd': Operation not permitted
pippo@venice:/tmp$ sudo chown pippo d
[sudo] password for pippo: 
pippo@venice:/tmp$ rmdir d

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5