[PATCH v4 4/4] btrfs: Allow rmdir(2) to delete an empty subvolume

[Misono Tomohiro]

Change the behaior of rmdir(2) and allow it to delete an empty subvolume
by calling btrfs_delete_subvolume() which is used by
btrfs_ioctl_snap_destroy(). The required lock for @dir and inode of
@dentry is already acquired in vfs layer.

We need some check before deleting a subvolume. Permission check is done
in vfs layer, emptiness check is in btrfs_rmdir() and additional check
(i.e. neither send is in progress nor the subvolume is a default subvolume)
is in btrfs_delete_subvolume().

Note that in btrfs_snap_destroy(), d_delete() is called after
btrfs_delete_subvolume(). For rmdir(2), d_delete() is called in vfs
layer later.

Tested-by: Goffredo Baroncelli 
Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2db2e860ba90..afc631f28088 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4537,7 +4537,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

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/4] btrfs: Refactor btrfs_ioctl_snap_destroy() by using btrfs_delete_subvolume()

[Misono Tomohiro]

Use btrfs_delete_subvolume() to refactor btrfs_ioctl_snap_destroy().
The permission check is still done in btrfs_ioctl_snap_destroy(). Also,
call of d_delete() is still required since btrfs_delete_subvolume()
does not call it.

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

Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/ctree.h |   5 ---
 fs/btrfs/inode.c |   4 +-
 fs/btrfs/ioctl.c | 131 +--
 3 files changed, 4 insertions(+), 136 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 618365eb9d84..6f23f0694ac3 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3162,11 +3162,6 @@ 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);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 54c50a9fa2ee..2db2e860ba90 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;
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(>root_item_lock);
-   root_flags = btrfs_root_flags(>root_item);
-   if (dest->send_in_progress == 0) {
-   btrfs_set_root_flags(>root_item,
-   root_flags | BTRFS_ROOT_SUBVOL_DEAD);
-   spin_unlock(>root_item_lock);
-   } else {
-   spin_unlock(>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(_info->subvol_sem);
-
-   err = may_destroy_subvol(dest);
-   if (err)
-   goto out_up_write;
-
-   btrfs_init_block_rsv(_rsv, BTRFS_BLOCK_RSV_TEMP);
-   /*
-* One for dir inode, two for dir entries, two for root
-* ref/backref.
-*/
-   err = btrfs_subvolume_reserve_metadata(root, _rsv,
-  5, _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 = _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;
-

[PATCH v4 2/4] btrfs: Add definition of btrfs_delete_subvolume()

[Misono Tomohiro]

Add new function btrfs_delete_subvolume() which is almost identical
to the secand half of btrfs_ioctl_snap_destroy(). This function requires
inode_lock for both @dir and inode of @dentry.

This performes additional check (1. send is not in progress. 2. the
subvolume is not a default subvolume. 3. the subvolume does not contain
other subvoume) and actual deletion process.

The code path is not changed yet and this function will be used to
refactor btrfs_ioctl_snap_destroy() and to allow rmdir(2) to delete an
empty subvolume.

Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/ctree.h |   1 +
 fs/btrfs/inode.c | 139 +++
 2 files changed, 140 insertions(+)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 2dbead106aab..618365eb9d84 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3167,6 +3167,7 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
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 db66fa4fede6..54c50a9fa2ee 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4387,6 +4387,145 @@ noinline int may_destroy_subvol(struct btrfs_root *root)
return ret;
 }
 
+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(>root_item_lock);
+   root_flags = btrfs_root_flags(>root_item);
+   if (dest->send_in_progress == 0) {
+   btrfs_set_root_flags(>root_item,
+   root_flags | BTRFS_ROOT_SUBVOL_DEAD);
+   spin_unlock(>root_item_lock);
+   } else {
+   spin_unlock(>root_item_lock);
+   btrfs_warn(fs_info,
+  "Attempt to delete subvolume %llu during send",
+  dest->root_key.objectid);
+   err = -EPERM;
+   return err;
+   }
+
+   down_write(_info->subvol_sem);
+
+   err = may_destroy_subvol(dest);
+   if (err)
+   goto out_up_write;
+
+   btrfs_init_block_rsv(_rsv, BTRFS_BLOCK_RSV_TEMP);
+   /*
+* One for dir inode, two for dir entries, two for root
+* ref/backref.
+*/
+   err = btrfs_subvolume_reserve_metadata(root, _rsv,
+  5, _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 = _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(>root_item.drop_progress, 0,
+   sizeof(dest->root_item.drop_progress));
+   dest->root_item.drop_level = 0;
+   btrfs_set_root_refs(>root_item, 0);
+
+   if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, >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) {
+   

[PATCH v4 1/4] btrfs: move may_destroy_subvol() from ioctl.c to inode.c

[Misono Tomohiro]

This is a preparation work to refactor btrfs_ioctl_snap_destroy()
and to allow rmdir(2) to delete an empty subvolume.

Signed-off-by: Tomohiro Misono 
---
 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, );
+   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, , 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], , 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, );
-   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, , path, 0, 0);
-   if (ret < 0)
-   goto out;
-   BUG_ON(ret == 0);
-
-   ret = 0;
-   if (path->slots[0] > 0) {
-   path->slots[0]--;
-   

[PATCH v4 0/4] Allow rmdir(2) to delete a subvolume

[Misono Tomohiro]

changelog:
  v3 -> v4 ... Reorganize patches and updates commit log.
   No code change in total
  v2 -> v3 ... Use if-else block instead of two if blocks and 
   add Tested-by tag in 2nd patch
  v1 -> v2 ... Split the patch to hopefully make review easier

Note: I will send a xfstest if this series is merged.

This series changes the behavior of rmdir(2) and allow it to delete an
empty subvolume.

1st to 3rd patches refactor btrfs_ioctl_snap_destroy() and no functional
change happens.

btrfs_ioctl_snap_destroy() is roughly comprised of two parts.
The first part is permission check of the (parent) subvolume. The second
part is additional check (1. send is not in progress. 2. the subvolume
is not a default subvolume. 3. the subvolume does not contain other
subvoume) and actual deletion process.

The aim of 1st to 3rd patches is to  extract the second part as
btrfs_delete_subvolume().

4th patch is just one line but this changes the behavior of rmdir(2)
by calling btrfs_delete_subvolume(). For rmdir(2), permission check is
already done in vfs layer.

Tomohiro Misono (4):
  btrfs: move may_destroy_subvol() from ioctl.c to inode.c
  btrfs: Add definition of btrfs_delete_subvolume()
  btrfs: Refactor btrfs_ioctl_snap_destroy() by using
btrfs_delete_subvolume()
  btrfs: Allow rmdir(2) to delete an empty subvolume

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

-- 
2.14.3


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Misono Tomohiro]

