On Thu, Aug 02, 2018 at 09:07:00PM +0800, Anand Jain wrote:
> >>> - num_devices = fs_devices->num_devices;
> >>> - btrfs_dev_replace_read_lock(&fs_info->dev_replace);
> >>> - if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
> >>> - BUG_ON(num_devices < 1);
> >>> - num_devices--;
> >>> - }
> >>> - btrfs_dev_replace_read_unlock(&fs_info->dev_replace);
> >>> + num_devices = btrfs_num_devices(fs_info);
> >>
> >> How about lifting the BUG_ON from btrfs_num_devices into a check in this
> >> function, so if num_devices < 1 then we just exit with -EINVAL or some
> >> such. We should be aiming at eliminating BUG_ONs.
> >
> > Right, in both cases it's possible to return with an error instead of
> > the BUG_ON.
>
> Actually we should just remove it as its a logical bug if num_devices <
> 1, so long we didn't hit this bug which means its stable OR keep BUG_ON
> it until we add RAID-N. ?
I think the BUG_ON is redundant, all the sanity checks at mount time or
ioctl remove/replace make sure that there are enough devices to perform
the action. Still, it can be an assert, such things do not hurt and may
be catch other errors someday.
--
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
