Re: [PATCH v2] dm: gracefully fail any request beyond the end of the device

[Mike Snitzer <snitzer@redhat.com>]

On Mon, Sep 24 2012 at  5:38am -0400,
Jun'ichi Nomura <j-nomura@ce.jp.nec.com> wrote:

> On 09/22/12 00:47, Mike Snitzer wrote:
> > @@ -1651,19 +1654,31 @@ static void dm_request_fn(struct request
> >  		if (!rq)
> >  			goto delay_and_out;
> >  
> > +		clone = rq->special;
> > +
> >  		/* always use block 0 to find the target for flushes for now */
> >  		pos = 0;
> >  		if (!(rq->cmd_flags & REQ_FLUSH))
> >  			pos = blk_rq_pos(rq);
> >  
> >  		ti = dm_table_find_target(map, pos);
> > -		BUG_ON(!dm_target_is_valid(ti));
> > +		if (!dm_target_is_valid(ti)) {
> > +			/*
> > +			 * Must perform setup, that dm_done() requires,
> > +			 * before calling dm_kill_unmapped_request
> > +			 */
> > +			DMERR_LIMIT("request attempted access beyond the end of device");
> > +			blk_start_request(rq);
> > +			atomic_inc(&md->pending[rq_data_dir(clone)]);
> > +			dm_get(md);
> > +			dm_kill_unmapped_request(clone, -EIO);
> > +			goto out;
> 
> This "goto out" should be "continue" so that request_fn
> process next requests in the queue.
> 
> Also I think introducing a function dm_start_request()
> will make this part of code a little bit easier for reading.
> An edited patch is attached.

Aside from the continue, matches exactly what I was going to do for v3
(based on Mike Christie's feedback -- which was to introduce
dm_start_request too).  Anyway, looks great.

I'll get a formal v3 posted so Alasdair can stage it.

Thanks,
Mike