On 2018/04/06 23:46, David Sterba wrote:
> On Fri, Mar 30, 2018 at 03:16:47PM +0900, 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.
>>
>> Tested-by: Goffredo Baroncelli 
>> Signed-off-by: Tomohiro Misono 
>> ---
>>  fs/btrfs/inode.c | 141 
>> ++-
>>  1 file changed, 140 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index db66fa4fede6..84dbb9cafd6b 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -4387,6 +4387,145 @@ 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(>root_item_lock);
>> +root_flags = btrfs_root_flags(>root_item);
>> +if (dest->send_in_progress == 0) {
>> +btrfs_set_root_flags(>root_item,
>> +root_flags | BTRFS_ROOT_SUBVOL_DEAD);
>> +spin_unlock(>root_item_lock);
>> +} else {
>> +spin_unlock(>root_item_lock);
>> +btrfs_warn(fs_info,
>> +   "Attempt to delete subvolume %llu during send",
>> +   dest->root_key.objectid);
>> +err = -EPERM;
>> +return err;
>> +}
>> +
>> +down_write(_info->subvol_sem);
>> +
>> +err = may_destroy_subvol(dest);
>> +if (err)
>> +goto out_up_write;
>> +
>> +btrfs_init_block_rsv(_rsv, BTRFS_BLOCK_RSV_TEMP);
>> +/*
>> + * One for dir inode, two for dir entries, two for root
>> + * ref/backref.
>> + */
>> +err = btrfs_subvolume_reserve_metadata(root, _rsv,
>> +   5, _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 = _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(>root_item.drop_progress, 0,
>> +sizeof(dest->root_item.drop_progress));
>> +dest->root_item.drop_level = 0;
>> +btrfs_set_root_refs(>root_item, 0);
>> +
>> +if (!test_and_set_bit(BTRFS_ROOT_ORPHAN_ITEM_INSERTED, >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,
>> +  

Funds Plans..

[Reem Al]

Hello,

My name is Hon. Ms. Reem Al-Hashimy I am the present Ministry of petroleum in 
UAE. I write to solicit for your partnership in claiming of $47 Million USD 
from a Financial Home in Uk.

The aforementioned fund $47 Million USD is my share percentage from a Oil 
companies that I helped financed, influentially but I don't want my government 
to know about the fund.

Furthermore, as a Ministry of Mining, I am not allowed to be part of such a 
deal, because it's against my country professional practice policy. So I am 
compelled to ask that you stand on my behalf and receive this fund into any 
account that is solely controlled by you. I will compensate you with 40% of the 
total amount involved as gratification for being my partner in the transfer. 
Please reply to my private email: (maithasa...@minister.com)


Regards,
Ms. Reem Al-Hashimy

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs.static: needs libbtrfsutil

[Baruch Siach]

Add libbtrfsutil objects to btrfs.static link command. This fixes static
build failure:

utils.static.o: In function `parse_qgroupid':
utils.c:(.text.parse_qgroupid+0xb0): undefined reference to 
`btrfs_util_is_subvolume'
props.static.o: In function `prop_read_only':
props.c:(.text.prop_read_only+0x70): undefined reference to 
`btrfs_util_set_subvolume_read_only'
...
Makefile:457: recipe for target 'btrfs.static' failed
make[1]: *** [btrfs.static] Error 1

Signed-off-by: Baruch Siach 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 5ba76d2eee40..9b97c36a44a5 100644
--- a/Makefile
+++ b/Makefile
@@ -453,7 +453,7 @@ btrfs: btrfs.o $(objects) $(cmds_objects) $(libs_static)
@echo "[LD] $@"
$(Q)$(CC) -o $@ $^ $(LDFLAGS) $(LIBS) $(LIBS_COMP)
 
-btrfs.static: btrfs.static.o $(static_objects) $(static_cmds_objects) 
$(static_libbtrfs_objects)
+btrfs.static: btrfs.static.o $(static_objects) $(static_cmds_objects) 
$(static_libbtrfs_objects) $(libbtrfsutil_objects)
@echo "[LD] $@"
$(Q)$(CC) -o $@ $^ $(STATIC_LDFLAGS) $(STATIC_LIBS) $(STATIC_LIBS_COMP)
 
-- 
2.16.3

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/3] btrfs: Add three new unprivileged ioctls to allow normal users to call "sub list/show" etc.

[David Sterba]

On Mon, Mar 19, 2018 at 04:27:09PM +0900, Misono, Tomohiro wrote:
> changelog:
> 
> v2-> v3
>  - fix kbuild test bot warning
> v1 -> v2
>   - completely reimplement 1st/2nd ioctl to have user friendly api
>   - various cleanup, remove unnecessary goto
> ===
> 
> This adds three new unprivileged ioctls:
> 
> 1st patch: ioctl which returns subvolume information of ROOT_ITEM and 
> ROOT_BACKREF
> 2nd patch: ioctl which returns subvolume information of ROOT_REF (without 
> subvolume name)
> 3rd patch: user version of ino_lookup ioctl which also peforms permission 
> check.

The overall approach to listing subvolumes looks good. We can enumerate
them, get the relations and the details. Making some sense of that in
the userspace is a whole different and maybe more difficult topic.

I hope we could use the opportunity to clean up the listing commandline
interface and output at the same time, as there's going to be possibly
some incompatibility introduced.

We need to start with current usecases and how they're implemented or
mis-implemented (ie. leading to confusion). The discussions I've read so
far cover a good part of that.

I'll review and add the kernel patches to misc-next to get some testing
coverage. As this is a non-restricted ioctl full of pointers and bytes
shuffled around, we will have to do an extra security-oriented review.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/8] btrfs: use iocb for __btrfs_buffered_write

[David Sterba]

On Fri, Nov 17, 2017 at 11:44:47AM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues 
> 
> Preparatory patch. It reduces the arguments to __btrfs_buffered_write
> to follow buffered_write() style.
> 
> Signed-off-by: Goldwyn Rodrigues 

I got pointed to this patch that it could be applied independently, so
I'm adding it to btrfs queue.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] btrfs: Factor out write portion of btrfs_get_blocks_direct

[David Sterba]

On Thu, Feb 22, 2018 at 06:12:14PM +0200, Nikolay Borisov wrote:
> Now that the read side is extracted into its own function, do the same
> to the write side. This leaves btrfs_get_blocks_direct_write with the
> sole purpose of handling common locking required. Also flip the
> condition in btrfs_get_blocks_direct_write so that the write case
> comes first and we check for if (Create) rather than if (!create). This
> is purely subjective but I believe makes reading a bit more "linear".

I subjectively agree.

> No functional changes.
> 
> Signed-off-by: Nikolay Borisov 

Reviewed-by: David Sterba 

Same comment as before, no __ and please use the single-return exit
block where possible.

> Both patches survived xfstests runs.

I'll add that to misc-next, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] btrfs: Factor out read portion of btrfs_get_blocks_direct

[David Sterba]

On Thu, Feb 22, 2018 at 06:12:13PM +0200, Nikolay Borisov wrote:
> Currently this function handles both the READ and WRITE dio cases. This
> is facilitated by a bunch of 'if' statements, a goto short-circuit
> statement and a very perverse aliasing of "!created"(READ) case
> by setting lockstart = lockend and checking for lockstart < lockend for
> detecting the write. Let's simplify this mess by extracting the
> READ-only code into a separate __btrfs_get_block_direct_read function.
> This is only the first step, the next one will be to factor out the
> write side as well. The end goal will be to have the common locking/
> unlocking code in btrfs_get_blocks_direct and then it will call either
> the read|write subvariants. No functional changes.

Reviewed-by: David Sterba 

uh what a convoluted code to untangle. A few style notes below.
> 
> Signed-off-by: Nikolay Borisov 
> ---
>  fs/btrfs/inode.c | 49 -
>  1 file changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 491a7397f6fa..fd99347d0c91 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7677,6 +7677,27 @@ static struct extent_map *create_io_em(struct inode 
> *inode, u64 start, u64 len,
>   return em;
>  }
>  
> +
> +static int __btrfs_get_blocks_direct_read(struct extent_map *em,

Please drop the "__" the function is static and there's no
underscore-less version.

> +  struct buffer_head *bh_result,
> +  struct inode *inode,
> +  u64 start, u64 len)
> +{
> + if (em->block_start == EXTENT_MAP_HOLE ||
> + test_bit(EXTENT_FLAG_PREALLOC, >flags))
> + return -ENOENT;
> +
> + len = min(len, em->len - (start - em->start));
> +
> + bh_result->b_blocknr = (em->block_start + (start - em->start)) >>
> + inode->i_blkbits;
> + bh_result->b_size = len;
> + bh_result->b_bdev = em->bdev;
> + set_buffer_mapped(bh_result);
> +
> + return 0;
> +}
> +
>  static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
>  struct buffer_head *bh_result, int create)
>  {
> @@ -7745,11 +7766,22 @@ static int btrfs_get_blocks_direct(struct inode 
> *inode, sector_t iblock,
>   goto unlock_err;
>   }
>  
> - /* Just a good old fashioned hole, return */
> - if (!create && (em->block_start == EXTENT_MAP_HOLE ||
> - test_bit(EXTENT_FLAG_PREALLOC, >flags))) {
> + if (!create) {
> + ret = __btrfs_get_blocks_direct_read(em, bh_result, inode,
> +start, len);
> + /* Can be negative only if we read from a hole */
> + if (ret < 0) {
> + ret = 0;
> + free_extent_map(em);
> + goto unlock_err;
> + }
> + /*
> +  * We need to unlock only the end area that we aren't using
> +  * if there is any leftover space
> +  */
> + free_extent_state(cached_state);
>   free_extent_map(em);
> - goto unlock_err;
> + return 0;

Please add a separate label for that, the funcion uses the single exit
block style (labels and one-or-two returns).

>   }
>  
>   /*
> @@ -7761,12 +7793,6 @@ static int btrfs_get_blocks_direct(struct inode 
> *inode, sector_t iblock,
>* just use the extent.
>*
>*/
> - if (!create) {
> - len = min(len, em->len - (start - em->start));
> - lockstart = start + len;
> - goto unlock;
> - }
> -
>   if (test_bit(EXTENT_FLAG_PREALLOC, >flags) ||
>   ((BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW) &&
>em->block_start != EXTENT_MAP_HOLE)) {
> @@ -7853,10 +7879,7 @@ static int btrfs_get_blocks_direct(struct inode 
> *inode, sector_t iblock,
>   clear_extent_bit(_I(inode)->io_tree, lockstart,
>lockend, unlock_bits, 1, 0,
>_state);
> - } else {
> - free_extent_state(cached_state);
>   }
> -
>   free_extent_map(em);
>  
>   return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/7] Superblock read and verify cleanups

[David Sterba]

On Fri, Mar 30, 2018 at 08:09:16AM +0800, Anand Jain wrote:
> v3->v4:
>  Update changelog and signoff.
>  Reintroduce explicit check for '-EUCLEAN'
>   at Patch 2/8 and 5/8.
> 
> v2->v3:
>  Squash
>   4/8 btrfs: make btrfs_check_super_csum() non static
>  to
>   6/8 btrfs: verify superblock checksum during scan
>  As in the individual patch mentioned
>  
> v1->v2:
>  Various changes suggested by Nicokay. Thanks. For specific changes
>  pls ref to the patch.
> 
> Patch 1-4/8 are preparatory patches adds cleanups and nonstatic requisites.
> 
> Patch 5/8 makes sure that all copies of the superblock have the same fsid
> when we scan the device.

This is IMO too strict and the need of workaround puts the burden on the
user while it's not necessary and scan or mount can continue as long as
the first superblock is valid.

> Patch 6/8 verifies superblock csum when we read it in the scan context.

This was the main intention, as I read the discussion in the thread
under the RFC patch.

The semantics of scanning is not exactly defined, in the current code
it's "pick any device that has the right magic and sb offset", so adding
the csum check would add some validation but otherwise it's fine. All
the work is left to mount and all reported errors can be examined
immediatelly.

Wiping the magic either manually or by wipefs will not touch the csum,
so we don't change the behaviour.

Reporting errors on each scan is a usability no-go. Patches 1-4 look ok,
I'll have to think more about 5.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: fix loss of prealloc extents past i_size after fsync log replay

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: c71bf099abdd Btrfs: Avoid orphan inodes cleanup while replaying 
log.

The bot has also determined it's probably a bug fixing patch. (score: 6.2138)

The bot has tested the following trees: v4.16.1, v4.15.16, v4.14.33, v4.9.93, 
v4.4.127.

v4.16.1: Build OK!
v4.15.16: Build OK!
v4.14.33: Build OK!
v4.9.93: Build failed! Errors:
tree-log.c:2367:24: error: ‘struct btrfs_fs_info’ has no member named 
‘sectorsize’
tree-log.c:2367:24: error: ‘struct btrfs_fs_info’ has no member named 
‘sectorsize’
tree-log.c:4224:13: error: ‘struct inode’ has no member named ‘flags’; did 
you mean ‘i_flags’?
tree-log.c:4229:38: error: ‘struct inode’ has no member named ‘vfs_inode’
tree-log.c:4239:4: error: implicit declaration of function ‘refcount_inc’; 
did you mean ‘i_readcount_inc’? [-Werror=implicit-function-declaration]

v4.4.127: Failed to apply! Possible dependencies:
0132761017e0 ("btrfs: fix string and comment grammatical issues and typos")


--
Thanks,
SashaN�r��yb�X��ǧv�^�)޺{.n�+{�n�߲)w*jg����ݢj/���z�ޖ��2�ޙ&�)ߡ�a�����G���h��j:+v���w��٥

Re: How to zero errors

[Austin S. Hemmelgarn]

On 2018-04-10 09:08, James Courtier-Dutton wrote:

Hi,

I have  disk that in the past had errors on it.
I have fixed up the errors.
btrfs scrub now reports no errors.
How do I reset these counters to zero?

BTRFS info (device sdc2): bdev /dev/sdc2 errs: wr 0, rd 35, flush 0,
corrupt 1, gen 0


Run `btrfs device stats` with the `-z` option on the filesystem.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

How to zero errors

[James Courtier-Dutton]

Hi,

I have  disk that in the past had errors on it.
I have fixed up the errors.
btrfs scrub now reports no errors.
How do I reset these counters to zero?

BTRFS info (device sdc2): bdev /dev/sdc2 errs: wr 0, rd 35, flush 0,
corrupt 1, gen 0


Kind Regards


James
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: do not abort transaction when failing to insert hole extent

[David Sterba]

On Mon, Apr 09, 2018 at 06:23:14PM -0700, Liu Bo wrote:
> >>> As maybe_insert_hole is only called by btrfs_cont_expand here, which
> >>> means it's a really hole, I don't expect drop_extents would drop
> >>> anything, we can remove this drop_extents and put an assert after
> >>> btrfs_insert_file_extent for checking EEXIST.
> >> Sounds good.
> > Let me make a v2 and have a fstests run.
> It turns out that the btrfs_drop_extents() here is quite necessary
> since fallocate(2) has a FALLOC_FL_KEEP_SIZE option, and when that
> happens, a hole extent would be appended between the EOF and fallocate
> range's start, then a later truncate up would have to drop these hole
> extents in order to expand with a new hole...

Would it make sense to split the cases where FALLOC_FL_KEEP_SIZE is and
is not used? Either passing an argument or 2 functions where one could
avoid the drop. I've looked at the code only briefly, so this may be a
nonsense in the end.

> As I don't see a way to gracefully solve this except keeping
> drop_extents(), lets drop this patch instead.

Understood.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] btrfs: use fs_info for btrfs_handle_em_exist tracepoint

