Commit caae78e0 authored by Omar Sandoval's avatar Omar Sandoval Committed by David Sterba

btrfs: move common inode creation code into btrfs_create_new_inode()

All of our inode creation code paths duplicate the calls to
btrfs_init_inode_security() and btrfs_add_link(). Subvolume creation
additionally duplicates property inheritance and the call to
btrfs_set_inode_index(). Fix this by moving the common code into
btrfs_create_new_inode(). This accomplishes a few things at once:

1. It reduces code duplication.

2. It allows us to set up the inode completely before inserting the
   inode item, removing calls to btrfs_update_inode().

3. It fixes a leak of an inode on disk in some error cases. For example,
   in btrfs_create(), if btrfs_new_inode() succeeds, then we have
   inserted an inode item and its inode ref. However, if something after
   that fails (e.g., btrfs_init_inode_security()), then we end the
   transaction and then decrement the link count on the inode. If the
   transaction is committed and the system crashes before the failed
   inode is deleted, then we leak that inode on disk. Instead, this
   refactoring aborts the transaction when we can't recover more
   gracefully.

4. It exposes various ways that subvolume creation diverges from mkdir
   in terms of inheriting flags, properties, permissions, and POSIX
   ACLs, a lot of which appears to be accidental. This patch explicitly
   does _not_ change the existing non-standard behavior, but it makes
   those differences more clear in the code and documents them so that
   we can discuss whether they should be changed.
