On 08/08/2018 01:09 AM, David Sterba wrote:
On Mon, Aug 06, 2018 at 04:57:45PM +0800, Anand Jain wrote:
On 08/03/2018 09:33 PM, Nikolay Borisov wrote:
On 3.08.2018 15:45, Anand Jain wrote:
Its a logical bug if we hit fs_devices::num_devices == 1 and if the
replace is running because, as fs_devices::num_devices counts the in memory
devices, so it should include the replace target which is running as
indicated by the flag. If this happens return the -EINVAL back.
Suggested-by: Nikolay Borisov <nbori...@suse.com>
Signed-off-by: Anand Jain <anand.j...@oracle.com>
---
Hi,
As it fixes the BUG_ON I have spun a new patch for this.
Instead of -EINVAL should we use ASSERT?
fs/btrfs/volumes.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 7359596ac8eb..ed2399caff80 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1855,9 +1855,11 @@ void btrfs_assign_next_active_device(struct btrfs_device
*device,
}
/* Returns btrfs_fs_devices::num_devices minus replace device if any */
-static u64 btrfs_num_devices(struct btrfs_fs_info *fs_info)
+static int btrfs_num_devices(struct btrfs_fs_info *fs_info, u64 *num_devices)
Why do you resort to this travesty of returning the value in an input
parameter? Having the function return int, assuming that we will always
have a positive device num and in case of an error return a negative
value. In the worst case when we get to see a btrfs fs consisting of 2
billion devices then we can start worrying that an int here won't do it.
Its theoretically wrong. I wonder if David is OK with this? Will wait
for his comments.
Theoretically, as in having 2^64 devices where the numbers would clash
with error code. Practically we're talking about tens maybe hundreds of
devices, anything that fits to 32 bits in the forseeable future. Yes I
agree with Nikolai.
But the BUG_ON can be removed, or the number of devices can be checked
in advance as mentioned in other mails, I'm starting to lose track of
what's the last version.
Other email thread [1] were talking about replacing the BUG_ON with
ASSERT, which means we don't need this patch.
[1]
[PATCH v3 4/4] btrfs: add helper btrfs_num_devices() to deduce num_devices
Thanks, Anand
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
