* [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume
@ 2018-03-26 8:28 Misono Tomohiro
2018-03-26 8:28 ` [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c Misono Tomohiro
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Misono Tomohiro @ 2018-03-26 8:28 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c
2018-03-26 8:28 [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Misono Tomohiro
@ 2018-03-26 8:28 ` Misono Tomohiro
2018-03-26 8:51 ` Nikolay Borisov
2018-03-26 8:30 ` [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume Misono Tomohiro
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Misono Tomohiro @ 2018-03-26 8:28 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume
2018-03-26 8:28 [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Misono Tomohiro
2018-03-26 8:28 ` [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c Misono Tomohiro
@ 2018-03-26 8:30 ` Misono Tomohiro
2018-03-27 12:39 ` Nikolay Borisov
2018-03-26 8:31 ` [PATCH v2 3/3] btrfs: Cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume() Misono Tomohiro
2018-03-26 19:45 ` [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Goffredo Baroncelli
3 siblings, 1 reply; 9+ messages in thread
From: Misono Tomohiro @ 2018-03-26 8:30 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] btrfs: Cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume()
2018-03-26 8:28 [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Misono Tomohiro
2018-03-26 8:28 ` [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c Misono Tomohiro
2018-03-26 8:30 ` [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume Misono Tomohiro
@ 2018-03-26 8:31 ` Misono Tomohiro
2018-03-26 19:45 ` [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Goffredo Baroncelli
3 siblings, 0 replies; 9+ messages in thread
From: Misono Tomohiro @ 2018-03-26 8:31 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c
2018-03-26 8:28 ` [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c Misono Tomohiro
@ 2018-03-26 8:51 ` Nikolay Borisov
2018-03-28 1:59 ` Misono Tomohiro
0 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2018-03-26 8:51 UTC (permalink / raw)
To: Misono Tomohiro, linux-btrfs
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)
> {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume
2018-03-26 8:28 [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Misono Tomohiro
` (2 preceding siblings ...)
2018-03-26 8:31 ` [PATCH v2 3/3] btrfs: Cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume() Misono Tomohiro
@ 2018-03-26 19:45 ` Goffredo Baroncelli
3 siblings, 0 replies; 9+ messages in thread
From: Goffredo Baroncelli @ 2018-03-26 19:45 UTC (permalink / raw)
To: Misono Tomohiro, linux-btrfs
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume
2018-03-26 8:30 ` [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume Misono Tomohiro
@ 2018-03-27 12:39 ` Nikolay Borisov
2018-03-28 2:00 ` Misono Tomohiro
0 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2018-03-27 12:39 UTC (permalink / raw)
To: Misono Tomohiro, linux-btrfs
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))
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c
2018-03-26 8:51 ` Nikolay Borisov
@ 2018-03-28 1:59 ` Misono Tomohiro
0 siblings, 0 replies; 9+ messages in thread
From: Misono Tomohiro @ 2018-03-28 1:59 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume
2018-03-27 12:39 ` Nikolay Borisov
@ 2018-03-28 2:00 ` Misono Tomohiro
0 siblings, 0 replies; 9+ messages in thread
From: Misono Tomohiro @ 2018-03-28 2:00 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-28 2:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-26 8:28 [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Misono Tomohiro
2018-03-26 8:28 ` [PATCH v2 1/3] btrfs: Move may_destroy_subvol() from ioctl.c to inode.c Misono Tomohiro
2018-03-26 8:51 ` Nikolay Borisov
2018-03-28 1:59 ` Misono Tomohiro
2018-03-26 8:30 ` [PATCH v2 2/3] btrfs: Allow rmdir(2) to delete a subvolume Misono Tomohiro
2018-03-27 12:39 ` Nikolay Borisov
2018-03-28 2:00 ` Misono Tomohiro
2018-03-26 8:31 ` [PATCH v2 3/3] btrfs: Cleanup btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume() Misono Tomohiro
2018-03-26 19:45 ` [PATCH v2 0/3] Allow rmdir(2) to delete a subvolume Goffredo Baroncelli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.