On 09/27/2017 10:17 PM, David Sterba wrote:
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.
Pls consider V4, in the ML.
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.
Fixed this in
[PATCH v4 3/3] btrfs: error out if btrfs_attach_transaction() 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.
As it goes deeper there are quite a number of things which aren't
un-done during fail error return .. adding one more to the list
is sb->super_copy updates. With this current design on this function
its kind of too difficult to undo and error return. As
btrfs_init_new_device() is shared between normal device add and
sprout device add. I am mulling on completely removing seed part
to outside of the btrfs_init_new_device(). such as ..
prepare sprout.
ret = btrfs_init_new_device() which is without the seed part
if(ret) undo_prepare_sprout
else finish sprouting.
Also with this I think we would find few duplicate code sections
between btrfs_init_new_device() and replace device which will be
a nice cleanup as a whole. This is a long term plan, for now
I think v4 is good.
Thanks, Anand
--
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
--
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
