All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: Lu Fengqi <lufq.fnst@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Subject: Re: [RFC PATCH v2 1/4] btrfs: factor out btrfs_link_subvol from create_subvol
Date: Tue, 11 Sep 2018 19:57:03 +0800	[thread overview]
Message-ID: <5a15af8a-687b-3a16-867b-9d1e72d24a28@gmx.com> (raw)
In-Reply-To: <20180911112903.25985-2-lufq.fnst@cn.fujitsu.com>


[-- Attachment #1.1: Type: text/plain, Size: 3322 bytes --]



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@cn.fujitsu.com>

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?

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);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2018-09-11 16:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-11 11:28 [RFC PATCH v2 0/4] undelete subvolume online version Lu Fengqi
2018-09-11 11:29 ` [RFC PATCH v2 1/4] btrfs: factor out btrfs_link_subvol from create_subvol Lu Fengqi
2018-09-11 11:57   ` Qu Wenruo [this message]
2018-09-11 12:10     ` Lu Fengqi
2018-09-11 11:29 ` [RFC PATCH v2 2/4] btrfs: don't BUG_ON() in btrfs_link_subvol() Lu Fengqi
2018-09-11 11:54   ` Qu Wenruo
2018-09-11 11:29 ` [RFC PATCH v2 3/4] btrfs: undelete: introduce btrfs_undelete_subvolume Lu Fengqi
2018-09-11 11:29 ` [RFC PATCH v2 4/4] btrfs: undelete: Add BTRFS_IOCTL_SUBVOL_UNDELETE ioctl Lu Fengqi
2018-09-11 11:34 ` [RFC PATCH v2 1/2] btrfs-progs: ioctl: add BTRFS_IOC_SUBVOL_UNDELETE to ioctl.h Lu Fengqi
2018-09-11 11:34   ` [RFC PATCH v2 2/2] btrfs-progs: subvolume: undelete: add btrfs subvolume undelete subcommand Lu Fengqi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5a15af8a-687b-3a16-867b-9d1e72d24a28@gmx.com \
    --to=quwenruo.btrfs@gmx.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=lufq.fnst@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.