Re: [PATCH] scsi: scsi_dh: Return error pointer in scsi_dh_attached_handler_name

[Martin Wilck <mwilck@suse.com>]

On Mon, 2025-12-01 at 20:00 -0500, Benjamin Marzinski wrote:
> On Thu, Nov 27, 2025 at 04:26:51PM +0100, Martin Wilck wrote:
> > On Fri, 2025-11-21 at 18:48 -0500, Benjamin Marzinski wrote:
> > > If scsi_dh_attached_handler_name() fails to allocate the handler
> > > name,
> > > dm-multipath (its only caller) assumes there is no attached
> > > device
> > > handler, and sets the device up incorrectly. Return an error
> > > pointer
> > > instead, so multipath can distinguish between failure and success
> > > where
> > > there is no attached device handler.
> > > 
> > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > ---
> > >  drivers/md/dm-mpath.c  | 8 ++++++++
> > >  drivers/scsi/scsi_dh.c | 8 +++++---
> > >  2 files changed, 13 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> > > index c18358271618..063dc526fe04 100644
> > > --- a/drivers/md/dm-mpath.c
> > > +++ b/drivers/md/dm-mpath.c
> > > @@ -950,6 +950,14 @@ static struct pgpath *parse_path(struct
> > > dm_arg_set *as, struct path_selector *ps
> > >  
> > >  	q = bdev_get_queue(p->path.dev->bdev);
> > >  	attached_handler_name = scsi_dh_attached_handler_name(q,
> > > GFP_KERNEL);
> > > +	if (IS_ERR(attached_handler_name)) {
> > > +		if (PTR_ERR(attached_handler_name) == -ENODEV)
> > > +			attached_handler_name = NULL;
> > 
> > What's the point of continuing here if we know that the SCSI device
> > doesn't exist?
> 
> we just know that it's not a SCSI device, so we clear
> attached_handler_name.  I suppose we could add another check to error
> out here if m->hw_handler_name is set. But if it is, the
> setup_scsi_dh()
> call just below will fail anyways, so there's not much difference.
> 
> But if you think adding that extra check makes things clearer, I'm
> fine
> with that.

Yes, I think so. We know it makes no sense to call setup_scsi_dh(), wo
we shouldn't try.

Martin