[David Sterba]

On Tue, Apr 10, 2018 at 10:45:39AM +0300, Nikolay Borisov wrote:
> On  9.04.2018 14:58, David Sterba wrote:
> > We really want to know to which filesystem the extent map events belong,
> > but as it cannot be reached from the extent_map pointers, we need to
> > pass it down the callchain.
> 
> I really dislike propagating arguments solely for tracepoints purposes,
> but if we don't have any other choice then I guess we should go with it.
> However...
> 
> > @@ -7092,7 +7092,7 @@ struct extent_map *btrfs_get_extent(struct 
> > btrfs_inode *inode,
> >  
> > err = 0;
> > write_lock(_tree->lock);
> > -   err = btrfs_add_extent_mapping(em_tree, , start, len);
> > +   err = btrfs_add_extent_mapping(fs_info, em_tree, , start, len);
> 
> This function is called only here, and we know that em_tree passed
> points to a struct, stored in btrfs_inode. So can't we use container_of
> to get the btrfs_inode in this function, from where we can reference the
> fs_info? I guess it could be a problem for tests.
> 
> 
> Admittedly  this feels somewhat hacky and I guess is not very
> future-proof, but it's still a viable alternative.

Sounds too fragile to me, from all the alternatives passing fs_info
looks like the cleanest way for now.  The filesystem UUID in the
tracepoint is IMO an important part.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] Tracepoint updates

