Re: [PATCH] dm-mirror: do not degrade the mirror on discard error

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Mon, 2015-02-16 at 07:44 -0500, Mikulas Patocka wrote:
> 
> On Sat, 14 Feb 2015, James Bottomley wrote:
> 
> > On Thu, 2015-02-12 at 10:09 -0500, Mikulas Patocka wrote:
> > > It may be possible that a device claims discard support but it rejects
> > > discards with -EOPNOTSUPP. It happens when using loopback on ext2/ext3
> > > filesystem driven by the ext4 driver. It may also happen if the underlying
> > > devices are moved from one disk on another.
> > > 
> > > If discard error happens, we reject the bio with -EOPNOTSUPP, but we do
> > > not degrade the array.
> > > 
> > > This patch fixes failed test shell/lvconvert-repair-transient.sh in the
> > > lvm2 testsuite if the testsuite is extracted on an ext2 or ext3 filesystem
> > > and it is being driven by the ext4 driver.
> > > 
> > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > > 
> > > Index: linux-2.6/drivers/md/dm-raid1.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/md/dm-raid1.c
> > > +++ linux-2.6/drivers/md/dm-raid1.c
> > > @@ -604,6 +604,15 @@ static void write_callback(unsigned long
> > >  		return;
> > >  	}
> > >  
> > > +	/*
> > > +	 * If the bio is discard, return an error, but do not
> > > +	 * degrade the array.
> > > +	 */
> > > +	if (bio->bi_rw & REQ_DISCARD) {
> > > +		bio_endio(bio, -EOPNOTSUPP);
> > 
> > I think the error gets ignored, so this is probably harmless, but
> > shouldn't you propagate the actual error here?  discard is advisory and
> > can fail for a variety of reasons (alignment being chief) for which
> > EOPNOTSUPP is inappropriate.
> > 
> > James
> 
> dm-io doesn't pass the error code to the callback, it only returns the 
> bitmask of failed devices. So, it is impossible to find the real error 
> code.

I already said this in the first sentence of the last paragraph of my
email.  The point isn't what it does today it's what might happen
tomorrow and the principle of least surprise.  One day, someone might
propagate the error.  When that happens, they'll be surprised to find
every discard failure reported as -ENOTSUPP and it will cost someone
time and effort to investigate and fix.  If you just propagate the error
today, you save all that work in the future.

James