On Tue, Sep 11, 2018 at 07:57:03PM +0800, Qu Wenruo wrote:
>
>
>On 2018/9/11 下午7:29, Lu Fengqi wrote:
>> The function btrfs_link_subvol is responsible to link the subvolume to
>> the specified directory, which is the opposite of what
>> btrfs_unlink_subvol does.
>>
>> No functional change.
>>
>> Signed-off-by: Lu Fengqi <lufq.fnst@xxxxxxxxxxxxxx>
>
>The patch itself is OK.
>
>Just small nitpicks inlined below.
>
>> ---
>> fs/btrfs/ioctl.c | 64 +++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 41 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 4905d13dee0a..1b03d07acde2 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -542,6 +542,45 @@ int btrfs_is_empty_uuid(u8 *uuid)
>> return 1;
>> }
>>
>> +static int btrfs_link_subvol(struct btrfs_trans_handle *trans,
>> + struct inode *dir, u64 objectid, const char *name,
>> + int namelen)
>> +{
>> + struct btrfs_root *root = BTRFS_I(dir)->root;
>> + struct btrfs_key key;
>> + u64 index = 0;
>> + int ret;
>> +
>> + /*
>> + * insert the directory item
>> + */
>> + ret = btrfs_set_inode_index(BTRFS_I(dir), &index);
>> + if (ret) {
>> + btrfs_abort_transaction(trans, ret);
>> + return ret;
>> + }
>> +
>> + key.objectid = objectid;
>> + key.type = BTRFS_ROOT_ITEM_KEY;
>> + key.offset = -1;
>> + ret = btrfs_insert_dir_item(trans, name, namelen, BTRFS_I(dir), &key,
>> + BTRFS_FT_DIR, index);
>> + if (ret) {
>> + btrfs_abort_transaction(trans, ret);
>> + return ret;
>> + }
>> +
>> + btrfs_i_size_write(BTRFS_I(dir), dir->i_size + namelen * 2);
>> + ret = btrfs_update_inode(trans, root, dir);
>> + BUG_ON(ret);
>
>What about clean up this BUG_ON()?
>
>> +
>> + ret = btrfs_add_root_ref(trans, objectid, root->root_key.objectid,
>> + btrfs_ino(BTRFS_I(dir)), index, name, namelen);
>> + BUG_ON(ret);
>
>And this one?
Sorry, this makes you confused. This is exactly the cleanup done by Patch 2, because
I want to just move the code in Patch 1.
Thanks,
Lu
>
>Thanks,
>Qu
>
>> +
>> + return ret;
>> +}
>> +
>> static noinline int create_subvol(struct inode *dir,
>> struct dentry *dentry,
>> const char *name, int namelen,
>> @@ -563,7 +602,6 @@ static noinline int create_subvol(struct inode *dir,
>> int err;
>> u64 objectid;
>> u64 new_dirid = BTRFS_FIRST_FREE_OBJECTID;
>> - u64 index = 0;
>> uuid_le new_uuid;
>>
>> root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
>> @@ -677,29 +715,9 @@ static noinline int create_subvol(struct inode *dir,
>> new_root->highest_objectid = new_dirid;
>> mutex_unlock(&new_root->objectid_mutex);
>>
>> - /*
>> - * insert the directory item
>> - */
>> - ret = btrfs_set_inode_index(BTRFS_I(dir), &index);
>> - if (ret) {
>> - btrfs_abort_transaction(trans, ret);
>> - goto fail;
>> - }
>> -
>> - ret = btrfs_insert_dir_item(trans, name, namelen, BTRFS_I(dir), &key,
>> - BTRFS_FT_DIR, index);
>> - if (ret) {
>> - btrfs_abort_transaction(trans, ret);
>> + ret = btrfs_link_subvol(trans, dir, objectid, name, namelen);
>> + if (ret)
>> goto fail;
>> - }
>> -
>> - btrfs_i_size_write(BTRFS_I(dir), dir->i_size + namelen * 2);
>> - ret = btrfs_update_inode(trans, root, dir);
>> - BUG_ON(ret);
>> -
>> - ret = btrfs_add_root_ref(trans, objectid, root->root_key.objectid,
>> - btrfs_ino(BTRFS_I(dir)), index, name, namelen);
>> - BUG_ON(ret);
>>
>> ret = btrfs_uuid_tree_add(trans, root_item->uuid,
>> BTRFS_UUID_KEY_SUBVOL, objectid);
>>
>