[Nikolay Borisov]

On  9.04.2018 14:58, David Sterba wrote:
> Switch the inode number type to u64, plus other cleanups and fixups.
> 
> David Sterba (7):
>   btrfs: tracepoints, use correct type for inode number
>   btrfs: tracepoints, use %llu instead of %Lu
>   btrfs: tracepoints, drop unnecessary ULL casts
>   btrfs: tracepoints, fix whitespace in strings
>   btrfs: tracepoints, use extended format with UUID where possible
>   btrfs: tests: pass fs_info to extent_map tests
>   btrfs: use fs_info for btrfs_handle_em_exist tracepoint

Reviewed-by: Nikolay Borisov 

> 
>  fs/btrfs/extent_map.c |   6 +-
>  fs/btrfs/extent_map.h |   3 +-
>  fs/btrfs/inode.c  |   2 +-
>  fs/btrfs/tests/extent-map-tests.c |  60 ++
>  include/trace/events/btrfs.h  | 225 
> +++---
>  5 files changed, 158 insertions(+), 138 deletions(-)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] btrfs: use fs_info for btrfs_handle_em_exist tracepoint

[Nikolay Borisov]

On  9.04.2018 14:58, David Sterba wrote:
> We really want to know to which filesystem the extent map events belong,
> but as it cannot be reached from the extent_map pointers, we need to
> pass it down the callchain.