Reviewed-by: default avatarSweet Tea Dorminy <sweettea-kernel@dorminy.me>
Signed-off-by: default avatarOmar Sandoval <osandov@fb.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 3538d68d
...@@ -3302,14 +3302,10 @@ struct btrfs_new_inode_args { ...@@ -3302,14 +3302,10 @@ struct btrfs_new_inode_args {
int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args, int btrfs_new_inode_prepare(struct btrfs_new_inode_args *args,
unsigned int *trans_num_items); unsigned int *trans_num_items);
int btrfs_create_new_inode(struct btrfs_trans_handle *trans, int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
struct btrfs_new_inode_args *args, struct btrfs_new_inode_args *args);
u64 *index);
void btrfs_new_inode_args_destroy(struct btrfs_new_inode_args *args); void btrfs_new_inode_args_destroy(struct btrfs_new_inode_args *args);
struct inode *btrfs_new_subvol_inode(struct user_namespace *mnt_userns, struct inode *btrfs_new_subvol_inode(struct user_namespace *mnt_userns,
struct inode *dir); struct inode *dir);
int btrfs_create_subvol_root(struct btrfs_trans_handle *trans,
struct btrfs_root *parent_root,
struct btrfs_new_inode_args *args);
void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state, void btrfs_set_delalloc_extent(struct inode *inode, struct extent_state *state,
unsigned *bits); unsigned *bits);
void btrfs_clear_delalloc_extent(struct inode *inode, void btrfs_clear_delalloc_extent(struct inode *inode,
......
This diff is collapsed.
...@@ -575,8 +575,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns, ...@@ -575,8 +575,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
struct inode *dir, struct dentry *dentry, struct inode *dir, struct dentry *dentry,
struct btrfs_qgroup_inherit *inherit) struct btrfs_qgroup_inherit *inherit)
{ {
const char *name = dentry->d_name.name;
int namelen = dentry->d_name.len;
struct btrfs_fs_info *fs_info = btrfs_sb(dir->i_sb); struct btrfs_fs_info *fs_info = btrfs_sb(dir->i_sb);
struct btrfs_trans_handle *trans; struct btrfs_trans_handle *trans;
struct btrfs_key key; struct btrfs_key key;
...@@ -596,7 +594,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns, ...@@ -596,7 +594,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
int ret; int ret;
dev_t anon_dev; dev_t anon_dev;
u64 objectid; u64 objectid;
u64 index = 0;
root_item = kzalloc(sizeof(*root_item), GFP_KERNEL); root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
if (!root_item) if (!root_item)
...@@ -713,7 +710,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns, ...@@ -713,7 +710,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
free_extent_buffer(leaf); free_extent_buffer(leaf);
leaf = NULL; leaf = NULL;
key.offset = (u64)-1;
new_root = btrfs_get_new_fs_root(fs_info, objectid, anon_dev); new_root = btrfs_get_new_fs_root(fs_info, objectid, anon_dev);
if (IS_ERR(new_root)) { if (IS_ERR(new_root)) {
ret = PTR_ERR(new_root); ret = PTR_ERR(new_root);
...@@ -731,47 +727,21 @@ static noinline int create_subvol(struct user_namespace *mnt_userns, ...@@ -731,47 +727,21 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
goto out; goto out;
} }
ret = btrfs_create_subvol_root(trans, root, &new_inode_args); ret = btrfs_uuid_tree_add(trans, root_item->uuid,
if (ret) { BTRFS_UUID_KEY_SUBVOL, objectid);
/* We potentially lose an unused inode item here */
btrfs_abort_transaction(trans, ret);
goto out;
}
/*
* insert the directory item
*/
ret = btrfs_set_inode_index(BTRFS_I(dir), &index);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out;
}
ret = btrfs_insert_dir_item(trans, name, namelen, BTRFS_I(dir), &key,
BTRFS_FT_DIR, index);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out;
}
btrfs_i_size_write(BTRFS_I(dir), dir->i_size + namelen * 2);
ret = btrfs_update_inode(trans, root, BTRFS_I(dir));
if (ret) { if (ret) {
btrfs_abort_transaction(trans, ret); btrfs_abort_transaction(trans, ret);
goto out; goto out;
} }
ret = btrfs_add_root_ref(trans, objectid, root->root_key.objectid, ret = btrfs_create_new_inode(trans, &new_inode_args);
btrfs_ino(BTRFS_I(dir)), index, name, namelen);
if (ret) { if (ret) {
btrfs_abort_transaction(trans, ret); btrfs_abort_transaction(trans, ret);
goto out; goto out;
} }
ret = btrfs_uuid_tree_add(trans, root_item->uuid, d_instantiate_new(dentry, new_inode_args.inode);
BTRFS_UUID_KEY_SUBVOL, objectid); new_inode_args.inode = NULL;
if (ret)
btrfs_abort_transaction(trans, ret);
out: out:
trans->block_rsv = NULL; trans->block_rsv = NULL;
...@@ -782,11 +752,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns, ...@@ -782,11 +752,6 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
btrfs_end_transaction(trans); btrfs_end_transaction(trans);
else else
ret = btrfs_commit_transaction(trans); ret = btrfs_commit_transaction(trans);
if (!ret) {
d_instantiate(dentry, new_inode_args.inode);
new_inode_args.inode = NULL;
}
out_new_inode_args: out_new_inode_args:
btrfs_new_inode_args_destroy(&new_inode_args); btrfs_new_inode_args_destroy(&new_inode_args);
out_inode: out_inode:
......
...@@ -380,9 +380,8 @@ static struct prop_handler prop_handlers[] = { ...@@ -380,9 +380,8 @@ static struct prop_handler prop_handlers[] = {
}, },
}; };
static int inherit_props(struct btrfs_trans_handle *trans, int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
struct inode *inode, struct inode *inode, struct inode *parent)
struct inode *parent)
{ {
struct btrfs_root *root = BTRFS_I(inode)->root; struct btrfs_root *root = BTRFS_I(inode)->root;
struct btrfs_fs_info *fs_info = root->fs_info; struct btrfs_fs_info *fs_info = root->fs_info;
...@@ -457,41 +456,6 @@ static int inherit_props(struct btrfs_trans_handle *trans, ...@@ -457,41 +456,6 @@ static int inherit_props(struct btrfs_trans_handle *trans,
return 0; return 0;
} }
int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
struct inode *inode,
struct inode *dir)
{
if (!dir)
return 0;
return inherit_props(trans, inode, dir);
}
int btrfs_subvol_inherit_props(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
struct btrfs_root *parent_root)
{
struct super_block *sb = root->fs_info->sb;
struct inode *parent_inode, *child_inode;
int ret;
parent_inode = btrfs_iget(sb, BTRFS_FIRST_FREE_OBJECTID, parent_root);
if (IS_ERR(parent_inode))
return PTR_ERR(parent_inode);
child_inode = btrfs_iget(sb, BTRFS_FIRST_FREE_OBJECTID, root);
if (IS_ERR(child_inode)) {
iput(parent_inode);
return PTR_ERR(child_inode);
}
ret = inherit_props(trans, child_inode, parent_inode);
iput(child_inode);
iput(parent_inode);
return ret;
}
void __init btrfs_props_init(void) void __init btrfs_props_init(void)
{ {
int i; int i;
......
...@@ -23,8 +23,4 @@ int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans, ...@@ -23,8 +23,4 @@ int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
struct inode *inode, struct inode *inode,
struct inode *dir); struct inode *dir);
int btrfs_subvol_inherit_props(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
struct btrfs_root *parent_root);
#endif #endif
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment