Re: [PATCH RESEND] dm-raid: Do not call BUG() in __rdev_sectors()

[Mike Snitzer <snitzer@redhat.com>]

On Wed, Jun 28 2017 at  4:04P -0400,
Hannes Reinecke <hare@suse.de> wrote:

> __rdev_sectors() might be called for an invalid/non-existing
> RAID set during raid_ctr(), which is perfectly fine and no
> reason to panic.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/md/dm-raid.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 7d89322..9bbb596 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -1571,7 +1571,8 @@ static sector_t __rdev_sectors(struct raid_set *rs)
>  			return rdev->sectors;
>  	}
>  
> -	BUG(); /* Constructor ensures we got some. */
> +	/* No valid raid devices */
> +	return 0;
>  }
>  
>  /* Calculate the sectors per device and per array used for @rs */
> -- 
> 1.8.5.6
> 

The use of BUG() is certainly suspect.  BUG() is very rarely the right
answer.

But I don't think returning 0 makes sense for all __rdev_sectors()
callers, e.g.: rs_setup_recovery()

__rdev_sectors() could easily be made to check if ti->error is passed as
a non-NULL pointer.. if so set *error, return 0, have ctr check for 0,
and gracefully fail ctr by returning the ti->error that __rdev_sectors()
set.  If the error pointer passed to __rdev_sectors() is NULL, resort to
BUG() still?  Would really like to avoid BUG().. but I'll defer to Heinz
on what might be a better response.

Alternatively, you could look to see where "Constructor ensures we got
some." and fix the fact that it clearly isn't ensuring as much for your
case?

Mike