I really dislike propagating arguments solely for tracepoints purposes,
but if we don't have any other choice then I guess we should go with it.
However...

> 
> Signed-off-by: David Sterba 
> ---
>  fs/btrfs/extent_map.c |  6 --
>  fs/btrfs/extent_map.h |  3 ++-
>  fs/btrfs/inode.c  |  2 +-
>  fs/btrfs/tests/extent-map-tests.c |  8 
>  include/trace/events/btrfs.h  | 12 +++-
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 53a0633c6ef7..581b42d23e0d 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -517,6 +517,7 @@ static noinline int merge_extent_mapping(struct 
> extent_map_tree *em_tree,
>  
>  /**
>   * btrfs_add_extent_mapping - add extent mapping into em_tree
> + * @fs_info - used for tracepoint
>   * @em_tree - the extent tree into which we want to insert the extent mapping
>   * @em_in   - extent we are inserting
>   * @start   - start of the logical range btrfs_get_extent() is requesting
> @@ -534,7 +535,8 @@ static noinline int merge_extent_mapping(struct 
> extent_map_tree *em_tree,
>   * Return 0 on success, otherwise -EEXIST.
>   *
>   */
> -int btrfs_add_extent_mapping(struct extent_map_tree *em_tree,
> +int btrfs_add_extent_mapping(struct btrfs_fs_info *fs_info,
> +  struct extent_map_tree *em_tree,
>struct extent_map **em_in, u64 start, u64 len)
>  {
>   int ret;
> @@ -552,7 +554,7 @@ int btrfs_add_extent_mapping(struct extent_map_tree 
> *em_tree,
>  
>   existing = search_extent_mapping(em_tree, start, len);
>  
> - trace_btrfs_handle_em_exist(existing, em, start, len);
> + trace_btrfs_handle_em_exist(fs_info, existing, em, start, len);
>  
>   /*
>* existing will always be non-NULL, since there must be
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index f6f8ba114977..f55c8b4ef120 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -91,6 +91,7 @@ int unpin_extent_cache(struct extent_map_tree *tree, u64 
> start, u64 len, u64 gen
>  void clear_em_logging(struct extent_map_tree *tree, struct extent_map *em);
>  struct extent_map *search_extent_mapping(struct extent_map_tree *tree,
>u64 start, u64 len);
> -int btrfs_add_extent_mapping(struct extent_map_tree *em_tree,
> +int btrfs_add_extent_mapping(struct btrfs_fs_info *fs_info,
> +  struct extent_map_tree *em_tree,
>struct extent_map **em_in, u64 start, u64 len);
>  #endif
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 1f091c2358a4..18c31006865f 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7092,7 +7092,7 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode 
> *inode,
>  
>   err = 0;
>   write_lock(_tree->lock);
> - err = btrfs_add_extent_mapping(em_tree, , start, len);
> + err = btrfs_add_extent_mapping(fs_info, em_tree, , start, len);

This function is called only here, and we know that em_tree passed
points to a struct, stored in btrfs_inode. So can't we use container_of
to get the btrfs_inode in this function, from where we can reference the
fs_info? I guess it could be a problem for tests.


Admittedly  this feels somewhat hacky and I guess is not very
future-proof, but it's still a viable alternative.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html