Re: [PATCH] btrfs: take the error path out if btrfs_attach_transaction() fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 27, 2017 at 05:50:52PM +0800, Anand Jain wrote:
> btrfs_init_new_device() calls btrfs_attach_transaction() to
> commit sys chunks, however take the error path out if it fails.
> 
> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> ---
>  fs/btrfs/volumes.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index fad3b10a1f81..b526d13a74da 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2494,7 +2494,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>  		if (IS_ERR(trans)) {
>  			if (PTR_ERR(trans) == -ENOENT)
>  				return 0;
> -			return PTR_ERR(trans);
> +			ret = PTR_ERR(trans);
> +			goto error_sysfs;

The label is introduced by another patch, please resend the whole
patchset, I've seen several iterations and feedback from various people
and I'm not sure I'm looking at the latest version.

Regarding error handling in btrfs_init_new_device, the seeding device
makes it hard to read. This patch would lead to a double unlock of
uuid_mutex and sb::s_umount, because the label error_sysfs will continue
to do the cleanups, that were already partially done in the containing
'if (seeding_dev)' block where the test fails.

I'd suggest to first get rid of the in-place returns and add necessary
labels or separate exit sequences and then address the new error
handling.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux