On Fri, 2019-07-26 at 11:00 -0700, Bart Van Assche wrote:
> On 7/26/19 10:00 AM, James Bottomley wrote:
> > On Fri, 2019-07-26 at 09:48 -0700, Bart Van Assche wrote:
> > > If scsi_target_block() fails that can break the code that calls
> > > this
> > > function. Hence complain loudly if scsi_target_block() fails.
> > > 
> > > Cc: Christoph Hellwig <h...@lst.de>
> > > Cc: Hannes Reinecke <h...@suse.com>
> > > Cc: Johannes Thumshirn <jthumsh...@suse.de>
> > > Cc: Ming Lei <ming....@redhat.com>
> > > Signed-off-by: Bart Van Assche <bvanass...@acm.org>
> > > ---
> > >   drivers/scsi/scsi_lib.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > index bbed72eff9c9..c9630bd59b5a 100644
> > > --- a/drivers/scsi/scsi_lib.c
> > > +++ b/drivers/scsi/scsi_lib.c
> > > @@ -2770,6 +2770,8 @@ int scsi_target_block(struct device *dev)
> > >           else
> > >                   device_for_each_child(dev, &ret, target_block);
> > >   
> > > + WARN_ONCE(ret, "ret = %d\n", ret);
> > > +
> > 
> > If this is the only point to the previous change to make SCSI
> > target block return an error, why not put the WARN_ONCE in
> > device_block?  That way you'll at least know which device was the
> > problem.
> 
> Hi James,
> 
> Adding a WARN_ON_ONCE() statement in device_block() sounds like a
> good idea to me. But since scsi_target_block() can fail, I think it
> should  have a return value that indicates whether or not it
> succeeded.

I don't disagree, but nothing in this patch set actually uses it ... is
there a plan for something to use the scsi_target_block return value to
perform some action other than issue a warning?   If not, it likely
makes better sense simply to add the warn once to the device block ...
unless there's some problem that might make it too verbose for a target
(say 100 devices each of which would issue the warning).

James

Reply via email to